kernel: sucompat: increase reliability, commonize and micro-optimize (tiann#2656)

On plain ARMv8.0 devices (A53,A57,A73), strncpy_from_user_nofault() sometimes
fails to copy `filename_user` string correctly. This breaks su ofc, breaking
some apps like Termux (Play Store ver), ZArchiver and Root Explorer.

This does NOT seem to affect newer ARMv8.2+ CPUs (A75/A76 and newer)

My speculation? ARMv8.0 has weak speculation :)

here we replace `ksu_strncpy_from_user_nofault` with ksu_strncpy_from_user_retry:
- ksu_strncpy_from_user_nofault as fast-path copy
- fallback to access_ok to validate the pointer + strncpy_from_user
- manual null-termination just in case, as strncpy_from_user_nofault also does it
- remove that memset, seems useless as it is an strncpy, not strncat

basically, we retry on pagefualt

for usercopies, its not like were doing
	memset(dest, 0, sizeof(dest));
	strncat(dest, var, bytes);

that memset seems unneeded. instead we use strncpy itself to do proper
error and oob check and null term it after.

as for optimizations
- just return early if unauthorized
- commonized logic
- reduced duplication

Tested on:
- ARMv8.0 A73.a53, A57.a53, A53.a53
- ARMv8.2 A76.a55

Stale: tiann#2656

Signed-off-by: backslashxx <[email protected]>

Author: backslashxx

kernel/kernel_compat.c

@@ -173,3 +173,12 @@ long ksu_strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
 	return ret;
 }
 #endif
+
+int ksu_access_ok(const void *addr, unsigned long size)
+{
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,0,0)
+	return access_ok(addr, size);
+#else
+	return access_ok(VERIFY_READ, addr, size);
+#endif
+}

kernel/kernel_compat.h

@@ -22,4 +22,6 @@ extern ssize_t ksu_kernel_read_compat(struct file *p, void *buf, size_t count,
 extern ssize_t ksu_kernel_write_compat(struct file *p, const void *buf,
 				       size_t count, loff_t *pos);
 
+extern int ksu_access_ok(const void *addr, unsigned long size);
+
 #endif

kernel/sucompat.c

@@ -48,50 +48,91 @@ static char __user *ksud_user_path(void)
 	return userspace_stack_buffer(ksud_path, sizeof(ksud_path));
 }
 
