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

@@ -183,3 +183,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

@@ -23,4 +23,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

@@ -51,136 +51,113 @@ static inline 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)
 {
-#ifndef CONFIG_KSU_SUSFS_SUS_SU
-	if (!ksu_is_allow_uid(current_uid().val)) {
-		return 0;
-	}
-#endif
+	long ret = ksu_strncpy_from_user_nofault(dst, unsafe_addr, count);
+	if (likely(ret >= 0))
+		return ret;
 
-#ifdef CONFIG_KSU_SUSFS_SUS_SU
-	char path[sizeof(su) + 1] = {0};
-#else
-	char path[sizeof(su) + 1];
-	memset(path, 0, sizeof(path));
-#endif
-	ksu_strncpy_from_user_nofault(path, *filename_user, sizeof(path));
+	// we faulted! fallback to slow path
+	if (unlikely(!ksu_access_ok(unsafe_addr, count)))
+		return -EFAULT;
 
-	if (unlikely(!memcmp(path, su, sizeof(su)))) {
-		pr_info("faccessat su->sh!\n");
-		*filename_user = sh_user_path();
-	}
-
-	return 0;
+	return strncpy_from_user(dst, unsafe_addr, count);
 }
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 1, 0) && defined(CONFIG_KSU_SUSFS_SUS_SU)
-struct filename* susfs_ksu_handle_stat(int *dfd, const char __user **filename_user, int *flags) {
-	struct filename *name = getname_flags(*filename_user, getname_statx_lookup_flags(*flags), NULL);
+// every little bit helps here
+__attribute__((hot, no_stack_protector))
+static __always_inline bool is_su_allowed(const void *ptr_to_check)
+{
+	barrier();
 
-	if (unlikely(IS_ERR(name) || name->name == NULL)) {
-		return name;
-	}
+	if (likely(!ksu_is_allow_uid(current_uid().val)))
+		return false;
 
-	if (likely(memcmp(name->name, su, sizeof(su)))) {
-		return name;
-	}
+	if (unlikely(!ptr_to_check))
+		return false;
 
-	const char sh[] = SH_PATH;
-	pr_info("vfs_fstatat su->sh!\n");
-	memcpy((void *)name->name, sh, sizeof(sh));
-	return name;
+	return true;
 }
-#endif
 
-int ksu_handle_stat(int *dfd, const char __user **filename_user, int *flags)
+static int ksu_sucompat_user_common(const char __user **filename_user,
+				const char *syscall_name,
+				const bool escalate)
 {
-#ifndef CONFIG_KSU_SUSFS_SUS_SU
-	if (!ksu_is_allow_uid(current_uid().val)) {
-		return 0;
-	}
-#endif
+	const char su[] = SU_PATH;
 
-	if (unlikely(!filename_user)) {
+	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;
-	}
 
-#ifdef CONFIG_KSU_SUSFS_SUS_SU
-	char path[sizeof(su) + 1] = {0};
-#else
-	char path[sizeof(su) + 1];
-	memset(path, 0, sizeof(path));
-#endif
-	ksu_strncpy_from_user_nofault(path, *filename_user, sizeof(path));
+	path[sizeof(path) - 1] = '\0';
 
-	if (unlikely(!memcmp(path, su, sizeof(su)))) {
-		pr_info("newfstatat 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;
 }
 
-// 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
-int ksu_handle_execveat_sucompat(int *fd, struct filename **filename_ptr,
-				 void *__never_use_argv, void *__never_use_envp,
-				 int *__never_use_flags)
+// sys_faccessat
+int ksu_handle_faccessat(int *dfd, const char __user **filename_user, int *mode,
+			 int *__unused_flags)
 {
-	struct filename *filename;
-
-	if (unlikely(!filename_ptr))
+	if (!is_su_allowed((const void *)filename_user))
 		return 0;
 
-	filename = *filename_ptr;
-	if (IS_ERR(filename)) {
-		return 0;
-	}
-
-	if (likely(memcmp(filename->name, su, sizeof(su))))
-		return 0;
+	return ksu_sucompat_user_common(filename_user, "faccessat", false);
+}
 
-#ifndef CONFIG_KSU_SUSFS_SUS_SU
-	if (!ksu_is_allow_uid(current_uid().val))
+// 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;
-#endif
-
-	pr_info("do_execveat_common su found\n");
-	memcpy((void *)filename->name, ksud_path, sizeof(ksud_path));
 
-	escape_to_root();
-
-	return 0;
+	return ksu_sucompat_user_common(filename_user, "newfstatat", false);
 }
 
+// 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)
 {
-	//const char su[] = SU_PATH;
-#ifdef CONFIG_KSU_SUSFS_SUS_SU
-	char path[sizeof(su) + 1] = {0};
-#else
-	char path[sizeof(su) + 1];
-#endif
-
-	if (unlikely(!filename_user))
+	if (!is_su_allowed((const void *)filename_user))
 		return 0;
 
-#ifndef CONFIG_KSU_SUSFS_SUS_SU
-	memset(path, 0, sizeof(path));
-#endif
-	ksu_strncpy_from_user_nofault(path, *filename_user, sizeof(path));
+	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
+int ksu_handle_execveat_sucompat(int *fd, struct filename **filename_ptr,
+				 void *__never_use_argv, void *__never_use_envp,
+				 int *__never_use_flags)
+{
+	struct filename *filename;
 
-	if (likely(memcmp(path, su, sizeof(su))))
+	if (!is_su_allowed((const void *)filename_ptr))
 		return 0;
 
-	if (!ksu_is_allow_uid(current_uid().val))
+	filename = *filename_ptr;
+	if (IS_ERR(filename)) {
 		return 0;
+	}
 
-	pr_info("sys_execve su found\n");
-	*filename_user = ksud_user_path();
+	if (likely(memcmp(filename->name, su, sizeof(su))))
+		return 0;
+
+	pr_info("do_execveat_common su found\n");
+	memcpy((void *)filename->name, ksud_path, sizeof(ksud_path));
 
 	escape_to_root();
 
@@ -189,6 +166,8 @@ int ksu_handle_execve_sucompat(int *fd, const char __user **filename_user,
 
 int ksu_handle_devpts(struct inode *inode)
 {
+	barrier();
+
 	if (!current->mm) {
 		return 0;
 	}