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
>
prev parent reply other threads:[~2026-07-06 12:32 UTC|newest]
Thread overview: 3+ 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 2:46 ` sashiko-bot
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.