Commit d839a656 authored by JP Kobryn's avatar JP Kobryn Committed by Masami Hiramatsu (Google)

kprobes: consistent rcu api usage for kretprobe holder

It seems that the pointer-to-kretprobe "rp" within the kretprobe_holder is
RCU-managed, based on the (non-rethook) implementation of get_kretprobe().
The thought behind this patch is to make use of the RCU API where possible
when accessing this pointer so that the needed barriers are always in place
and to self-document the code.

The __rcu annotation to "rp" allows for sparse RCU checking. Plain writes
done to the "rp" pointer are changed to make use of the RCU macro for
assignment. For the single read, the implementation of get_kretprobe()
is simplified by making use of an RCU macro which accomplishes the same,
but note that the log warning text will be more generic.

I did find that there is a difference in assembly generated between the
usage of the RCU macros vs without. For example, on arm64, when using
rcu_assign_pointer(), the corresponding store instruction is a
store-release (STLR) which has an implicit barrier. When normal assignment
is done, a regular store (STR) is found. In the macro case, this seems to
be a result of rcu_assign_pointer() using smp_store_release() when the
value to write is not NULL.

Link: https://lore.kernel.org/all/20231122132058.3359-1-inwardvessel@gmail.com/

Fixes: d741bf41 ("kprobes: Remove kretprobe hash")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarJP Kobryn <inwardvessel@gmail.com>
Acked-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
parent d67f39d2
...@@ -139,7 +139,7 @@ static inline bool kprobe_ftrace(struct kprobe *p) ...@@ -139,7 +139,7 @@ static inline bool kprobe_ftrace(struct kprobe *p)
* *
*/ */
struct kretprobe_holder { struct kretprobe_holder {
struct kretprobe *rp; struct kretprobe __rcu *rp;
struct objpool_head pool; struct objpool_head pool;
}; };
...@@ -245,10 +245,7 @@ unsigned long kretprobe_trampoline_handler(struct pt_regs *regs, ...@@ -245,10 +245,7 @@ unsigned long kretprobe_trampoline_handler(struct pt_regs *regs,
static nokprobe_inline struct kretprobe *get_kretprobe(struct kretprobe_instance *ri) static nokprobe_inline struct kretprobe *get_kretprobe(struct kretprobe_instance *ri)
{ {
RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(), return rcu_dereference_check(ri->rph->rp, rcu_read_lock_any_held());
"Kretprobe is accessed from instance under preemptive context");
return READ_ONCE(ri->rph->rp);
} }
static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_instance *ri) static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_instance *ri)
......
...@@ -2252,7 +2252,7 @@ int register_kretprobe(struct kretprobe *rp) ...@@ -2252,7 +2252,7 @@ int register_kretprobe(struct kretprobe *rp)
rp->rph = NULL; rp->rph = NULL;
return -ENOMEM; return -ENOMEM;
} }
rp->rph->rp = rp; rcu_assign_pointer(rp->rph->rp, rp);
rp->nmissed = 0; rp->nmissed = 0;
/* Establish function entry probe point */ /* Establish function entry probe point */
ret = register_kprobe(&rp->kp); ret = register_kprobe(&rp->kp);
...@@ -2300,7 +2300,7 @@ void unregister_kretprobes(struct kretprobe **rps, int num) ...@@ -2300,7 +2300,7 @@ void unregister_kretprobes(struct kretprobe **rps, int num)
#ifdef CONFIG_KRETPROBE_ON_RETHOOK #ifdef CONFIG_KRETPROBE_ON_RETHOOK
rethook_free(rps[i]->rh); rethook_free(rps[i]->rh);
#else #else
rps[i]->rph->rp = NULL; rcu_assign_pointer(rps[i]->rph->rp, NULL);
#endif #endif
} }
mutex_unlock(&kprobe_mutex); mutex_unlock(&kprobe_mutex);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment