Commit a1461f1f authored by Masami Hiramatsu (Google)'s avatar Masami Hiramatsu (Google)
Browse files

rethook: Use __rcu pointer for rethook::handler 

Since the rethook::handler is an RCU-maganged pointer so that it will
notice readers the rethook is stopped (unregistered) or not, it should
be an __rcu pointer and use appropriate functions to be accessed. This
will use appropriate memory barrier when accessing it. OTOH,
rethook::data is never changed, so we don't need to check it in
get_kretprobe().

NOTE: To avoid sparse warning, rethook::handler is defined by a raw
function pointer type with __rcu instead of rethook_handler_t.

Link: https://lore.kernel.org/all/170126066201.398836.837498688669005979.stgit@devnote2/



Fixes: 54ecbe6f ("rethook: Add a generic return hook")
Cc: stable@vger.kernel.org
Reported-by: default avatarkernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202311241808.rv9ceuAh-lkp@intel.com/


Tested-by: default avatarJP Kobryn <inwardvessel@gmail.com>
Signed-off-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
parent d839a656

include/linux/kprobes.h

+2 −4
Original line number Diff line number Diff line
@@ -197,10 +197,8 @@ extern int arch_trampoline_kprobe(struct kprobe *p); 
#ifdef CONFIG_KRETPROBE_ON_RETHOOK

static nokprobe_inline struct kretprobe *get_kretprobe(struct kretprobe_instance *ri)

{

	RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),

		"Kretprobe is accessed from instance under preemptive context");



	return (struct kretprobe *)READ_ONCE(ri->node.rethook->data);

	/* rethook::data is non-changed field, so that you can access it freely. */

	return (struct kretprobe *)ri->node.rethook->data;

}

static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_instance *ri)

{

include/linux/rethook.h

+6 −1
Original line number Diff line number Diff line
@@ -28,7 +28,12 @@ typedef void (*rethook_handler_t) (struct rethook_node *, void *, unsigned long, 
 */

struct rethook {

	void			*data;

	rethook_handler_t	handler;

	/*

	 * To avoid sparse warnings, this uses a raw function pointer with

	 * __rcu, instead of rethook_handler_t. But this must be same as

	 * rethook_handler_t.

	 */

	void (__rcu *handler) (struct rethook_node *, void *, unsigned long, struct pt_regs *);

	struct objpool_head	pool;

	struct rcu_head		rcu;

};

kernel/trace/rethook.c

+14 −9
Original line number Diff line number Diff line
@@ -48,7 +48,7 @@ static void rethook_free_rcu(struct rcu_head *head) 
 */

void rethook_stop(struct rethook *rh)

{

	WRITE_ONCE(rh->handler, NULL);

	rcu_assign_pointer(rh->handler, NULL);

}



/**
@@ -63,7 +63,7 @@ void rethook_stop(struct rethook *rh) 
 */

void rethook_free(struct rethook *rh)

{

	WRITE_ONCE(rh->handler, NULL);

	rethook_stop(rh);



	call_rcu(&rh->rcu, rethook_free_rcu);

}
@@ -82,6 +82,12 @@ static int rethook_fini_pool(struct objpool_head *head, void *context) 
	return 0;

}



static inline rethook_handler_t rethook_get_handler(struct rethook *rh)

{

	return (rethook_handler_t)rcu_dereference_check(rh->handler,

							rcu_read_lock_any_held());

}



/**

 * rethook_alloc() - Allocate struct rethook.

 * @data: a data to pass the @handler when hooking the return.
@@ -107,7 +113,7 @@ struct rethook *rethook_alloc(void *data, rethook_handler_t handler, 
		return ERR_PTR(-ENOMEM);



	rh->data = data;

	rh->handler = handler;

	rcu_assign_pointer(rh->handler, handler);



	/* initialize the objpool for rethook nodes */

	if (objpool_init(&rh->pool, num, size, GFP_KERNEL, rh,
@@ -135,9 +141,10 @@ static void free_rethook_node_rcu(struct rcu_head *head) 
 */

void rethook_recycle(struct rethook_node *node)

{

	lockdep_assert_preemption_disabled();

	rethook_handler_t handler;



	if (likely(READ_ONCE(node->rethook->handler)))

	handler = rethook_get_handler(node->rethook);

	if (likely(handler))

		objpool_push(node, &node->rethook->pool);

	else

		call_rcu(&node->rcu, free_rethook_node_rcu);
@@ -153,9 +160,7 @@ NOKPROBE_SYMBOL(rethook_recycle); 
 */

struct rethook_node *rethook_try_get(struct rethook *rh)

{

	rethook_handler_t handler = READ_ONCE(rh->handler);



	lockdep_assert_preemption_disabled();

	rethook_handler_t handler = rethook_get_handler(rh);



	/* Check whether @rh is going to be freed. */

	if (unlikely(!handler))
@@ -300,7 +305,7 @@ unsigned long rethook_trampoline_handler(struct pt_regs *regs, 
		rhn = container_of(first, struct rethook_node, llist);

		if (WARN_ON_ONCE(rhn->frame != frame))

			break;

		handler = READ_ONCE(rhn->rethook->handler);

		handler = rethook_get_handler(rhn->rethook);

		if (handler)

			handler(rhn, rhn->rethook->data,

				correct_ret_addr, regs);