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: Tue, 11 Oct 2022 14:26:45 +0800 From: Xu Kuohai <xukuohai@...wei.com> To: Andrii Nakryiko <andrii.nakryiko@...il.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 10/11/2022 9:34 AM, Andrii Nakryiko wrote: > 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? > Fine, but since free(NULL) works perfectly, will call free(name) unconditionally, and also initialize name to NULL, and call free(name) before sscanf. > >> 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