| Message ID | 20250110023138.659519-1-ihor.solodrai@pm.me (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [dwarves] btf_encoder: always initialize func_state to 0 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 10/01/2025 02:31, Ihor Solodrai wrote: > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges > into the master branch. > > The segfault happened at free(func_state->annots) in > btf_encoder__delete_saved_funcs(). > > func_state->annots arrived there uninitialized because after patch [2] > in some cases func_state may be allocated with a realloc, but was not > zeroed out. > > Fix this bug by always memset-ing a func_state to zero in > btf_encoder__alloc_func_state(). > > [1] https://github.com/kernel-patches/bpf/actions/runs/12700574327 > [2] https://lore.kernel.org/dwarves/20250109185950.653110-11-ihor.solodrai@pm.me/ Thanks for the quick fix! Reproduced this on an aarch64 system: BTF [M] kernel/resource_kunit.ko /bin/sh: line 1: 630875 Segmentation fault (core dumped) LLVM_OBJCOPY="objcopy" pahole -J -j --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs --lang_exclude=rust --btf_base ./vmlinux kernel/kcsan/kcsan_test.ko make[2]: *** [scripts/Makefile.modfinal:57: kernel/kcsan/kcsan_test.ko] Error 139 make[2]: *** Deleting file 'kernel/kcsan/kcsan_test.ko' make[2]: *** Waiting for unfinished jobs.... /bin/sh: line 1: 630907 Segmentation fault (core dumped) LLVM_OBJCOPY="objcopy" pahole -J -j --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs --lang_exclude=rust --btf_base ./vmlinux kernel/torture.ko make[2]: *** [scripts/Makefile.modfinal:56: kernel/torture.ko] Error 139 make[2]: *** Deleting file 'kernel/torture.ko' ...and verified that with the fix all works well. Nit: missing Signed-off-by Tested-by: Alan Maguire <alan.maguire@oracle.com> > --- > btf_encoder.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/btf_encoder.c b/btf_encoder.c > index 78efd70..511c1ea 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1, > > static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder) > { > - struct btf_encoder_func_state *tmp; > + struct btf_encoder_func_state *state, *tmp; > > if (encoder->func_states.cnt >= encoder->func_states.cap) { > > @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e > encoder->func_states.array = tmp; > } > > - return &encoder->func_states.array[encoder->func_states.cnt++]; > + state = &encoder->func_states.array[encoder->func_states.cnt++]; > + memset(state, 0, sizeof(*state)); > + > + return state; > } > > static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)
On Fri, Jan 10, 2025 at 10:39:50AM +0000, Alan Maguire wrote: > On 10/01/2025 02:31, Ihor Solodrai wrote: > > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges > > into the master branch. > > > > The segfault happened at free(func_state->annots) in > > btf_encoder__delete_saved_funcs(). > > > > func_state->annots arrived there uninitialized because after patch [2] > > in some cases func_state may be allocated with a realloc, but was not > > zeroed out. > > > > Fix this bug by always memset-ing a func_state to zero in > > btf_encoder__alloc_func_state(). > > > > [1] https://github.com/kernel-patches/bpf/actions/runs/12700574327 > > [2] https://lore.kernel.org/dwarves/20250109185950.653110-11-ihor.solodrai@pm.me/ > > > Thanks for the quick fix! Reproduced this on an aarch64 system: > > BTF [M] kernel/resource_kunit.ko > /bin/sh: line 1: 630875 Segmentation fault (core dumped) > LLVM_OBJCOPY="objcopy" pahole -J -j > --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs > --lang_exclude=rust --btf_base ./vmlinux kernel/kcsan/kcsan_test.ko > make[2]: *** [scripts/Makefile.modfinal:57: kernel/kcsan/kcsan_test.ko] > Error 139 > make[2]: *** Deleting file 'kernel/kcsan/kcsan_test.ko' > make[2]: *** Waiting for unfinished jobs.... > /bin/sh: line 1: 630907 Segmentation fault (core dumped) > LLVM_OBJCOPY="objcopy" pahole -J -j > --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs > --lang_exclude=rust --btf_base ./vmlinux kernel/torture.ko > make[2]: *** [scripts/Makefile.modfinal:56: kernel/torture.ko] Error 139 > make[2]: *** Deleting file 'kernel/torture.ko' > > ...and verified that with the fix all works well. > > Nit: missing Signed-off-by I'm adding it, ok? - Arnaldo > Tested-by: Alan Maguire <alan.maguire@oracle.com> Thanks! > > --- > > btf_encoder.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > index 78efd70..511c1ea 100644 > > --- a/btf_encoder.c > > +++ b/btf_encoder.c > > @@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1, > > > > static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder) > > { > > - struct btf_encoder_func_state *tmp; > > + struct btf_encoder_func_state *state, *tmp; > > > > if (encoder->func_states.cnt >= encoder->func_states.cap) { > > > > @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e > > encoder->func_states.array = tmp; > > } > > > > - return &encoder->func_states.array[encoder->func_states.cnt++]; > > + state = &encoder->func_states.array[encoder->func_states.cnt++]; > > + memset(state, 0, sizeof(*state)); > > + > > + return state; > > } > > > > static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)
On Fri, Jan 10, 2025 at 02:31:41AM +0000, Ihor Solodrai wrote: > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges > into the master branch. In the past the libbpf github actions was tracking the tmp.master (it would be better to track "next") branch and I was looking at when it passed to then move "next" to master, that would be great to have so that we wouldn't be having these bugs in the git history, avoiding force pushes. Anyhway, thanks for the fix, I'll add it and push it out. - Arnaldo > The segfault happened at free(func_state->annots) in > btf_encoder__delete_saved_funcs(). > > func_state->annots arrived there uninitialized because after patch [2] > in some cases func_state may be allocated with a realloc, but was not > zeroed out. > > Fix this bug by always memset-ing a func_state to zero in > btf_encoder__alloc_func_state(). > > [1] https://github.com/kernel-patches/bpf/actions/runs/12700574327 > [2] https://lore.kernel.org/dwarves/20250109185950.653110-11-ihor.solodrai@pm.me/ > --- > btf_encoder.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/btf_encoder.c b/btf_encoder.c > index 78efd70..511c1ea 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1, > > static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder) > { > - struct btf_encoder_func_state *tmp; > + struct btf_encoder_func_state *state, *tmp; > > if (encoder->func_states.cnt >= encoder->func_states.cap) { > > @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e > encoder->func_states.array = tmp; > } > > - return &encoder->func_states.array[encoder->func_states.cnt++]; > + state = &encoder->func_states.array[encoder->func_states.cnt++]; > + memset(state, 0, sizeof(*state)); > + > + return state; > } > > static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func) > -- > 2.47.1 >
On Fri, Jan 10, 2025 at 02:31:41AM +0000, Ihor Solodrai wrote: > @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e > encoder->func_states.array = tmp; > } > - return &encoder->func_states.array[encoder->func_states.cnt++]; > + state = &encoder->func_states.array[encoder->func_states.cnt++]; > + memset(state, 0, sizeof(*state)); > + > + return state; Just a super nit, the following is equivalent and shorter: state = &encoder->func_states.array[encoder->func_states.cnt++]; return memset(state, 0, sizeof(*state)); :-) But nah, I'm appling your patch as-is. Thanks! - Arnaldo
diff --git a/btf_encoder.c b/btf_encoder.c index 78efd70..511c1ea 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1, static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder) { - struct btf_encoder_func_state *tmp; + struct btf_encoder_func_state *state, *tmp; if (encoder->func_states.cnt >= encoder->func_states.cap) { @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e encoder->func_states.array = tmp; } - return &encoder->func_states.array[encoder->func_states.cnt++]; + state = &encoder->func_states.array[encoder->func_states.cnt++]; + memset(state, 0, sizeof(*state)); + + return state; } static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)