Commit 1703abb4 authored by Thomas Weißschuh's avatar Thomas Weißschuh Committed by Masami Hiramatsu (Google)
Browse files

kprobes: Reduce preempt disable scope in check_kprobe_access_safe() 

Commit a189d035 ("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: default avatarThomas Weißschuh <thomas.weissschuh@linutronix.de>
Reviewed-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
parent 30c8fd31

kernel/kprobes.c

+12 −12
Original line number Diff line number Diff line
@@ -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;