kprobes: Reduce preempt disable scope in check_kprobe_access_safe()

Commit a189d0350f387 ("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>

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index b027a4030976a2864e9e828179013f79f143be97..cb9dbdafbbcf2156ab50140beefe7e737bbd2f68 100644 (file)
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -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;