The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Varun R Mallya <varunrmallya@gmail.com>
To: Guangshuo Li <lgs201920130244@gmail.com>
Cc: Andrii Nakryiko <andrii@kernel.org>,
	Eduard Zingerman <eddyz87@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	Jiri Olsa <jolsa@kernel.org>,
	Emil Tsalapatis <emil@etsalapatis.com>,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] libbpf: Limit bitfield dump reads to checked bytes
Date: Mon, 6 Jul 2026 18:01:56 +0530	[thread overview]
Message-ID: <akugPNylTrOa_bNn@computer> (raw)
In-Reply-To: <20260706023632.625353-1-lgs201920130244@gmail.com>

On Mon, Jul 06, 2026 at 10:36:32AM +0800, Guangshuo Li wrote:
> btf_dump_get_bitfield_value() bounds-checks the number of bytes covered
> by the bitfield, but the value-building loops read the full backing type
> size. For narrow bitfields this can read past the checked range.
> 
> Checking the full backing type size is too strict for packed bitfields.
> For example, a packed structure can contain an 8-bit field backed by an
> int type whose BTF size is 4, while the actual object bytes covering the
> field are only 1 byte.
> 
> Keep the bounds check based on the bytes covered by the field, and make
> the value-building loops consume the same checked byte range. Preserve
> the full-size read for the non-bitfield path used by unaligned enum
> values.

This seems like it's heading in the correct direction, but to justify a change
here, could you provide examples that can break the older
implementation, if possible, with an ASAN report ? If it's not really
possible to craft something like that, then this would be an unnecessary
change.

> Fixes: 5714ca8cba5e ("libbpf: Fix OOB read in btf_dump_get_bitfield_value")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v2:
>   - Do not check the full backing type size, as it rejects valid packed
>     bitfields.
>   - Limit the value-building loops to the checked byte range instead.
>   - Preserve the non-bitfield path used for unaligned enum values.
> 
>  tools/lib/bpf/btf_dump.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index cc1ba65bb6c5..5d185f7961ff 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c
> @@ -1766,17 +1766,21 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d,
>  	__u64 num = 0;
>  	int i;
>  
> -	/* Calculate how many bytes cover the bitfield */
> -	start_bit = bits_offset % 8;
> -	nr_bytes = (start_bit + bit_sz + 7) / 8;
> +	if (bit_sz) {
> +		/* Calculate how many bytes cover the bitfield */
> +		start_bit = bits_offset % 8;
> +		nr_bytes = (start_bit + bit_sz + 7) / 8;
> +	} else {
> +		nr_bytes = t->size;
> +	}
>  
>  	/* Bound check */
>  	if (data + nr_bytes > d->typed_dump->data_end)
>  		return -E2BIG;
>  
>  	/* Maximum supported bitfield size is 64 bits */
> -	if (t->size > 8) {
> -		pr_warn("unexpected bitfield size %d\n", t->size);
> +	if (nr_bytes < 1 || nr_bytes > 8) {
> +		pr_warn("unexpected bitfield size %d\n", nr_bytes);
>  		return -EINVAL;
>  	}
>  
> @@ -1784,13 +1788,13 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d,
>  	 * stored in num, then we left/right shift num to eliminate irrelevant bits.
>  	 */
>  #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> -	for (i = t->size - 1; i >= 0; i--)
> +	for (i = nr_bytes - 1; i >= 0; i--)
>  		num = num * 256 + bytes[i];
> -	nr_copy_bits = bit_sz + bits_offset;
> +	nr_copy_bits = bit_sz ? bit_sz + bits_offset : nr_bytes * 8;
>  #elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> -	for (i = 0; i < t->size; i++)
> +	for (i = 0; i < nr_bytes; i++)
>  		num = num * 256 + bytes[i];
> -	nr_copy_bits = t->size * 8 - bits_offset;
> +	nr_copy_bits = nr_bytes * 8 - bits_offset;
>  #else
>  # error "Unrecognized __BYTE_ORDER__"
>  #endif
> -- 
> 2.43.0
> 

      reply	other threads:[~2026-07-06 12:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06  2:36 [PATCH v2] libbpf: Limit bitfield dump reads to checked bytes Guangshuo Li
2026-07-06 12:31 ` Varun R Mallya [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akugPNylTrOa_bNn@computer \
    --to=varunrmallya@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=jolsa@kernel.org \
    --cc=lgs201920130244@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox