Re: [PATCH dwarves] dwarves: zero-initialize struct cu in cu__new() to prevent incorrect BTF types

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em Fri, Oct 21, 2022 at 09:35:50AM -0700, Andrii Nakryiko escreveu:
> On Fri, Oct 21, 2022 at 9:32 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
> > On Fri, Oct 21, 2022 at 04:02:03PM +0100, Alan Maguire wrote:
> > > BTF deduplication was throwing some strange results, where core kernel
> > > data types were failing to deduplicate due to the return values
> > > of function type members being void (0) instead of the actual type
> > > (unsigned int).  An example of this can be seen below, where
> > > "struct dst_ops" was failing to deduplicate between kernel and
> > > module:
> > >
> > > struct dst_ops {
> > >         short unsigned int family;
> > >         unsigned int gc_thresh;
> > >         int (*gc)(struct dst_ops *);
> > >         struct dst_entry * (*check)(struct dst_entry *, __u32);
> > >         unsigned int (*default_advmss)(const struct dst_entry *);
> > >         unsigned int (*mtu)(const struct dst_entry *);
> > > ...
> > >
> > > struct dst_ops___2 {
> > >         short unsigned int family;
> > >         unsigned int gc_thresh;
> > >         int (*gc)(struct dst_ops___2 *);
> > >         struct dst_entry___2 * (*check)(struct dst_entry___2 *, __u32);
> > >         void (*default_advmss)(const struct dst_entry___2 *);
> > >         void (*mtu)(const struct dst_entry___2 *);
> > > ...
> > >
> > > This was seen with
> > >
> > > bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> > >
> > > ...which rewrites the return value as 0 (void) when it is marked
> > > as matching DW_TAG_unspecified_type:
> > >
> > > static int32_t btf_encoder__tag_type(struct btf_encoder *encoder, uint32_t type_id_off, uint32_t tag_type)
> > > {
> > >        if (tag_type == 0)
> > >                return 0;
> > >
> > >        if (encoder->cu->unspecified_type.tag && tag_type == encoder->cu->unspecified_type.type) {
> > >                // No provision for encoding this, turn it into void.
> > >                return 0;
> > >        }
> > >
> > >        return type_id_off + tag_type;
> > > }
> > >
> > > However the odd thing was that on further examination, the unspecified type
> > > was not being set, so why was this logic being tripped?  Futher debugging
> > > showed that the encoder->cu->unspecified_type.tag value was garbage, and
> > > the type id happened to collide with "unsigned int"; as a result we
> > > were replacing unsigned ints with void return values, and since this
> > > was being done to function type members in structs, it triggered a
> > > type mismatch which failed deduplication between kernel and module.
> > >
> > > The fix is simply to calloc() the cu in cu__new() instead.
> > >
> > > Fixes: bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> > > Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> >
> > awesome, this fixes the missing dedup I was seeing
> > with current pahole:
> >
> >         $ bpftool btf dump file ./vmlinux.test | grep "STRUCT 'task_struct'" | wc -l
> >         69
> >
> > with this patch:
> >
> >         $ bpftool btf dump file ./vmlinux.test | grep "STRUCT 'task_struct'" | wc -l
> >         1
> >
> 
> Nice and a great catch! I generally try to stick to calloc() in libbpf
> exactly so I don't have to worry about stuff like this.
> 
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
 
> > Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>

Thanks, applied, my bad, I just changed it to zalloc():

⬢[acme@toolbox pahole]$ grep -A3 'zalloc(size_t' dutil.c
void *zalloc(size_t size)
{
        return calloc(1, size);
}
⬢[acme@toolbox pahole]$

That is used in many places, but unfortunately not on this specific case
:-\

- Arnaldo

⬢[acme@toolbox pahole]$ grep 'zalloc(' *.c
btf_encoder.c:	struct btf_encoder *encoder = zalloc(sizeof(*encoder));
btf_loader.c:	struct tag *tag = zalloc(size);
btf_loader.c:		struct class_member *member = zalloc(sizeof(*member));
btf_loader.c:	struct tag *tag = zalloc(sizeof(*tag));
ctf_loader.c:	struct tag *tag = zalloc(size);
ctf_loader.c:		struct class_member *member = zalloc(sizeof(*member));
ctf_loader.c:		struct class_member *member = zalloc(sizeof(*member));
ctf_loader.c:	struct tag *tag = zalloc(sizeof(*tag));
dutil.c:void *zalloc(size_t size)
dwarf_loader.c:	struct dwarf_cu *dwarf_cu = cu__zalloc(cu, sizeof(*dwarf_cu));
dwarf_loader.c:	struct dwarf_tag *dtag = cu__zalloc(dcu->cu, (sizeof(*dtag) + (spec ? sizeof(dwarf_off_ref) : 0)));
dwarf_loader.c:	struct tag *tag = cu__zalloc(dcu->cu, size);
dwarf_loader.c:		struct type *new_typedef = cu__zalloc(cu, sizeof(*new_typedef));
dwarf_loader.c:		recoded = cu__zalloc(cu, sizeof(*recoded));
dwarf_loader.c:		struct base_type *new_bt = cu__zalloc(cu, sizeof(*new_bt));
dwarf_loader.c:		struct type *new_enum = cu__zalloc(cu, sizeof(*new_enum));
dwarf_loader.c:	annot = zalloc(sizeof(*annot));
dwarf_loader.c:			dcu = zalloc(sizeof(*dcu));
dwarves.c:static void *obstack_zalloc(struct obstack *obstack, size_t size)
dwarves.c:void *cu__zalloc(struct cu *cu, size_t size)
dwarves.c:		return obstack_zalloc(&cu->obstack, size);
dwarves.c:	return zalloc(size);
dwarves.c:	struct cu *cu = zalloc(sizeof(*cu) + build_id_len);
elf_symtab.c:	struct elf_symtab *symtab = zalloc(sizeof(*symtab));
libctf.c:	struct ctf *ctf = zalloc(sizeof(*ctf));
pahole.c:	struct prototype *prototype = zalloc(sizeof(*prototype) + strlen(expression) + 1);
⬢[acme@toolbox pahole]$



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux