Re: [PATCH v2 5/5] uprobes: make uprobe_register() return struct uprobe *

[Andrii Nakryiko]

On Mon, Jul 29, 2024 at 6:45 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> This way uprobe_unregister() and uprobe_apply() can use "struct uprobe *"
> rather than inode + offset. This simplifies the code and allows to avoid
> the unnecessary find_uprobe() + put_uprobe() in these functions.
>
> TODO: uprobe_unregister() still needs get_uprobe/put_uprobe to ensure that
> this uprobe can't be freed before up_write(&uprobe->register_rwsem).
>
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
>  include/linux/uprobes.h     | 15 +++++-----
>  kernel/events/uprobes.c     | 56 +++++++++++++++----------------------
>  kernel/trace/bpf_trace.c    | 25 ++++++++---------
>  kernel/trace/trace_uprobe.c | 26 ++++++++---------
>  4 files changed, 55 insertions(+), 67 deletions(-)
>

You'll need something like below to not break our bpf_testmod. And
please send pull patch sets, not individually updated patches, it's a
PITA to deal with. Thanks!

commit 9f739a9997ab833394196459fa7e6dd4d13dd48b (HEAD -> uprobes-oleg-cleanups)
Author: Andrii Nakryiko <andrii@xxxxxxxxxx>
Date:   Wed Jul 31 09:15:46 2024 -0700

    uprobes: fix bpf_testmod after uprobe_register/uprobe_unregister API change

    Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>

diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 5f152afdec2f..73a6b041bcce 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -431,6 +431,7 @@ uprobe_ret_handler(struct uprobe_consumer *self,
unsigned long func,
 }

 struct testmod_uprobe {
+       struct uprobe *uprobe;
        struct path path;
        loff_t offset;
        struct uprobe_consumer consumer;
@@ -458,12 +459,14 @@ static int testmod_register_uprobe(loff_t offset)
        if (err)
                goto out;

-       err = uprobe_register(d_real_inode(uprobe.path.dentry),
-                             offset, 0, &uprobe.consumer);
-       if (err)
+       uprobe.uprobe = uprobe_register(d_real_inode(uprobe.path.dentry),
+                                       offset, 0, &uprobe.consumer);
+       if (IS_ERR(uprobe.uprobe)) {
                path_put(&uprobe.path);
-       else
+               uprobe.uprobe = NULL;
+       } else {
                uprobe.offset = offset;
+       }

 out:
        mutex_unlock(&testmod_uprobe_mutex);
@@ -474,10 +477,10 @@ static void testmod_unregister_uprobe(void)
 {
        mutex_lock(&testmod_uprobe_mutex);

-       if (uprobe.offset) {
-               uprobe_unregister(d_real_inode(uprobe.path.dentry),
-                                 uprobe.offset, &uprobe.consumer);
+       if (uprobe.uprobe) {
+               uprobe_unregister(uprobe.uprobe, &uprobe.consumer);
                uprobe.offset = 0;
+               uprobe.uprobe = NULL;
        }

        mutex_unlock(&testmod_uprobe_mutex);


[...]