From: Eduard Zingerman <eddyz87@gmail.com>
To: Hou Tao <houtao1@huawei.com>, Paul Moses <p@1g4.org>,
Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: martin.lau@linux.dev, ast@kernel.org, daniel@iogearbox.net,
andrii@kernel.org, bpf@vger.kernel.org, song@kernel.org,
yonghong.song@linux.dev, jolsa@kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH bpf] bpf: Validate BTF repeated field counts before expansion
Date: Mon, 08 Jun 2026 23:25:44 -0700 [thread overview]
Message-ID: <670285aeb26ab8c4d4254a73a50f513bf9355246.camel@gmail.com> (raw)
In-Reply-To: <83321d7b-516c-f4a0-65ea-6fd224ba3110@huawei.com>
On Tue, 2026-06-09 at 09:30 +0800, Hou Tao wrote:
> Hi,
>
> On 6/9/2026 3:59 AM, Eduard Zingerman wrote:
> > On Sun, 2026-06-07 at 17:53 +0000, Paul Moses wrote:
> >
> > [...]
> >
> > The repro is legit.
> > Here is a somewhat minimized version as a selftest:
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> > index a9de328a8697..212ca4472a89 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> > @@ -4258,6 +4258,44 @@ static struct btf_raw_test raw_tests[] = {
> > .max_entries = 1,
> > },
> >
> > +{
> > +#define N 0x1999999aU
> > + .descr = "repeat fields overflow",
> > + .raw_types = {
> > + /* int */ /* [1] */
> > + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
> > + /* struct target {} */ /* [2] */
> > + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 0), 1),
> > + /* type_tag "kptr_untrusted" -> target */ /* [3] */
> > + BTF_TYPE_TAG_ENC(NAME_TBD, 2),
> > + /* target * (kptr) */ /* [4] */
> > + BTF_PTR_ENC(3),
> > + /* struct outer { target *kp; elem items[N]; } */ /* [5] */
> > + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 2), (N * 8u + 8u)),
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* kp */
> > + BTF_MEMBER_ENC(NAME_TBD, 6, 64), /* items */
> > + /* elem[N] */ /* [6] */
> > + BTF_TYPE_ARRAY_ENC(7, 1, N),
> > + /* struct elem { target *f0..f9; } */ /* [7] */
> > + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 10), 8),
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f0 */
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f1 */
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f2 */
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f3 */
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f4 */
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f5 */
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f6 */
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f7 */
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f8 */
> > + BTF_MEMBER_ENC(NAME_TBD, 4, 0), /* f9 */
> > + BTF_END_RAW,
> > + },
> > + BTF_STR_SEC("\0target\0kptr_untrusted\0outer\0kp\0items\0elem"
> > + "\0f0\0f1\0f2\0f3\0f4\0f5\0f6\0f7\0f8\0f9"),
> > + .btf_load_err = true,
> > +#undef N
> > +},
> > +
> > }; /* struct btf_raw_test raw_tests[] */
> >
> > static const char *get_next_str(const char *start, const char *end)
> >
> > However, as far as I understand the repro hits an overflow only
> > because `BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 10), 8)`
> > lies about `struct elem` size. It is specified as 8, while in reality it is 80.
>
> No exactly. Every field in the struct elem has the same offset (0), so
> the size of struct elem is correct, but the field definition of struct
> elem is incorrect.
Hm, right, thank you.
Ok, this means that the fix suggested by Paul should be landed
and there are no further issues with BTF validation.
(But it would be nice to have selftest included in the patchset).
> > The size of 80 would make `struct outer` unrepresentable in BTF,
> > because (N * 80 + 8) exceeds u32 range, and that's what btf_type->size uses.
> > Given that btf_repeat_fields() only traverses structs/arrays but not unions,
> > I suspect that overflow won't happen in `field_cnt * (repeat_cnt + 1)`
> > if proper size checks were implemented in btf_struct_check_meta() / btf_struct_resolve().
> > Even more, If I change "kptr_untrusted" to "kptr_untrusted11" to avoid fields parsing,
> > the kernel accepts the bogus BTF.
>
> btf_struct_check_meta has checked the validity of field offset. However
> it seems the checking is loose:
>
> /*
> * ">" instead of ">=" because the last member could be
> * "char a[0];"
> */
> if (last_offset > offset) {
> btf_verifier_log_member(env, t, member,
> "Invalid member
> bits_offset");
> return -EINVAL;
> }
>
> For struct elem, all fields have the offset 0, so the offset checking is
> passed. From the code snippet above, it seems BTF tries to support the
> following struct definition below, is it OK to only support the last
> zero-sized field in struct definition ?
>
> struct elem {
> int a;
> char b[0];
> int c;
> char d[0];
> }
I think C standard only allows zero sized arrays to be placed at the
end of the structure definition. Broader question is if we want BTF
validation to reject overlapping fields outside of unions.
Not sure if there is a practical reason to do so.
next prev parent reply other threads:[~2026-06-09 6:25 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-05 23:43 Paul Moses
2026-06-07 8:59 ` Kumar Kartikeya Dwivedi
2026-06-07 10:11 ` Paul Moses
2026-06-07 11:08 ` Kumar Kartikeya Dwivedi
2026-06-07 17:53 ` Paul Moses
2026-06-08 19:59 ` Eduard Zingerman
2026-06-09 1:30 ` Hou Tao
2026-06-09 6:25 ` Eduard Zingerman [this message]
2026-06-07 16:55 ` Alexei Starovoitov
2026-06-08 20:01 ` Eduard Zingerman
2026-06-09 8:43 ` Kumar Kartikeya Dwivedi
2026-06-10 2:13 ` Hou Tao
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=670285aeb26ab8c4d4254a73a50f513bf9355246.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=houtao1@huawei.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=p@1g4.org \
--cc=song@kernel.org \
--cc=stable@vger.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
Powered by JetHome