rethook: Use __rcu pointer for rethook::handler

commit a1461f1fd6cfdc4b8917c9d4a91e92605d1f28dc 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: 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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 85a64cb..38a7742 100644 (file)
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -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)
 {

diff --git a/include/linux/rethook.h b/include/linux/rethook.h
index 26b6f3c..544e1bb 100644 (file)
--- a/include/linux/rethook.h
+++ b/include/linux/rethook.h
@@ -29,7 +29,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 freelist_head    pool;
        refcount_t              ref;
        struct rcu_head         rcu;

diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
index 5eb9b59..3cebcba 100644 (file)
--- a/kernel/trace/rethook.c
+++ b/kernel/trace/rethook.c
@@ -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,
                                correct_ret_addr, regs);