[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240811123504.GB30068@redhat.com>
Date: Sun, 11 Aug 2024 14:35:04 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: syzbot <syzbot+f7a1c2c2711e4a780f19@...kaller.appspotmail.com>,
Andrii Nakryiko <andrii@...nel.org>, jolsa@...nel.org
Cc: acme@...nel.org, adrian.hunter@...el.com,
alexander.shishkin@...ux.intel.com, irogers@...gle.com,
kan.liang@...ux.intel.com, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, mark.rutland@....com,
mhiramat@...nel.org, mingo@...hat.com, namhyung@...nel.org,
peterz@...radead.org, syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in
__uprobe_unregister
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);
Powered by blists - more mailing lists