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

[Jiri Olsa]

On Wed, Jul 31, 2024 at 09:18:00AM -0700, Andrii Nakryiko wrote:

SNIP

> 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;

ugh, I think we leak &uprobe.path.. I can send follow up fix if needed

jirka

>         }
> 
>         mutex_unlock(&testmod_uprobe_mutex);
> 
> 
> [...]