Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Arnaldo Carvalho de Melo <acme@kernel.org>]

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)