On Wed, Dec 06, 2023 at 10:57:11AM +0900, mhiramat@xxxxxxxxxx wrote: > From: JP Kobryn <inwardvessel@xxxxxxxxx> > > 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@xxxxxxxxx/ > > Fixes: d741bf41d7c7 ("kprobes: Remove kretprobe hash") > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: JP Kobryn <inwardvessel@xxxxxxxxx> > Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> > (cherry picked from commit d839a656d0f3caca9f96e9bf912fd394ac6a11bc) > --- > include/linux/kprobes.h | 8 +++----- > kernel/kprobes.c | 4 ++-- > 2 files changed, 5 insertions(+), 7 deletions(-) Did you build this? It breaks the build in 6.6.y in horrible ways: ./include/linux/kprobes.h:145:33: error: field ‘pool’ has incomplete type 145 | struct objpool_head pool; | ^~~~ I'll drop this, can you please provide a working version? thanks, greg k-h
