Thread (13 messages) flat view 13 messages, 4 authors, 2024-10-04

Re: [PATCH 1/3] uprobes: allow put_uprobe() from non-sleepable softirq context

From: Alexei Starovoitov <hidden>
Date: 2024-09-10 02:52:06
Also in: bpf, lkml

On Mon, Sep 9, 2024 at 3:49 PM Andrii Nakryiko [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Currently put_uprobe() might trigger mutex_lock()/mutex_unlock(), which
makes it unsuitable to be called from more restricted context like softirq.

Let's make put_uprobe() agnostic to the context in which it is called,
and use work queue to defer the mutex-protected clean up steps.

To avoid unnecessarily increasing the size of struct uprobe, we colocate
work_struct in parallel with rb_node and rcu, both of which are unused
by the time we get to schedule clean up work.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/events/uprobes.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index a2e6a57f79f2..377bd524bc8b 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -27,6 +27,7 @@
 #include <linux/shmem_fs.h>
 #include <linux/khugepaged.h>
 #include <linux/rcupdate_trace.h>
+#include <linux/workqueue.h>

 #include <linux/uprobes.h>
@@ -54,14 +55,20 @@ DEFINE_STATIC_PERCPU_RWSEM(dup_mmap_sem);
 #define UPROBE_COPY_INSN       0

 struct uprobe {
-       struct rb_node          rb_node;        /* node in the rb tree */
+       union {
+               struct {
+                       struct rb_node          rb_node;        /* node in the rb tree */
+                       struct rcu_head         rcu;
+               };
+               /* work is used only during freeing, rcu and rb_node are unused at that point */
+               struct work_struct work;
+       };
        refcount_t              ref;
        struct rw_semaphore     register_rwsem;
        struct rw_semaphore     consumer_rwsem;
        struct list_head        pending_list;
        struct list_head        consumers;
        struct inode            *inode;         /* Also hold a ref to inode */
-       struct rcu_head         rcu;
        loff_t                  offset;
        loff_t                  ref_ctr_offset;
        unsigned long           flags;
@@ -620,11 +627,28 @@ static inline bool uprobe_is_active(struct uprobe *uprobe)
        return !RB_EMPTY_NODE(&uprobe->rb_node);
 }

+static void uprobe_free_deferred(struct work_struct *work)
+{
+       struct uprobe *uprobe = container_of(work, struct uprobe, work);
+
+       /*
+        * If application munmap(exec_vma) before uprobe_unregister()
+        * gets called, we don't get a chance to remove uprobe from
+        * delayed_uprobe_list from remove_breakpoint(). Do it here.
+        */
+       mutex_lock(&delayed_uprobe_lock);
+       delayed_uprobe_remove(uprobe, NULL);
+       mutex_unlock(&delayed_uprobe_lock);
+
+       kfree(uprobe);
+}
+
 static void uprobe_free_rcu(struct rcu_head *rcu)
 {
        struct uprobe *uprobe = container_of(rcu, struct uprobe, rcu);

-       kfree(uprobe);
+       INIT_WORK(&uprobe->work, uprobe_free_deferred);
+       schedule_work(&uprobe->work);
 }

 static void put_uprobe(struct uprobe *uprobe)
It seems put_uprobe hunk was lost, since the patch is not doing
what commit log describes.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help