| Message ID | 20221011120108.782373-1-xukuohai@huaweicloud.com |
|---|---|
| Headers | show |
| Series | Fix bugs found by ASAN when running selftests | expand |
On 10/11/22 5:01 AM, Xu Kuohai wrote: > From: Xu Kuohai <xukuohai@huawei.com> > > This series fixes bugs found by ASAN when running bpf selftests on arm64 Overall lgtm. Acked-by: Martin KaFai Lau <martin.lau@kernel.org>
On Tue, Oct 11, 2022 at 4:43 AM Xu Kuohai <xukuohai@huaweicloud.com> wrote: > > From: Xu Kuohai <xukuohai@huawei.com> > > In the arm64 version of parse_usdt_arg(), when sscanf returns 2, reg_name > is allocated but not freed. Fix it. > > Fixes: 0f8619929c57 ("libbpf: Usdt aarch64 arg parsing support") > Signed-off-by: Xu Kuohai <xukuohai@huawei.com> > --- > tools/lib/bpf/usdt.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c > index e83b497c2245..49f3c3b7f609 100644 > --- a/tools/lib/bpf/usdt.c > +++ b/tools/lib/bpf/usdt.c > @@ -1348,25 +1348,23 @@ static int calc_pt_regs_off(const char *reg_name) > > static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg) > { > - char *reg_name = NULL; > + char reg_name[16]; > int arg_sz, len, reg_off; > long off; > > - if (sscanf(arg_str, " %d @ \[ %m[a-z0-9], %ld ] %n", &arg_sz, ®_name, &off, &len) == 3) { > + if (sscanf(arg_str, " %d @ \[ %15[a-z0-9], %ld ] %n", &arg_sz, reg_name, &off, &len) == 3) { It would be nice to do the same change for other architectures where it makes sense and avoid having to deal with unnecessary memory allocations. Please send follow up patches with similar changes for other implementations of parse_usdt_arg. Thanks. > /* Memory dereference case, e.g., -4@[sp, 96] */ > arg->arg_type = USDT_ARG_REG_DEREF; > arg->val_off = off; > reg_off = calc_pt_regs_off(reg_name); > - free(reg_name); > if (reg_off < 0) > return reg_off; > arg->reg_off = reg_off; > - } else if (sscanf(arg_str, " %d @ \[ %m[a-z0-9] ] %n", &arg_sz, ®_name, &len) == 2) { > + } else if (sscanf(arg_str, " %d @ \[ %15[a-z0-9] ] %n", &arg_sz, reg_name, &len) == 2) { > /* Memory dereference case, e.g., -4@[sp] */ > arg->arg_type = USDT_ARG_REG_DEREF; > arg->val_off = 0; > reg_off = calc_pt_regs_off(reg_name); > - free(reg_name); > if (reg_off < 0) > return reg_off; > arg->reg_off = reg_off; > @@ -1375,12 +1373,11 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec > arg->arg_type = USDT_ARG_CONST; > arg->val_off = off; > arg->reg_off = 0; > - } else if (sscanf(arg_str, " %d @ %m[a-z0-9] %n", &arg_sz, ®_name, &len) == 2) { > + } else if (sscanf(arg_str, " %d @ %15[a-z0-9] %n", &arg_sz, reg_name, &len) == 2) { > /* Register read case, e.g., -8@x4 */ > arg->arg_type = USDT_ARG_REG; > arg->val_off = 0; > reg_off = calc_pt_regs_off(reg_name); > - free(reg_name); > if (reg_off < 0) > return reg_off; > arg->reg_off = reg_off; > -- > 2.30.2 >
Hello: This series was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <andrii@kernel.org>: On Tue, 11 Oct 2022 08:01:02 -0400 you wrote: > From: Xu Kuohai <xukuohai@huawei.com> > > This series fixes bugs found by ASAN when running bpf selftests on arm64. > > v4: > - Address Andrii's suggestions > > [...] Here is the summary with links: - [bpf-next,v4,1/6] libbpf: Fix use-after-free in btf_dump_name_dups https://git.kernel.org/bpf/bpf-next/c/02c1e5b0bbb8 - [bpf-next,v4,2/6] libbpf: Fix memory leak in parse_usdt_arg() https://git.kernel.org/bpf/bpf-next/c/cd168cc6f685 - [bpf-next,v4,3/6] selftests/bpf: Fix memory leak caused by not destroying skeleton https://git.kernel.org/bpf/bpf-next/c/fbca16071678 - [bpf-next,v4,4/6] selftest/bpf: Fix memory leak in kprobe_multi_test https://git.kernel.org/bpf/bpf-next/c/159c69121102 - [bpf-next,v4,5/6] selftests/bpf: Fix error failure of case test_xdp_adjust_tail_grow https://git.kernel.org/bpf/bpf-next/c/496848b47126 - [bpf-next,v4,6/6] selftest/bpf: Fix error usage of ASSERT_OK in xdp_adjust_tail.c https://git.kernel.org/bpf/bpf-next/c/cafecc0e3df3 You are awesome, thank you!
On 10/13/2022 11:47 PM, Andrii Nakryiko wrote: > On Tue, Oct 11, 2022 at 4:43 AM Xu Kuohai <xukuohai@huaweicloud.com> wrote: >> >> From: Xu Kuohai <xukuohai@huawei.com> >> >> In the arm64 version of parse_usdt_arg(), when sscanf returns 2, reg_name >> is allocated but not freed. Fix it. >> >> Fixes: 0f8619929c57 ("libbpf: Usdt aarch64 arg parsing support") >> Signed-off-by: Xu Kuohai <xukuohai@huawei.com> >> --- >> tools/lib/bpf/usdt.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c >> index e83b497c2245..49f3c3b7f609 100644 >> --- a/tools/lib/bpf/usdt.c >> +++ b/tools/lib/bpf/usdt.c >> @@ -1348,25 +1348,23 @@ static int calc_pt_regs_off(const char *reg_name) >> >> static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg) >> { >> - char *reg_name = NULL; >> + char reg_name[16]; >> int arg_sz, len, reg_off; >> long off; >> >> - if (sscanf(arg_str, " %d @ \[ %m[a-z0-9], %ld ] %n", &arg_sz, ®_name, &off, &len) == 3) { >> + if (sscanf(arg_str, " %d @ \[ %15[a-z0-9], %ld ] %n", &arg_sz, reg_name, &off, &len) == 3) { > > It would be nice to do the same change for other architectures where > it makes sense and avoid having to deal with unnecessary memory > allocations. Please send follow up patches with similar changes for > other implementations of parse_usdt_arg. Thanks. > ok, will do > >> /* Memory dereference case, e.g., -4@[sp, 96] */ >> arg->arg_type = USDT_ARG_REG_DEREF; >> arg->val_off = off; >> reg_off = calc_pt_regs_off(reg_name); >> - free(reg_name); >> if (reg_off < 0) >> return reg_off; >> arg->reg_off = reg_off; >> - } else if (sscanf(arg_str, " %d @ \[ %m[a-z0-9] ] %n", &arg_sz, ®_name, &len) == 2) { >> + } else if (sscanf(arg_str, " %d @ \[ %15[a-z0-9] ] %n", &arg_sz, reg_name, &len) == 2) { >> /* Memory dereference case, e.g., -4@[sp] */ >> arg->arg_type = USDT_ARG_REG_DEREF; >> arg->val_off = 0; >> reg_off = calc_pt_regs_off(reg_name); >> - free(reg_name); >> if (reg_off < 0) >> return reg_off; >> arg->reg_off = reg_off; >> @@ -1375,12 +1373,11 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec >> arg->arg_type = USDT_ARG_CONST; >> arg->val_off = off; >> arg->reg_off = 0; >> - } else if (sscanf(arg_str, " %d @ %m[a-z0-9] %n", &arg_sz, ®_name, &len) == 2) { >> + } else if (sscanf(arg_str, " %d @ %15[a-z0-9] %n", &arg_sz, reg_name, &len) == 2) { >> /* Register read case, e.g., -8@x4 */ >> arg->arg_type = USDT_ARG_REG; >> arg->val_off = 0; >> reg_off = calc_pt_regs_off(reg_name); >> - free(reg_name); >> if (reg_off < 0) >> return reg_off; >> arg->reg_off = reg_off; >> -- >> 2.30.2 >> > .
From: Xu Kuohai <xukuohai@huawei.com> This series fixes bugs found by ASAN when running bpf selftests on arm64. v4: - Address Andrii's suggestions v3: https://lore.kernel.org/bpf/5311e154-c2d4-91a5-ccb8-f5adede579ed@huawei.com - Fix error failure of case test_xdp_adjust_tail_grow exposed by this series v2: https://lore.kernel.org/bpf/20221010070454.577433-1-xukuohai@huaweicloud.com - Rebase and fix conflict v1: https://lore.kernel.org/bpf/20221009131830.395569-1-xukuohai@huaweicloud.com Xu Kuohai (6): libbpf: Fix use-after-free in btf_dump_name_dups libbpf: Fix memory leak in parse_usdt_arg() selftests/bpf: Fix memory leak caused by not destroying skeleton selftest/bpf: Fix memory leak in kprobe_multi_test selftests/bpf: Fix error failure of case test_xdp_adjust_tail_grow selftest/bpf: Fix error usage of ASSERT_OK in xdp_adjust_tail.c tools/lib/bpf/btf_dump.c | 30 +++++++++++++++++-- tools/lib/bpf/usdt.c | 11 +++---- .../bpf/prog_tests/kprobe_multi_test.c | 26 ++++++++-------- .../selftests/bpf/prog_tests/map_kptr.c | 3 +- .../selftests/bpf/prog_tests/tracing_struct.c | 3 +- .../bpf/prog_tests/xdp_adjust_tail.c | 7 +++-- 6 files changed, 53 insertions(+), 27 deletions(-)