Skip to content

Commit

Permalink
kernel: remove unused CONFIG guard becuase GKI kernel enable kprobe b…
Browse files Browse the repository at this point in the history 
…y default
  • Loading branch information
@tiann
tiann committed Feb 16, 2025
1 parent 4593ae8 commit 500ff9b
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 60 deletions.
2 changes: 0 additions & 2 deletions kernel/core_hook.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,9 +862,7 @@ void __init ksu_core_init(void)

void ksu_core_exit(void)
{
#ifdef CONFIG_KPROBES
pr_info("ksu_core_kprobe_exit\n");
// we dont use this now
// ksu_kprobe_exit();
#endif
}
6 changes: 0 additions & 6 deletions kernel/ksu.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,8 @@ int __init kernelsu_init(void)

ksu_throne_tracker_init();

#ifdef CONFIG_KPROBES
ksu_sucompat_init();
ksu_ksud_init();
#else
pr_alert("KPROBES is disabled, KernelSU may not work, please check https://kernelsu.org/guide/how-to-integrate-for-non-gki.html");
#endif

#ifdef MODULE
#ifndef CONFIG_KSU_DEBUG
Expand All @@ -80,10 +76,8 @@ void kernelsu_exit(void)

destroy_workqueue(ksu_workqueue);

#ifdef CONFIG_KPROBES
ksu_ksud_exit();
ksu_sucompat_exit();
#endif

ksu_core_exit();
}
Expand Down
44 changes: 0 additions & 44 deletions kernel/ksud.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,9 @@ static void stop_vfs_read_hook();
static void stop_execve_hook();
static void stop_input_hook();

#ifdef CONFIG_KPROBES
static struct work_struct stop_vfs_read_work;
static struct work_struct stop_execve_hook_work;
static struct work_struct stop_input_hook_work;
#else
bool ksu_vfs_read_hook __read_mostly = true;
bool ksu_execveat_hook __read_mostly = true;
bool ksu_input_hook __read_mostly = true;
#endif

u32 ksu_devpts_sid;

