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: 54ecbe6f1ed5 ("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>

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 64672bace56099001bc7baa398f2892e932875eb..0ff44d6633e33956c2828da6e341da5136eb3ad7 100644 (file)
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -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)
 {

diff --git a/include/linux/rethook.h b/include/linux/rethook.h
index ce69b2b7bc35861f2cd69ad65f0871c6ec9228f3..ba60962805f6d74b1f8468421b85b1d3be1f951d 100644 (file)
--- a/include/linux/rethook.h
+++ b/include/linux/rethook.h
@@ -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;
 };

diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
index 6fd7d4ecbbc67b85ff2d96a07e8f22c7e89a41b8..fa03094e9e6981b8211eb3bc050dea8678c02c1b 100644 (file)
--- a/kernel/trace/rethook.c
+++ b/kernel/trace/rethook.c
@@ -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);