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]

 



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


Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>

thanks,
jirka


> ---
>  dwarves.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/dwarves.c b/dwarves.c
> index fbebc1d..424381d 100644
> --- a/dwarves.c
> +++ b/dwarves.c
> @@ -626,7 +626,7 @@ struct cu *cu__new(const char *name, uint8_t addr_size,
>  		   const unsigned char *build_id, int build_id_len,
>  		   const char *filename, bool use_obstack)
>  {
> -	struct cu *cu = malloc(sizeof(*cu) + build_id_len);
> +	struct cu *cu = calloc(1, sizeof(*cu) + build_id_len);
>  
>  	if (cu != NULL) {
>  		uint32_t void_id;
> -- 
> 2.31.1
> 



[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