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.

Apply the susfs patch

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

Co-authored-by: backslashxx <118538522+backslashxx@users.noreply.github.com>
Co-authored-by: ShirkNeko <109797057+ShirkNeko@users.noreply.github.com>
Co-authored-by: rsuntk <rsuntk@yukiprjkt.my.id>
Signed-off-by: ShirkNeko <109797057+ShirkNeko@users.noreply.github.com>

kernel/Makefile

@@ -122,9 +122,6 @@ ccflags-y += -DKSU_COMPAT_HAS_SELINUX_STATE
 endif
 
 # Handle optional backports
-ifeq ($(shell grep -q "strncpy_from_user_nofault" $(srctree)/include/linux/uaccess.h; echo $$?),0)
-ccflags-y += -DKSU_OPTIONAL_STRNCPY
-endif
 ifeq ($(shell grep -q "ssize_t kernel_read" $(srctree)/fs/read_write.c; echo $$?),0)
 ccflags-y += -DKSU_OPTIONAL_KERNEL_READ
 endif

kernel/kernel_compat.c

@@ -145,80 +145,12 @@ ssize_t ksu_kernel_write_compat(struct file *p, const void *buf, size_t count,
 #endif
 }
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 8, 0) || defined(KSU_OPTIONAL_STRNCPY)
-long ksu_strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
-				   long count)
-{
-	return strncpy_from_user_nofault(dst, unsafe_addr, count);
-}
-#elif LINUX_VERSION_CODE >= KERNEL_VERSION(5, 3, 0)
-long ksu_strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
-				   long count)
-{
-	return strncpy_from_unsafe_user(dst, unsafe_addr, count);
-}
-#else
-// Copied from: https://elixir.bootlin.com/linux/v4.9.337/source/mm/maccess.c#L201
-long ksu_strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
-				   long count)
-{
-	mm_segment_t old_fs = get_fs();
-	long ret;
-
-	if (unlikely(count <= 0))
-		return 0;
-
-	set_fs(USER_DS);
-	pagefault_disable();
-	ret = strncpy_from_user(dst, unsafe_addr, count);
-	pagefault_enable();
-	set_fs(old_fs);
-
-	if (ret >= count) {
-		ret = count;
-		dst[ret - 1] = '\0';
-	} else if (ret > 0) {
-		ret++;
-	}
-
-	return ret;
-}
-#endif
-
-long ksu_strncpy_from_user_retry(char *dst, const void __user *unsafe_addr,
-				   long count)
-{
-	long ret;
-
-	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))) {
-#ifdef CONFIG_KSU_DEBUG
-		pr_err("%s: faulted!\n", __func__);
-#endif
-		return -EFAULT;
-	}
-
-	// why we don't do like how strncpy_from_user_nofault?
-	ret = strncpy_from_user(dst, unsafe_addr, count);
-
-	if (ret >= count) {
-		ret = count;
-		dst[ret - 1] = '\0';
-	} else if (likely(ret >= 0)) {
-		ret++;
-	}
-
-	return ret;
-}
-
 long ksu_copy_from_user_nofault(void *dst, const void __user *src, size_t size)
 {
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 8, 0)
 	return copy_from_user_nofault(dst, src, size);
+#elif LINUX_VERSION_CODE >= KERNEL_VERSION(5, 3, 0)
+	return probe_user_read(dst, src, size);
 #else
 	// https://elixir.bootlin.com/linux/v5.8/source/mm/maccess.c#L205
 	long ret = -EFAULT;
@@ -238,3 +170,22 @@ long ksu_copy_from_user_nofault(void *dst, const void __user *src, size_t size)
 	return 0;
 #endif
 }
+
+/*
+ * ksu_copy_from_user_retry
+ * try nofault copy first, if it fails, try with plain
+ * paramters are the same as copy_from_user
+ * 0 = success
+ * + hot since this is reused on sucompat
+ */
+__attribute__((hot))
+long ksu_copy_from_user_retry(void *to, 
+		const void __user *from, unsigned long count)
+{
+	long ret = ksu_copy_from_user_nofault(to, from, count);
+	if (likely(!ret))
+		return ret;
+
+	// we faulted! fallback to slow path
+	return copy_from_user(to, from, count);
+}

kernel/kernel_compat.h

@@ -57,13 +57,6 @@ static inline __maybe_unused size_t list_count_nodes(const struct list_head *hea
 #endif
 #endif
 
-extern long ksu_strncpy_from_user_nofault(char *dst,
-					  const void __user *unsafe_addr,
-					  long count);
-extern long ksu_strncpy_from_user_retry(char *dst,
-					  const void __user *unsafe_addr,
-					  long count);
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 10, 0) ||	\
 	defined(CONFIG_IS_HW_HISI) ||	\
 	defined(CONFIG_KSU_ALLOWLIST_WORKAROUND)
@@ -78,22 +71,7 @@ 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 long ksu_copy_from_user_nofault(void *dst, const void __user *src, size_t size);
-/*
+extern long ksu_copy_from_user_retry(void *to, const void __user *from, unsigned long count);
- * ksu_copy_from_user_retry
- * try nofault copy first, if it fails, try with plain
- * paramters are the same as copy_from_user
- * 0 = success
- */
-static long ksu_copy_from_user_retry(void *to, 
-		const void __user *from, unsigned long count)
-{
-	long ret = ksu_copy_from_user_nofault(to, from, count);
-	if (likely(!ret))
-		return ret;
-
-	// we faulted! fallback to slow path
-	return copy_from_user(to, from, count);
-}
 
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 0, 0)
 #define ksu_access_ok(addr, size)	access_ok(addr, size)

kernel/ksud.c

