| Message ID | 20221025080628.523300-1-nashuiliang@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Clean up all resources when register_fprobe_ips returns an error | expand |
On Tue, Oct 25, 2022 at 04:06:28PM +0800, Chuang Wang wrote: > When register_fprobe_ips returns an error, bpf_link_cleanup just cleans > up bpf_link_primer's resources and forgets to clean up > bpf_kprobe_multi_link, addrs, cookies. > > So, by adding 'goto error', this ensures that all resources are cleaned > up. > > Cc: stable@vger.kernel.org > Fixes: 0dcac2725406 ("bpf: Add multi kprobe link") > Signed-off-by: Chuang Wang <nashuiliang@gmail.com> > --- > kernel/trace/bpf_trace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 1ed08967fb97..5b806ef20857 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -2778,7 +2778,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > err = register_fprobe_ips(&link->fp, addrs, cnt); > if (err) { > bpf_link_cleanup(&link_primer); > - return err; > + goto error; that should be taken care of bpf_kprobe_multi_link_dealloc, through the fput path in bpf_link_cleanup do you see any related memory leak like report in kmemleak? jirka > } > > return bpf_link_settle(&link_primer); > -- > 2.34.1 >
Sorry, I didn't notice this. On Tue, Oct 25, 2022 at 5:01 PM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Tue, Oct 25, 2022 at 04:06:28PM +0800, Chuang Wang wrote: > > When register_fprobe_ips returns an error, bpf_link_cleanup just cleans > > up bpf_link_primer's resources and forgets to clean up > > bpf_kprobe_multi_link, addrs, cookies. > > > > So, by adding 'goto error', this ensures that all resources are cleaned > > up. > > > > Cc: stable@vger.kernel.org > > Fixes: 0dcac2725406 ("bpf: Add multi kprobe link") > > Signed-off-by: Chuang Wang <nashuiliang@gmail.com> > > --- > > kernel/trace/bpf_trace.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index 1ed08967fb97..5b806ef20857 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -2778,7 +2778,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > > err = register_fprobe_ips(&link->fp, addrs, cnt); > > if (err) { > > bpf_link_cleanup(&link_primer); > > - return err; > > + goto error; > > that should be taken care of bpf_kprobe_multi_link_dealloc, > through the fput path in bpf_link_cleanup > > do you see any related memory leak like report in kmemleak? > > jirka > > > } > > > > return bpf_link_settle(&link_primer); > > -- > > 2.34.1 > >
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 1ed08967fb97..5b806ef20857 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2778,7 +2778,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr err = register_fprobe_ips(&link->fp, addrs, cnt); if (err) { bpf_link_cleanup(&link_primer); - return err; + goto error; } return bpf_link_settle(&link_primer);
When register_fprobe_ips returns an error, bpf_link_cleanup just cleans up bpf_link_primer's resources and forgets to clean up bpf_kprobe_multi_link, addrs, cookies. So, by adding 'goto error', this ensures that all resources are cleaned up. Cc: stable@vger.kernel.org Fixes: 0dcac2725406 ("bpf: Add multi kprobe link") Signed-off-by: Chuang Wang <nashuiliang@gmail.com> --- kernel/trace/bpf_trace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)