-int ksu_handle_faccessat(int *dfd, const char __user **filename_user, int *mode,
-			 int *__unused_flags)
+static long ksu_strncpy_from_user_retry(char *dst, 
+			const void __user *unsafe_addr, long count)
+{
+	long ret = ksu_strncpy_from_user_nofault(dst, unsafe_addr, count);
+	if (likely(ret >= 0))
+		return ret;
+
+	// we faulted! fallback to slow path
+	if (unlikely(!ksu_access_ok(unsafe_addr, count)))
+		return -EFAULT;
+
+	return strncpy_from_user(dst, unsafe_addr, count);
+}
+
+// every little bit helps here
+__attribute__((hot, no_stack_protector))
+static __always_inline bool is_su_allowed(const void *ptr_to_check)
+{
+	barrier();
+
+	if (likely(!ksu_is_allow_uid(current_uid().val)))
+		return false;
+
+	if (unlikely(!ptr_to_check))
+		return false;
+
+	return true;
+}
+
+static int ksu_sucompat_user_common(const char __user **filename_user,
+				const char *syscall_name,
+				const bool escalate)
 {
 	const char su[] = SU_PATH;
 
-	if (!ksu_is_allow_uid(current_uid().val)) {
+	char path[sizeof(su)]; // sizeof includes nullterm already!
+	long len = ksu_strncpy_from_user_retry(path, *filename_user, sizeof(path));
+	if (len <= 0) // sizeof(su) is not zero
 		return 0;
-	}
 
-	char path[sizeof(su) + 1];
-	memset(path, 0, sizeof(path));
-	ksu_strncpy_from_user_nofault(path, *filename_user, sizeof(path));
+	path[sizeof(path) - 1] = '\0';
 
-	if (unlikely(!memcmp(path, su, sizeof(su)))) {
-		pr_info("faccessat su->sh!\n");
+	if (memcmp(path, su, sizeof(su)))
+		return 0;
+
+	if (escalate) {
+		pr_info("%s su found\n", syscall_name);
+		*filename_user = ksud_user_path();
+		escape_to_root(); // escalate !!
+	} else {
+		pr_info("%s su->sh!\n", syscall_name);
 		*filename_user = sh_user_path();
 	}
 
 	return 0;
 }
 
-int ksu_handle_stat(int *dfd, const char __user **filename_user, int *flags)
+// sys_faccessat
+int ksu_handle_faccessat(int *dfd, const char __user **filename_user, int *mode,
+			 int *__unused_flags)
 {
-	// const char sh[] = SH_PATH;
-	const char su[] = SU_PATH;
-
-	if (!ksu_is_allow_uid(current_uid().val)) {
+	if (!is_su_allowed((const void *)filename_user))
 		return 0;
-	}
 
-	if (unlikely(!filename_user)) {
+	return ksu_sucompat_user_common(filename_user, "faccessat", false);
+}
+
+// sys_newfstatat, sys_fstat64
+int ksu_handle_stat(int *dfd, const char __user **filename_user, int *flags)
+{
+	if (!is_su_allowed((const void *)filename_user))
 		return 0;
-	}
 
-	char path[sizeof(su) + 1];
-	memset(path, 0, sizeof(path));
-	ksu_strncpy_from_user_nofault(path, *filename_user, sizeof(path));
+	return ksu_sucompat_user_common(filename_user, "newfstatat", false);
+}
 
-	if (unlikely(!memcmp(path, su, sizeof(su)))) {
-		pr_info("newfstatat su->sh!\n");
-		*filename_user = sh_user_path();
-	}
+// sys_execve, compat_sys_execve
+int ksu_handle_execve_sucompat(int *fd, const char __user **filename_user,
+			       void *__never_use_argv, void *__never_use_envp,
+			       int *__never_use_flags)
+{
+	if (!is_su_allowed((const void *)filename_user))
+		return 0;
 
-	return 0;
+	return ksu_sucompat_user_common(filename_user, "sys_execve", true);
 }
 
 // the call from execve_handler_pre won't provided correct value for __never_use_argument, use them after fix execve_handler_pre, keeping them for consistence for manually patched code
@@ -103,7 +144,7 @@ int ksu_handle_execveat_sucompat(int *fd, struct filename **filename_ptr,
 	const char sh[] = KSUD_PATH;
 	const char su[] = SU_PATH;
 
-	if (unlikely(!filename_ptr))
+	if (!is_su_allowed((const void *)filename_ptr))
 		return 0;
 
 	filename = *filename_ptr;
@@ -114,9 +155,6 @@ int ksu_handle_execveat_sucompat(int *fd, struct filename **filename_ptr,
 	if (likely(memcmp(filename->name, su, sizeof(su))))
 		return 0;
 
-	if (!ksu_is_allow_uid(current_uid().val))
-		return 0;
-
 	pr_info("do_execveat_common su found\n");
 	memcpy((void *)filename->name, sh, sizeof(sh));
 
@@ -125,35 +163,10 @@ int ksu_handle_execveat_sucompat(int *fd, struct filename **filename_ptr,
 	return 0;
 }
 
-int ksu_handle_execve_sucompat(int *fd, const char __user **filename_user,
-			       void *__never_use_argv, void *__never_use_envp,
-			       int *__never_use_flags)
-{
-	const char su[] = SU_PATH;
-	char path[sizeof(su) + 1];
-
-	if (unlikely(!filename_user))
-		return 0;
-
-	memset(path, 0, sizeof(path));
-	ksu_strncpy_from_user_nofault(path, *filename_user, sizeof(path));
-
-	if (likely(memcmp(path, su, sizeof(su))))
-		return 0;
-
-	if (!ksu_is_allow_uid(current_uid().val))
-		return 0;
-
-	pr_info("sys_execve su found\n");
-	*filename_user = ksud_user_path();
-
-	escape_to_root();
-
-	return 0;
-}
-
 int ksu_handle_devpts(struct inode *inode)
 {
+	barrier();
+
 	if (!current->mm) {
 		return 0;
 	}