@@ -636,7 +636,7 @@ static int ksu_execve_ksud_common(const char __user *filename_user,
 	if (!filename_user)
 		return 0;
 
-	len = ksu_strncpy_from_user_nofault(path, filename_user, 32);
+	len = ksu_copy_from_user_retry(path, filename_user, 32);
 	if (len <= 0)
 		return 0;
 

kernel/sucompat.c

@@ -55,38 +55,83 @@ 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,
+// every little bit helps here
-			 int *__unused_flags)
+__attribute__((hot, no_stack_protector))
+static __always_inline bool is_su_allowed(const void *ptr_to_check)
 {
 
 #ifndef CONFIG_KSU_KPROBES_HOOK
 	if (!ksu_sucompat_hook_state) {
-		return 0;
+		return false;
 	}
 #endif
 
 #ifndef CONFIG_KSU_SUSFS_SUS_SU
-	if (!ksu_is_allow_uid(current_uid().val)) {
+	if (likely(!ksu_is_allow_uid(current_uid().val)))
+		return false;
+#endif
+
+	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)
+{
+	char path[sizeof(su)]; // sizeof includes nullterm already!
+	if (ksu_copy_from_user_retry(path, *filename_user, sizeof(path)))
 		return 0;
-	}
-#endif
 
-#ifdef CONFIG_KSU_SUSFS_SUS_SU
+	path[sizeof(path) - 1] = '\0';
-	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));
 
-	if (unlikely(!memcmp(path, su, sizeof(su)))) {
+	if (memcmp(path, su, sizeof(su)))
-		pr_info("faccessat su->sh!\n");
+		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;
 }
 
+// sys_faccessat
+int ksu_handle_faccessat(int *dfd, const char __user **filename_user, int *mode,
+			 int *__unused_flags)
+{
+	if (!is_su_allowed((const void *)filename_user))
+		return 0;
+
+	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;
+
+	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)
+{
+	if (!is_su_allowed((const void *)filename_user))
+		return 0;
+
+	return ksu_sucompat_user_common(filename_user, "sys_execve", true);
+}
+
 #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);
@@ -106,56 +151,6 @@ struct filename* susfs_ksu_handle_stat(int *dfd, const char __user **filename_us
 }
 #endif
 
-int ksu_handle_stat(int *dfd, const char __user **filename_user, int *flags)
-{
-
-#ifndef CONFIG_KSU_KPROBES_HOOK
-	if (!ksu_sucompat_hook_state) {
-		return 0;
-	}
-#endif
-
-#ifndef CONFIG_KSU_SUSFS_SUS_SU
-	if (!ksu_is_allow_uid(current_uid().val)) {
-		return 0;
-	}
-#endif
-
-	if (unlikely(!filename_user)) {
-		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
-// Remove this later!! we use syscall hook, so this will never happen!!!!!
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 18, 0) && 0
-	// it becomes a `struct filename *` after 5.18
-	// https://elixir.bootlin.com/linux/v5.18/source/fs/stat.c#L216
-	const char sh[] = SH_PATH;
-	struct filename *filename = *((struct filename **)filename_user);
-	if (IS_ERR(filename)) {
-		return 0;
-	}
-	if (likely(memcmp(filename->name, su, sizeof(su))))
-		return 0;
-	pr_info("vfs_statx su->sh!\n");
-	memcpy((void *)filename->name, sh, sizeof(sh));
-#else
-	ksu_strncpy_from_user_nofault(path, *filename_user, sizeof(path));
-
-	if (unlikely(!memcmp(path, su, sizeof(su)))) {
-		pr_info("newfstatat su->sh!\n");
-		*filename_user = sh_user_path();
-	}
-#endif
-
-	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,
@@ -163,13 +158,7 @@ int ksu_handle_execveat_sucompat(int *fd, struct filename **filename_ptr,
 {
 	struct filename *filename;
 
-#ifndef CONFIG_KSU_KPROBES_HOOK
+	if (!is_su_allowed((const void *)filename_ptr))
-	if (!ksu_sucompat_hook_state) {
-		return 0;
-	}
-#endif
-
-	if (unlikely(!filename_ptr))
 		return 0;
 
 	filename = *filename_ptr;
@@ -180,11 +169,6 @@ int ksu_handle_execveat_sucompat(int *fd, struct filename **filename_ptr,
 	if (likely(memcmp(filename->name, su, sizeof(su))))
 		return 0;
 
-#ifndef CONFIG_KSU_SUSFS_SUS_SU
-	if (!ksu_is_allow_uid(current_uid().val))
-		return 0;
-#endif
-
 	pr_info("do_execveat_common su found\n");
 	memcpy((void *)filename->name, ksud_path, sizeof(ksud_path));
 
@@ -193,55 +177,6 @@ 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;
-#ifdef CONFIG_KSU_SUSFS_SUS_SU
-	char path[sizeof(su) + 1] = {0};
-#else
-	char path[sizeof(su) + 1];
-#endif
-
-#ifndef CONFIG_KSU_KPROBES_HOOK
-	if (!ksu_sucompat_hook_state) {
-		return 0;
-	}
-#endif
-
-	if (unlikely(!filename_user))
-		return 0;
-
-	/*
-	 * nofault variant fails silently due to pagefault_disable
-	 * some cpus dont really have that good speculative execution
-	 * access_ok to substitute set_fs, we check if pointer is accessible
-	 */
-	if (!ksu_access_ok(*filename_user, sizeof(path)))
-		return 0;
-
-	// success = returns number of bytes and should be less than path
-	long len = strncpy_from_user(path, *filename_user, sizeof(path));
-	if (len <= 0 || len > sizeof(path))
-		return 0;
-	// strncpy_from_user_nofault does this too
-	path[sizeof(path) - 1] = '\0';
-
-	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;
-}
-
 // dummified
 int ksu_handle_devpts(struct inode *inode)
 {