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

Jiri Olsa
2024-08-01T11:32:05Z

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