Skip to content

Commit 40505f8

Browse files
ederevxbackslashxx
authored andcommitted
ksud: Address pagefault in ksu_handle_execveat_ksud (tiann#662)
* ksud: Address pagefault in ksu_handle_execveat_ksud As pointed out by @backslashxx, when strncpy pagefaults, it causes the first_arg to be completely NULL in some systems. This causes second_stage initialization to fail hence causing SU to be non-functional. This patch copies ksu_strncpy_from_user_retry from @backslashxx's commit: backslashxx@e2fe25e This adds a fallback to perform a normal strncpy_from_user when nofault fails which allows us to get the first_arg in such cases. Co-authored-by: backslashxx <[email protected]> Signed-off-by: Edrick Sinsuan <[email protected]> * Revert "ksud: Add second_stage init variant (tiann#653)" This reverts commit c6b60a2. --------- Signed-off-by: Edrick Sinsuan <[email protected]> Co-authored-by: backslashxx <[email protected]>
1 parent 5afce2d commit 40505f8

File tree

3 files changed

+30
-5
lines changed

3 files changed

+30
-5
lines changed

kernel/kernel_compat.c

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,3 +183,26 @@ long ksu_strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
183183
return ret;
184184
}
185185
#endif
186+
187+
static inline int ksu_access_ok(const void *addr, unsigned long size)
188+
{
189+
#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,0,0)
190+
return access_ok(addr, size);
191+
#else
192+
return access_ok(VERIFY_READ, addr, size);
193+
#endif
194+
}
195+
196+
long ksu_strncpy_from_user_retry(char *dst, const void __user *unsafe_addr,
197+
long count)
198+
{
199+
long ret = ksu_strncpy_from_user_nofault(dst, unsafe_addr, count);
200+
if (likely(ret >= 0))
201+
return ret;
202+
203+
// we faulted! fallback to slow path
204+
if (unlikely(!ksu_access_ok(unsafe_addr, count)))
205+
return -EFAULT;
206+
207+
return strncpy_from_user(dst, unsafe_addr, count);
208+
}

kernel/kernel_compat.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
extern long ksu_strncpy_from_user_nofault(char *dst,
2424
const void __user *unsafe_addr,
2525
long count);
26+
extern long ksu_strncpy_from_user_retry(char *dst,
27+
const void __user *unsafe_addr,
28+
long count);
2629

2730
#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 10, 0) || defined(CONFIG_IS_HW_HISI) || defined(CONFIG_KSU_ALLOWLIST_WORKAROUND)
2831
extern struct key *init_session_keyring;

kernel/ksud.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,12 +196,11 @@ int ksu_handle_execveat_ksud(int *fd, struct filename **filename_ptr,
196196
const char __user *p = get_user_arg_ptr(*argv, 1);
197197
if (p && !IS_ERR(p)) {
198198
char first_arg[16];
199-
ksu_strncpy_from_user_nofault(
199+
ksu_strncpy_from_user_retry(
200200
first_arg, p, sizeof(first_arg));
201201
pr_info("/system/bin/init first arg: %s\n",
202202
first_arg);
203-
if (!strcmp(first_arg, "second_stage") ||
204-
(argc == 2 && !strcmp(first_arg, ""))) {
203+
if (!strcmp(first_arg, "second_stage")) {
205204
pr_info("/system/bin/init second_stage executed\n");
206205
ksu_apply_kernelsu_rules();
207206
init_second_stage_executed = true;
@@ -222,7 +221,7 @@ int ksu_handle_execveat_ksud(int *fd, struct filename **filename_ptr,
222221
const char __user *p = get_user_arg_ptr(*argv, 1);
223222
if (p && !IS_ERR(p)) {
224223
char first_arg[16];
225-
ksu_strncpy_from_user_nofault(
224+
ksu_strncpy_from_user_retry(
226225
first_arg, p, sizeof(first_arg));
227226
pr_info("/init first arg: %s\n", first_arg);
228227
if (!strcmp(first_arg, "--second-stage")) {
@@ -247,7 +246,7 @@ int ksu_handle_execveat_ksud(int *fd, struct filename **filename_ptr,
247246
}
248247
char env[256];
249248
// Reading environment variable strings from user space
250-
if (ksu_strncpy_from_user_nofault(
249+
if (ksu_strncpy_from_user_retry(
251250
env, p, sizeof(env)) < 0)
252251
continue;
253252
// Parsing environment variable names and values

0 commit comments

Comments
 (0)