| Message ID | 20220217180210.2981502-1-fallentree@fb.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b75dacaac4650478ed5a9d33975b91b99016daff |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] selftests/bpf: fix crash in core_reloc when bpftool btfgen fails | expand |
On Thu, Feb 17, 2022 at 10:02:10AM -0800, Yucong Sun wrote: > Initialize obj to null and skip closing if null. > > Signed-off-by: Yucong Sun <fallentree@fb.com> > --- > tools/testing/selftests/bpf/prog_tests/core_reloc.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c > index baf53c23c08d..7211243a22c3 100644 > --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c > +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c > @@ -861,7 +861,7 @@ static void run_core_reloc_tests(bool use_btfgen) > struct bpf_link *link = NULL; > struct bpf_map *data_map; > struct bpf_program *prog; > - struct bpf_object *obj; > + struct bpf_object *obj = NULL; > uint64_t my_pid_tgid; > struct data *data; > void *mmap_data = NULL; > @@ -992,7 +992,8 @@ static void run_core_reloc_tests(bool use_btfgen) > remove(btf_file); > bpf_link__destroy(link); > link = NULL; > - bpf_object__close(obj); > + if (obj) > + bpf_object__close(obj); Should it be: bpf_object__close(obj); obj = NULL: > } > } > > -- > 2.30.2 >
> Should it be: > bpf_object__close(obj); > obj = NULL: No, the actual crash is caused by this line: https://github.com/kernel-patches/bpf/blob/bpf-next/tools/testing/selftests/bpf/prog_tests/core_reloc.c#L896 When run_btfgen fails, the obj contains uninitialized data and then bpf_object__close(obj) crashes.
On Thu, Feb 17, 2022 at 10:55 AM sunyucong@gmail.com <sunyucong@gmail.com> wrote: > > > Should it be: > > bpf_object__close(obj); > > obj = NULL: > > No, the actual crash is caused by this line: > https://github.com/kernel-patches/bpf/blob/bpf-next/tools/testing/selftests/bpf/prog_tests/core_reloc.c#L896 > > When run_btfgen fails, the obj contains uninitialized data and then > bpf_object__close(obj) crashes. Martin's point is that you have to NULL out obj so that on the next iteration this doesn't crash again. I'll fix it up while applying.
On Thu, Feb 17, 2022 at 10:57 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Feb 17, 2022 at 10:55 AM sunyucong@gmail.com > <sunyucong@gmail.com> wrote: > > > > > Should it be: > > > bpf_object__close(obj); > > > obj = NULL: > > > > No, the actual crash is caused by this line: > > https://github.com/kernel-patches/bpf/blob/bpf-next/tools/testing/selftests/bpf/prog_tests/core_reloc.c#L896 > > > > When run_btfgen fails, the obj contains uninitialized data and then > > bpf_object__close(obj) crashes. > > > Martin's point is that you have to NULL out obj so that on the next > iteration this doesn't crash again. I'll fix it up while applying. But I actually ended up replacing two goto cleanup with continue (there is no clean up to do). And adjusted commit message to reflect that. Applied to bpf-next, thanks for the fix!
Hello: This patch was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <andrii@kernel.org>: On Thu, 17 Feb 2022 10:02:10 -0800 you wrote: > Initialize obj to null and skip closing if null. > > Signed-off-by: Yucong Sun <fallentree@fb.com> > --- > tools/testing/selftests/bpf/prog_tests/core_reloc.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Here is the summary with links: - [bpf-next] selftests/bpf: fix crash in core_reloc when bpftool btfgen fails https://git.kernel.org/bpf/bpf-next/c/b75dacaac465 You are awesome, thank you!
diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c index baf53c23c08d..7211243a22c3 100644 --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c @@ -861,7 +861,7 @@ static void run_core_reloc_tests(bool use_btfgen) struct bpf_link *link = NULL; struct bpf_map *data_map; struct bpf_program *prog; - struct bpf_object *obj; + struct bpf_object *obj = NULL; uint64_t my_pid_tgid; struct data *data; void *mmap_data = NULL; @@ -992,7 +992,8 @@ static void run_core_reloc_tests(bool use_btfgen) remove(btf_file); bpf_link__destroy(link); link = NULL; - bpf_object__close(obj); + if (obj) + bpf_object__close(obj); } }
Initialize obj to null and skip closing if null. Signed-off-by: Yucong Sun <fallentree@fb.com> --- tools/testing/selftests/bpf/prog_tests/core_reloc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)