Expand Down Expand Up @@ -150,11 +144,6 @@ int ksu_handle_execveat_ksud(int *fd, struct filename **filename_ptr,
struct user_arg_ptr *argv,
struct user_arg_ptr *envp, int *flags)
{
#ifndef CONFIG_KPROBES
if (!ksu_execveat_hook) {
return 0;
}
#endif
struct filename *filename;

static const char app_process[] = "/system/bin/app_process";
Expand Down Expand Up @@ -306,11 +295,6 @@ static ssize_t read_iter_proxy(struct kiocb *iocb, struct iov_iter *to)
int ksu_handle_vfs_read(struct file **file_ptr, char __user **buf_ptr,
size_t *count_ptr, loff_t **pos)
{
#ifndef CONFIG_KPROBES
if (!ksu_vfs_read_hook) {
return 0;
}
#endif
struct file *file;
char __user *buf;
size_t count;
Expand Down Expand Up @@ -419,11 +403,6 @@ static bool is_volumedown_enough(unsigned int count)
int ksu_handle_input_handle_event(unsigned int *type, unsigned int *code,
int *value)
{
#ifndef CONFIG_KPROBES
if (!ksu_input_hook) {
return 0;
}
#endif
if (*type == EV_KEY && *code == KEY_VOLUMEDOWN) {
int val = *value;
pr_info("KEY_VOLUMEDOWN val: %d\n", val);
Expand Down Expand Up @@ -461,8 +440,6 @@ bool ksu_is_safe_mode()
return false;
}

#ifdef CONFIG_KPROBES

static int sys_execve_handler_pre(struct kprobe *p, struct pt_regs *regs)
{
struct pt_regs *real_regs = PT_REAL_REGS(regs);
Expand Down Expand Up @@ -515,7 +492,6 @@ static struct kprobe vfs_read_kp = {
.pre_handler = sys_read_handler_pre,
};


static struct kprobe input_event_kp = {
.symbol_name = "input_event",
.pre_handler = input_handle_event_handler_pre,
Expand All @@ -535,28 +511,17 @@ static void do_stop_input_hook(struct work_struct *work)
{
unregister_kprobe(&input_event_kp);
}
#endif

static void stop_vfs_read_hook()
{
#ifdef CONFIG_KPROBES
bool ret = schedule_work(&stop_vfs_read_work);
pr_info("unregister vfs_read kprobe: %d!\n", ret);
#else
ksu_vfs_read_hook = false;
pr_info("stop vfs_read_hook\n");
#endif
}

static void stop_execve_hook()
{
#ifdef CONFIG_KPROBES
bool ret = schedule_work(&stop_execve_hook_work);
pr_info("unregister execve kprobe: %d!\n", ret);
#else
ksu_execveat_hook = false;
pr_info("stop execve_hook\n");
#endif
}

static void stop_input_hook()
Expand All @@ -566,19 +531,13 @@ static void stop_input_hook()
return;
}
input_hook_stopped = true;
#ifdef CONFIG_KPROBES
bool ret = schedule_work(&stop_input_hook_work);
pr_info("unregister input kprobe: %d!\n", ret);
#else
ksu_input_hook = false;
pr_info("stop input_hook\n");
#endif
}

// ksud: module support
void ksu_ksud_init()
{
#ifdef CONFIG_KPROBES
int ret;

ret = register_kprobe(&execve_kp);
Expand All @@ -593,15 +552,12 @@ void ksu_ksud_init()
INIT_WORK(&stop_vfs_read_work, do_stop_vfs_read_hook);
INIT_WORK(&stop_execve_hook_work, do_stop_execve_hook);
INIT_WORK(&stop_input_hook_work, do_stop_input_hook);
#endif
}

void ksu_ksud_exit()
{
#ifdef CONFIG_KPROBES
unregister_kprobe(&execve_kp);
// this should be done before unregister vfs_read_kp
// unregister_kprobe(&vfs_read_kp);
unregister_kprobe(&input_event_kp);
#endif
}
8 changes: 0 additions & 8 deletions kernel/sucompat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,6 @@ int ksu_handle_devpts(struct inode *inode)
return 0;
}

#ifdef CONFIG_KPROBES

static int faccessat_handler_pre(struct kprobe *p, struct pt_regs *regs)
{
struct pt_regs *real_regs = PT_REAL_REGS(regs);
Expand Down Expand Up @@ -232,8 +230,6 @@ static int pts_unix98_lookup_pre(struct kprobe *p, struct pt_regs *regs)
return ksu_handle_devpts(inode);
}

#endif

static struct kprobe *init_kprobe(const char *name,
kprobe_pre_handler_t handler)
{
Expand Down Expand Up @@ -269,19 +265,15 @@ static struct kprobe *su_kps[4];
// sucompat: permited process can execute 'su' to gain root access.
void ksu_sucompat_init()
{
#ifdef CONFIG_KPROBES
su_kps[0] = init_kprobe(SYS_EXECVE_SYMBOL, execve_handler_pre);
su_kps[1] = init_kprobe(SYS_FACCESSAT_SYMBOL, faccessat_handler_pre);
su_kps[2] = init_kprobe(SYS_NEWFSTATAT_SYMBOL, newfstatat_handler_pre);
su_kps[3] = init_kprobe("pts_unix98_lookup", pts_unix98_lookup_pre);
#endif
}

void ksu_sucompat_exit()
{
#ifdef CONFIG_KPROBES
for (int i = 0; i < ARRAY_SIZE(su_kps); i++) {
destroy_kprobe(&su_kps[i]);
}
#endif
}

5 comments on commit 500ff9b

@0ctobot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KPROBES is enabled by default on GKI, but that doesn't mean all GKI kernels will have KPROBES. We're dealing with a kernel-based root solution here, it stands to reason that custom kernels will be involved, and some of them will have KPROBES disabled.

The guarding isn't doing any harm by existing, and covers edge cases such as monolithic builds of GKI kernels.

@TogoFire
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it's understandable to disregard this, it's nonetheless a serious issue. I'm surprised the official repository would implement something like this. Enabling Kprobes by default is a security risk. Kprobes is generally discouraged due to security vulnerabilities, performance degradation, and its problematic nature.

@tiann
Copy link
Owner Author

@tiann tiann commented on 500ff9b Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you believe kprobe is problematic, discuss it with Google and Qualcomm. Every device with an official GKI kernel has kprobe enabled. Blaming and complaining are unproductive compared to submitting a PR.

@backslashxx
Copy link
Contributor

@backslashxx backslashxx commented on 500ff9b Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0ctobot @TogoFire

actually kernelsu kprobe hooks are better, when I tried ridding off kprobe use on kernelsu,
I noticed that it hooks in a way less intrusive way than original manual hooks from the guide.

original manual hooks on the guide is so fucking intrusive and obsolete man.
nobody really got it up to what the kernelsu driver code can do.
it might be an interest to both of you

backslashxx#5

@0ctobot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0ctobot @TogoFire

actually kernelsu kprobe hooks are better, when I tried ridding off kprobe use on kernelsu, I noticed that it hooks in a way less intrusive way than original manual hooks from the guide.

original manual hooks on the guide is so fucking intrusive and obsolete man. nobody really got it up to what the kernelsu driver code can do. it might be an interest to both of you

backslashxx#5

It's not really a matter of what I think is better, or what I prefer, it's what I have to do as I maintain a monolithic 6.1 kernel which, while technically GKI, no longer functions as one and has no kernel module support and thus I couldn't use KPROBES even if I wanted to.

I get it, at that point, for all intents and purposes I'm non-GKI so fuck me. However, this is not the only scenario that one could reasonably envision KPROBES being disabled on a GKI kernel. So I just brought it up because I think it's a bit silly to operate under the assumption the KPROBES will always be enabled on GKI kernels. There is no drawback to having the guarding I'm place, and it covers edge cases.

I didn't anticipate that this was going to be received positively, but I thought I would mention it nonetheless.

Please sign in to comment.