kprobes: Reduce preempt disable scope in check_kprobe_access_safe()

Commit a189d0350f ("kprobes: disable preempt for module_text_address() and kernel_text_address()")
introduced a preempt_disable() region to protect against concurrent
module unloading. However this region also includes the call to
jump_label_text_reserved() which takes a long time;
up to 400us, iterating over approx 6000 jump tables.

The scope protected by preempt_disable() is largen than necessary.
core_kernel_text() does not need to be protected as it does not interact
with module code at all.
Only the scope from __module_text_address() to try_module_get() needs to
be protected.
By limiting the critical section to __module_text_address() and
try_module_get() the function responsible for the latency spike remains
preemptible.

This works fine even when !CONFIG_MODULES as in that case
try_module_get() will always return true and that block can be optimized
away.

Limit the critical section to __module_text_address() and
try_module_get(). Use guard(preempt)() for easier error handling.

While at it also remove a spurious *probed_mod = NULL in an error
path. On errors the output parameter is never inspected by the caller.
Some error paths were clearing the parameters, some didn't.
Align them for clarity.

Link: https://lore.kernel.org/all/20241121-kprobes-preempt-v1-1-fd581ee7fcbb@linutronix.de/

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
This commit is contained in:
Thomas Weißschuh 2024-12-08 23:26:49 +09:00 committed by Masami Hiramatsu (Google)
parent 494b332064
commit e77ab93e0e

View File

@ -39,6 +39,7 @@
#include <linux/static_call.h>
#include <linux/perf_event.h>
#include <linux/execmem.h>
#include <linux/cleanup.h>
#include <asm/sections.h>
#include <asm/cacheflush.h>
@ -1566,16 +1567,25 @@ static int check_kprobe_address_safe(struct kprobe *p,
if (ret)
return ret;
jump_label_lock();
preempt_disable();
/* Ensure the address is in a text area, and find a module if exists. */
*probed_mod = NULL;
if (!core_kernel_text((unsigned long) p->addr)) {
guard(preempt)();
*probed_mod = __module_text_address((unsigned long) p->addr);
if (!(*probed_mod)) {
ret = -EINVAL;
goto out;
}
/*
* We must hold a refcount of the probed module while updating
* its code to prohibit unexpected unloading.
*/
if (unlikely(!try_module_get(*probed_mod))) {
ret = -ENOENT;
goto out;
}
}
/* Ensure it is not in reserved area. */
if (in_gate_area_no_mm((unsigned long) p->addr) ||
@ -1584,21 +1594,13 @@ static int check_kprobe_address_safe(struct kprobe *p,
static_call_text_reserved(p->addr, p->addr) ||
find_bug((unsigned long)p->addr) ||
is_cfi_preamble_symbol((unsigned long)p->addr)) {
module_put(*probed_mod);
ret = -EINVAL;
goto out;
}
/* Get module refcount and reject __init functions for loaded modules. */
if (IS_ENABLED(CONFIG_MODULES) && *probed_mod) {
/*
* We must hold a refcount of the probed module while updating
* its code to prohibit unexpected unloading.
*/
if (unlikely(!try_module_get(*probed_mod))) {
ret = -ENOENT;
goto out;
}
/*
* If the module freed '.init.text', we couldn't insert
* kprobes in there.
@ -1606,13 +1608,11 @@ static int check_kprobe_address_safe(struct kprobe *p,
if (within_module_init((unsigned long)p->addr, *probed_mod) &&
!module_is_coming(*probed_mod)) {
module_put(*probed_mod);
*probed_mod = NULL;
ret = -ENOENT;
}
}
out:
preempt_enable();
jump_label_unlock();
return ret;