rethook: Use __rcu pointer for rethook::handler

commit a1461f1fd6 upstream.

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: 54ecbe6f1e ("rethook: Add a generic return hook")
Cc: stable@vger.kernel.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202311241808.rv9ceuAh-lkp@intel.com/
Tested-by: JP Kobryn <inwardvessel@gmail.com>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Masami Hiramatsu (Google) 2023-12-01 14:53:56 +09:00 committed by Greg Kroah-Hartman
parent 68bc7b2003
commit a5325a055f
3 changed files with 22 additions and 14 deletions

View File

@ -202,10 +202,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)
{

View File

@ -29,7 +29,12 @@ typedef void (*rethook_handler_t) (struct rethook_node *, void *, struct pt_regs
*/
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 *, struct pt_regs *);
struct freelist_head pool;
refcount_t ref;
struct rcu_head rcu;

View File

@ -63,7 +63,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);
}
/**
@ -78,11 +78,17 @@ 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);
}
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.
@ -102,7 +108,7 @@ struct rethook *rethook_alloc(void *data, rethook_handler_t handler)
}
rh->data = data;
rh->handler = handler;
rcu_assign_pointer(rh->handler, handler);
rh->pool.head = NULL;
refcount_set(&rh->ref, 1);
@ -142,9 +148,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))
freelist_add(&node->freelist, &node->rethook->pool);
else
call_rcu(&node->rcu, free_rethook_node_rcu);
@ -160,11 +167,9 @@ NOKPROBE_SYMBOL(rethook_recycle);
*/
struct rethook_node *rethook_try_get(struct rethook *rh)
{
rethook_handler_t handler = READ_ONCE(rh->handler);
rethook_handler_t handler = rethook_get_handler(rh);
struct freelist_node *fn;
lockdep_assert_preemption_disabled();
/* Check whether @rh is going to be freed. */
if (unlikely(!handler))
return NULL;
@ -312,7 +317,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, regs);