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: 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