Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[Oleg Nesterov]

On 08/11, Oleg Nesterov wrote:
>
> Hmm, bpf_uprobe_multi_link_attach() looks obviously wrong.
>
> bpf_link_prime() is called after the
>
>       for (i = 0; i < cnt; i++) {
>               uprobe_register(...);
>               ...
>       }
>
> loop. If bpf_link_prime() fails, bpf_uprobe_multi_link_attach() just do
> kvfree(uprobes) without _unregister(). In particular, this leaks the freed
> bpf_uprobe->consumer in the uprobe->consumers list.
>
> After that another _unregister() on the same uprobe can hit the problem.
>
> I guess we need a simple patch for -stable...

Something like below on top of perf/core. But I don't like the usage of
"i" in the +error_unregister path...

Oleg.

--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -3486,17 +3486,19 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr 
*attr, struct bpf_prog *pr
                                                    &uprobes[i].consumer);
                if (IS_ERR(uprobes[i].uprobe)) {
                        err = PTR_ERR(uprobes[i].uprobe);
-                       bpf_uprobe_unregister(uprobes, i);
-                       goto error_free;
+                       goto error_unregister;
                }
        }
 
        err = bpf_link_prime(&link->link, &link_primer);
        if (err)
-               goto error_free;
+               goto error_unregister;
 
        return bpf_link_settle(&link_primer);
 
+error_unregister:
+       bpf_uprobe_unregister(uprobes, i);
+
 error_free:
        kvfree(uprobes);
        kfree(link);