lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 10 Oct 2022 18:34:58 -0700 From: Andrii Nakryiko <andrii.nakryiko@...il.com> To: Xu Kuohai <xukuohai@...wei.com> Cc: bpf@...r.kernel.org, linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, netdev@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>, "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Jesper Dangaard Brouer <hawk@...nel.org>, Kumar Kartikeya Dwivedi <memxor@...il.com>, Alan Maguire <alan.maguire@...cle.com>, Delyan Kratunov <delyank@...com>, Lorenzo Bianconi <lorenzo@...nel.org> Subject: Re: [PATCH bpf v3 4/6] selftest/bpf: Fix memory leak in kprobe_multi_test On Mon, Oct 10, 2022 at 7:08 AM Xu Kuohai <xukuohai@...wei.com> wrote: > > The get_syms() function in kprobe_multi_test.c does not free the string > memory allocated by sscanf correctly. Fix it. > > Fixes: 5b6c7e5c4434 ("selftests/bpf: Add attach bench test") > Signed-off-by: Xu Kuohai <xukuohai@...wei.com> > Acked-by: Jiri Olsa <jolsa@...nel.org> > --- > .../bpf/prog_tests/kprobe_multi_test.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c > index d457a55ff408..07dd2c5b7f98 100644 > --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c > +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c > @@ -360,15 +360,14 @@ static int get_syms(char ***symsp, size_t *cntp) > * to them. Filter out the current culprits - arch_cpu_idle > * and rcu_* functions. > */ > - if (!strcmp(name, "arch_cpu_idle")) > - continue; > - if (!strncmp(name, "rcu_", 4)) > - continue; > - if (!strcmp(name, "bpf_dispatcher_xdp_func")) > - continue; > - if (!strncmp(name, "__ftrace_invalid_address__", > - sizeof("__ftrace_invalid_address__") - 1)) > + if (!strcmp(name, "arch_cpu_idle") || > + !strncmp(name, "rcu_", 4) || > + !strcmp(name, "bpf_dispatcher_xdp_func") || > + !strncmp(name, "__ftrace_invalid_address__", > + sizeof("__ftrace_invalid_address__") - 1)) { > + free(name); > continue; > + } it seems cleaner if we add if (name) free(name) under error: goto label. And in the success case when we assign name to syms[cnt] we can reset name to NULL to avoid double-free. WDYT? > err = hashmap__add(map, name, NULL); > if (err) { > free(name); > @@ -394,7 +393,7 @@ static int get_syms(char ***symsp, size_t *cntp) > hashmap__free(map); > if (err) { > for (i = 0; i < cnt; i++) > - free(syms[cnt]); > + free(syms[i]); > free(syms); > } > return err; > -- > 2.30.2 >
Powered by blists - more mailing lists