* [PATCH bpf-next v2 0/2] bpf: Align syscall writeback behavior with user-declared size
@ 2026-05-31 0:47 Yuyang Huang
2026-05-31 0:47 ` [PATCH bpf-next v2 1/2] bpf: reject BPF_PROG_QUERY with short uattr size Yuyang Huang
2026-05-31 0:47 ` [PATCH bpf-next v2 2/2] selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries Yuyang Huang
0 siblings, 2 replies; 9+ messages in thread
From: Yuyang Huang @ 2026-05-31 0:47 UTC (permalink / raw)
To: Yuyang Huang
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Eduard Zingerman, Jiri Olsa, John Fastabend,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Shuah Khan, Song Liu,
Yonghong Song, Leon Hwang, bpf, linux-kernel, linux-kselftest
This series addresses an out-of-bounds write regression in BPF_PROG_QUERY.
Based on upstream feedback, we simplified the fix by checking the size only
in the front-gate bpf_prog_query() function and returning -EFAULT.
Changes since v1:
- Simplify the kernel fix to checking the size only in bpf_prog_query().
- Revert all other subsystem query plumbing changes.
- Update BPF selftest to target BPF_CGROUP_INET_INGRESS cgroup query, and
add verification for attr size boundaries.
Yuyang Huang (2):
bpf: reject BPF_PROG_QUERY with short uattr size
selftests/bpf: add verification for BPF_PROG_QUERY attr size
boundaries
kernel/bpf/syscall.c | 6 +-
.../selftests/bpf/prog_tests/bpf_attr_size.c | 65 +++++++++++++++++++
2 files changed, 69 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next v2 1/2] bpf: reject BPF_PROG_QUERY with short uattr size
2026-05-31 0:47 [PATCH bpf-next v2 0/2] bpf: Align syscall writeback behavior with user-declared size Yuyang Huang
@ 2026-05-31 0:47 ` Yuyang Huang
2026-05-31 0:59 ` sashiko-bot
2026-05-31 0:47 ` [PATCH bpf-next v2 2/2] selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries Yuyang Huang
1 sibling, 1 reply; 9+ messages in thread
From: Yuyang Huang @ 2026-05-31 0:47 UTC (permalink / raw)
To: Yuyang Huang
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Eduard Zingerman, Jiri Olsa, John Fastabend,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Shuah Khan, Song Liu,
Yonghong Song, Leon Hwang, bpf, linux-kernel, linux-kselftest,
Maciej Żenczykowski, Lorenzo Colitti
BPF_PROG_QUERY writes back the 'query.revision' field unconditionally to
userspace. If userspace passes a smaller 'bpf_attr' structure (e.g. 40
bytes, which was the layout before the addition of 'query.revision'),
the kernel performs an out-of-bounds write.
Fix this by returning -EFAULT in bpf_prog_query() if the user-provided
attribute size is smaller than the offset of the 'query.revision' field.
Fixes: 120933984460 ("bpf: Implement mprog API on top of existing cgroup progs")
Cc: Maciej Żenczykowski <maze@google.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Yuyang Huang <yuyanghuang@google.com>
---
kernel/bpf/syscall.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index a3c0214ca934..c9a5415ad437 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4654,8 +4654,10 @@ static int bpf_prog_detach(const union bpf_attr *attr)
#define BPF_PROG_QUERY_LAST_FIELD query.revision
static int bpf_prog_query(const union bpf_attr *attr,
- union bpf_attr __user *uattr)
+ union bpf_attr __user *uattr, u32 uattr_size)
{
+ if (uattr_size < offsetofend(union bpf_attr, query.revision))
+ return -EFAULT;
if (!bpf_net_capable())
return -EPERM;
if (CHECK_ATTR(BPF_PROG_QUERY))
@@ -6260,7 +6262,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
err = bpf_prog_detach(&attr);
break;
case BPF_PROG_QUERY:
- err = bpf_prog_query(&attr, uattr.user);
+ err = bpf_prog_query(&attr, uattr.user, size);
break;
case BPF_PROG_TEST_RUN:
err = bpf_prog_test_run(&attr, uattr.user);
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next v2 2/2] selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries
2026-05-31 0:47 [PATCH bpf-next v2 0/2] bpf: Align syscall writeback behavior with user-declared size Yuyang Huang
2026-05-31 0:47 ` [PATCH bpf-next v2 1/2] bpf: reject BPF_PROG_QUERY with short uattr size Yuyang Huang
@ 2026-05-31 0:47 ` Yuyang Huang
2026-05-31 1:11 ` sashiko-bot
2026-05-31 1:28 ` bot+bpf-ci
1 sibling, 2 replies; 9+ messages in thread
From: Yuyang Huang @ 2026-05-31 0:47 UTC (permalink / raw)
To: Yuyang Huang
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Eduard Zingerman, Jiri Olsa, John Fastabend,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Shuah Khan, Song Liu,
Yonghong Song, Leon Hwang, bpf, linux-kernel, linux-kselftest,
Maciej Żenczykowski, Lorenzo Colitti
Add a new selftest to verify that the BPF syscall (specifically
BPF_PROG_QUERY) correctly rejects queries with a user-declared size below
the mandatory minimum (which now covers query.revision) with -EFAULT, and
succeeds when given the full size.
Cc: Maciej Żenczykowski <maze@google.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Yuyang Huang <yuyanghuang@google.com>
---
.../selftests/bpf/prog_tests/bpf_attr_size.c | 65 +++++++++++++++++++
1 file changed, 65 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c b/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
new file mode 100644
index 000000000000..4fbe56cb29d4
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Google LLC */
+#include <linux/bpf.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <test_progs.h>
+#include <cgroup_helpers.h>
+#include "cgroup_skb_direct_packet_access.skel.h"
+
+#define OLD_QUERY_SIZE offsetofend(union bpf_attr, query.prog_cnt)
+#define FULL_QUERY_SIZE offsetofend(union bpf_attr, query.revision)
+
+static void test_query_size_boundaries(void)
+{
+ struct cgroup_skb_direct_packet_access *skel;
+ struct bpf_link *link = NULL;
+ union bpf_attr attr;
+ int cg_fd = -1;
+ int err;
+
+ skel = cgroup_skb_direct_packet_access__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel_load"))
+ return;
+
+ cg_fd = test__join_cgroup("/attr_size_cg");
+ if (!ASSERT_GE(cg_fd, 0, "join_cgroup"))
+ goto cleanup;
+
+ link = bpf_program__attach_cgroup(skel->progs.direct_packet_access,
+ cg_fd);
+ if (!ASSERT_OK_PTR(link, "cg_attach"))
+ goto cleanup;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.query.target_fd = cg_fd;
+ attr.query.attach_type = BPF_CGROUP_INET_INGRESS;
+
+ err = syscall(__NR_bpf, BPF_PROG_QUERY, &attr, OLD_QUERY_SIZE);
+ ASSERT_EQ(err, -1, "query_old_size_fails");
+ ASSERT_EQ(errno, EFAULT, "query_old_size_efault");
+
+ memset(&attr, 0, sizeof(attr));
+ attr.query.target_fd = cg_fd;
+ attr.query.attach_type = BPF_CGROUP_INET_INGRESS;
+
+ err = syscall(__NR_bpf, BPF_PROG_QUERY, &attr, FULL_QUERY_SIZE);
+ if (!ASSERT_OK(err, "query_full_size"))
+ goto cleanup;
+
+ ASSERT_EQ(attr.query.prog_cnt, 1, "prog_cnt_written");
+ ASSERT_GT(attr.query.revision, 0, "revision_written");
+
+cleanup:
+ if (link)
+ bpf_link__destroy(link);
+ if (cg_fd >= 0)
+ close(cg_fd);
+ cgroup_skb_direct_packet_access__destroy(skel);
+}
+
+void test_bpf_attr_size(void)
+{
+ if (test__start_subtest("query_size_boundaries"))
+ test_query_size_boundaries();
+}
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: reject BPF_PROG_QUERY with short uattr size
2026-05-31 0:47 ` [PATCH bpf-next v2 1/2] bpf: reject BPF_PROG_QUERY with short uattr size Yuyang Huang
@ 2026-05-31 0:59 ` sashiko-bot
2026-05-31 3:01 ` Alexei Starovoitov
0 siblings, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-05-31 0:59 UTC (permalink / raw)
To: Yuyang Huang; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The patch unconditionally rejects BPF_PROG_QUERY requests with a smaller attribute size, breaking backward compatibility for older userspace applications.
- [Low] Returning -EFAULT for a structure size mismatch is semantically incorrect.
--
commit 205b70f382d21dfafc18f8275384ff0f06e59f6e
Author: Yuyang Huang <yuyanghuang@google.com>
bpf: reject BPF_PROG_QUERY with short uattr size
This commit modifies bpf_prog_query() to return an error if the user-provided
attribute size is smaller than the offset of the query.revision field. This
aims to prevent an out-of-bounds write to userspace.
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 9e91fb2fb492c..4dbf53e81a38c 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4719,8 +4719,10 @@ static int bpf_prog_detach(const union bpf_attr *attr)
> #define BPF_PROG_QUERY_LAST_FIELD query.revision
>
> static int bpf_prog_query(const union bpf_attr *attr,
> - union bpf_attr __user *uattr)
> + union bpf_attr __user *uattr, u32 uattr_size)
> {
> + if (uattr_size < offsetofend(union bpf_attr, query.revision))
[Severity: Critical]
Does this unconditionally reject older userspace applications that pass a
smaller uattr_size? The BPF syscall generally maintains backward
compatibility by allowing older binaries to pass smaller structures.
By rejecting the call entirely at the top-level dispatcher, wouldn't this
break ABI for all attachment types (like BPF_FLOW_DISSECTOR), even if those
specific handlers never attempt to write to query.revision?
Could we instead pass uattr_size down to the lower-level query handlers and
conditionally skip writing the revision field to user space when the provided
buffer size is insufficient?
> + return -EFAULT;
[Severity: Low]
Is -EFAULT the correct error code for a structure size mismatch?
Standard practice usually returns -EINVAL (or -E2BIG) for mismatched or
inadequate sizes. -EFAULT typically indicates an invalid memory address
during a copy operation, which might mislead userspace developers trying to
debug their applications.
> if (!bpf_net_capable())
> return -EPERM;
> if (CHECK_ATTR(BPF_PROG_QUERY))
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260531004748.3567875-1-yuyanghuang@google.com?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries
2026-05-31 0:47 ` [PATCH bpf-next v2 2/2] selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries Yuyang Huang
@ 2026-05-31 1:11 ` sashiko-bot
2026-05-31 1:44 ` Yuyang Huang
2026-05-31 1:28 ` bot+bpf-ci
1 sibling, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-05-31 1:11 UTC (permalink / raw)
To: Yuyang Huang; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The selftest validates and enshrines an ABI-breaking kernel behavior where BPF_PROG_QUERY rejects valid queries from older user-space binaries with -EFAULT.
--
commit a51bd156081fb623564c01426a789df272b5e924
Author: Yuyang Huang <yuyanghuang@google.com>
selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries
This commit adds a selftest to verify that BPF_PROG_QUERY rejects queries
with a user-declared size below the size covering query.revision with
-EFAULT. It also ensures the query succeeds when given the full size.
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c b/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
> new file mode 100644
> index 0000000000000..4fbe56cb29d4a
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Google LLC */
> +#include <linux/bpf.h>
[ ... ]
> +#define OLD_QUERY_SIZE offsetofend(union bpf_attr, query.prog_cnt)
> +#define FULL_QUERY_SIZE offsetofend(union bpf_attr, query.revision)
> +
> +static void test_query_size_boundaries(void)
> +{
[ ... ]
> + memset(&attr, 0, sizeof(attr));
> + attr.query.target_fd = cg_fd;
> + attr.query.attach_type = BPF_CGROUP_INET_INGRESS;
> +
> + err = syscall(__NR_bpf, BPF_PROG_QUERY, &attr, OLD_QUERY_SIZE);
> + ASSERT_EQ(err, -1, "query_old_size_fails");
> + ASSERT_EQ(errno, EFAULT, "query_old_size_efault");
[Severity: Critical]
Does this selftest validate an ABI-breaking kernel behavior where
BPF_PROG_QUERY rejects valid queries from older user-space binaries with
-EFAULT?
Older user-space applications (like an older bpftool) compiled with older
headers will supply a smaller bpf_attr size up to query.prog_cnt.
Previously, the kernel would accept this size, but this test now enforces
that the kernel rejects these historically valid queries unconditionally.
Should the kernel conditionally copy to query.revision only if the
user-provided size is large enough, rather than breaking backwards
compatibility for existing binaries?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260531004748.3567875-1-yuyanghuang@google.com?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries
2026-05-31 0:47 ` [PATCH bpf-next v2 2/2] selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries Yuyang Huang
2026-05-31 1:11 ` sashiko-bot
@ 2026-05-31 1:28 ` bot+bpf-ci
1 sibling, 0 replies; 9+ messages in thread
From: bot+bpf-ci @ 2026-05-31 1:28 UTC (permalink / raw)
To: yuyanghuang, yuyanghuang
Cc: ast, andrii, daniel, eddyz87, jolsa, john.fastabend, memxor,
martin.lau, shuah, song, yonghong.song, leon.hwang, bpf,
linux-kernel, linux-kselftest, maze, lorenzo, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c b/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
> new file mode 100644
> index 000000000000..4fbe56cb29d4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
> @@ -0,0 +1,65 @@
[ ... ]
> + err = syscall(__NR_bpf, BPF_PROG_QUERY, &attr, OLD_QUERY_SIZE);
> + ASSERT_EQ(err, -1, "query_old_size_fails");
> + ASSERT_EQ(errno, EFAULT, "query_old_size_efault");
This test asserts errno == EFAULT for a short query size. A v2 review
comment on the companion kernel patch (bpf: reject BPF_PROG_QUERY with
short uattr size) questioned whether -EFAULT is the right error code for
a structure size mismatch, noting that -EINVAL or -E2BIG is the more
conventional return code and that -EFAULT usually indicates an invalid
memory address during a copy.
That comment was not answered on the v2 thread. If the kernel patch's
error code changes to -EINVAL or -E2BIG, would this assertion need to
change too?
https://lore.kernel.org/all/20260531004748.3567875-2-yuyanghuang@google.com/
[ ... ]
This selftest verifies the fix for an out-of-bounds write to userspace in
BPF_PROG_QUERY, where __cgroup_bpf_query() unconditionally wrote
uattr->query.revision even when userspace passed a smaller bpf_attr. The
companion kernel fix carries a Fixes: tag, but this selftest commit does
not.
Should this include:
Fixes: 120933984460 ("bpf: Implement mprog API on top of existing cgroup progs")
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26699503418
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries
2026-05-31 1:11 ` sashiko-bot
@ 2026-05-31 1:44 ` Yuyang Huang
0 siblings, 0 replies; 9+ messages in thread
From: Yuyang Huang @ 2026-05-31 1:44 UTC (permalink / raw)
To: sashiko-reviews; +Cc: bpf
On Sun, May 31, 2026 at 9:11 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] The selftest validates and enshrines an ABI-breaking kernel behavior where BPF_PROG_QUERY rejects valid queries from older user-space binaries with -EFAULT.
> --
>
> commit a51bd156081fb623564c01426a789df272b5e924
> Author: Yuyang Huang <yuyanghuang@google.com>
>
> selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries
>
> This commit adds a selftest to verify that BPF_PROG_QUERY rejects queries
> with a user-declared size below the size covering query.revision with
> -EFAULT. It also ensures the query succeeds when given the full size.
>
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c b/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
> > new file mode 100644
> > index 0000000000000..4fbe56cb29d4a
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_attr_size.c
> > @@ -0,0 +1,65 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2026 Google LLC */
> > +#include <linux/bpf.h>
>
> [ ... ]
>
> > +#define OLD_QUERY_SIZE offsetofend(union bpf_attr, query.prog_cnt)
> > +#define FULL_QUERY_SIZE offsetofend(union bpf_attr, query.revision)
> > +
> > +static void test_query_size_boundaries(void)
> > +{
>
> [ ... ]
>
> > + memset(&attr, 0, sizeof(attr));
> > + attr.query.target_fd = cg_fd;
> > + attr.query.attach_type = BPF_CGROUP_INET_INGRESS;
> > +
> > + err = syscall(__NR_bpf, BPF_PROG_QUERY, &attr, OLD_QUERY_SIZE);
> > + ASSERT_EQ(err, -1, "query_old_size_fails");
> > + ASSERT_EQ(errno, EFAULT, "query_old_size_efault");
>
> [Severity: Critical]
> Does this selftest validate an ABI-breaking kernel behavior where
> BPF_PROG_QUERY rejects valid queries from older user-space binaries with
> -EFAULT?
>
> Older user-space applications (like an older bpftool) compiled with older
> headers will supply a smaller bpf_attr size up to query.prog_cnt.
> Previously, the kernel would accept this size, but this test now enforces
> that the kernel rejects these historically valid queries unconditionally.
>
> Should the kernel conditionally copy to query.revision only if the
> user-provided size is large enough, rather than breaking backwards
> compatibility for existing binaries?
>
The proposal in the patch is implemented based on the suggestion from
the previous review thread:
https://lore.kernel.org/bpf/DIUR88219HWQ.26T4D7ZX8CNTC@gmail.com/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: reject BPF_PROG_QUERY with short uattr size
2026-05-31 0:59 ` sashiko-bot
@ 2026-05-31 3:01 ` Alexei Starovoitov
2026-05-31 3:12 ` Yuyang Huang
0 siblings, 1 reply; 9+ messages in thread
From: Alexei Starovoitov @ 2026-05-31 3:01 UTC (permalink / raw)
To: sashiko-reviews, Yuyang Huang; +Cc: bpf
On Sat May 30, 2026 at 5:59 PM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] The patch unconditionally rejects BPF_PROG_QUERY requests with a smaller attribute size, breaking backward compatibility for older userspace applications.
> - [Low] Returning -EFAULT for a structure size mismatch is semantically incorrect.
> --
>
> commit 205b70f382d21dfafc18f8275384ff0f06e59f6e
> Author: Yuyang Huang <yuyanghuang@google.com>
>
> bpf: reject BPF_PROG_QUERY with short uattr size
>
> This commit modifies bpf_prog_query() to return an error if the user-provided
> attribute size is smaller than the offset of the query.revision field. This
> aims to prevent an out-of-bounds write to userspace.
>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 9e91fb2fb492c..4dbf53e81a38c 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -4719,8 +4719,10 @@ static int bpf_prog_detach(const union bpf_attr *attr)
>> #define BPF_PROG_QUERY_LAST_FIELD query.revision
>>
>> static int bpf_prog_query(const union bpf_attr *attr,
>> - union bpf_attr __user *uattr)
>> + union bpf_attr __user *uattr, u32 uattr_size)
>> {
>> + if (uattr_size < offsetofend(union bpf_attr, query.revision))
>
> [Severity: Critical]
> Does this unconditionally reject older userspace applications that pass a
> smaller uattr_size? The BPF syscall generally maintains backward
> compatibility by allowing older binaries to pass smaller structures.
>
> By rejecting the call entirely at the top-level dispatcher, wouldn't this
> break ABI for all attachment types (like BPF_FLOW_DISSECTOR), even if those
> specific handlers never attempt to write to query.revision?
bot is correct. this is too broad.
> Could we instead pass uattr_size down to the lower-level query handlers and
> conditionally skip writing the revision field to user space when the provided
> buffer size is insufficient?
ok. let's propagate uattr_size all the way to __cgroup_bpf_query() and do:
if (uattr_size >= offsetofend(union bpf_attr, query.revision) &&
copy_to_user(&uattr->query.revision, &revision, sizeof(revision)))
return -EFAULT;
With single fixes tag:
Fixes: 120933984460 ("bpf: Implement mprog API on top of existing cgroup progs")
and explain in commit log like:
"
query.revision in bpf_mprog_query is structurally identical to the cgroup case: a late tail field, written unconditionally.
But the backward-compat hazard is not the same
The min-historical-size test is per command, and bpf_mprog_query only serves attach types that were born with revision in the struct:
- tcx_prog_query → BPF_TCX_INGRESS/EGRESS
- netkit_prog_query → BPF_NETKIT_PRIMARY/PEER
tcx, netkit, the revision field, and bpf_mprog_query itself all landed in the same v6.6 merge window (053c8e1f235d added the mprog query API + revision; tcx in
e420bed02507, netkit in 35dfaad7188c). There has never been a tcx/netkit BPF_PROG_QUERY userspace that doesn't know about revision. So for these commands the
minimum legitimate struct already covers offset 56–64 — no old binary can be broken here.
Contrast with cgroup: BPF_PROG_QUERY on cgroup attach types shipped in 2017; revision write-back was bolted on years later (120933984460). That path has a real
population of pre-revision callers.
"
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: reject BPF_PROG_QUERY with short uattr size
2026-05-31 3:01 ` Alexei Starovoitov
@ 2026-05-31 3:12 ` Yuyang Huang
0 siblings, 0 replies; 9+ messages in thread
From: Yuyang Huang @ 2026-05-31 3:12 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: sashiko-reviews, bpf
On Sun, May 31, 2026 at 11:02 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat May 30, 2026 at 5:59 PM PDT, sashiko-bot wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [Critical] The patch unconditionally rejects BPF_PROG_QUERY requests with a smaller attribute size, breaking backward compatibility for older userspace applications.
> > - [Low] Returning -EFAULT for a structure size mismatch is semantically incorrect.
> > --
> >
> > commit 205b70f382d21dfafc18f8275384ff0f06e59f6e
> > Author: Yuyang Huang <yuyanghuang@google.com>
> >
> > bpf: reject BPF_PROG_QUERY with short uattr size
> >
> > This commit modifies bpf_prog_query() to return an error if the user-provided
> > attribute size is smaller than the offset of the query.revision field. This
> > aims to prevent an out-of-bounds write to userspace.
> >
> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> >> index 9e91fb2fb492c..4dbf53e81a38c 100644
> >> --- a/kernel/bpf/syscall.c
> >> +++ b/kernel/bpf/syscall.c
> >> @@ -4719,8 +4719,10 @@ static int bpf_prog_detach(const union bpf_attr *attr)
> >> #define BPF_PROG_QUERY_LAST_FIELD query.revision
> >>
> >> static int bpf_prog_query(const union bpf_attr *attr,
> >> - union bpf_attr __user *uattr)
> >> + union bpf_attr __user *uattr, u32 uattr_size)
> >> {
> >> + if (uattr_size < offsetofend(union bpf_attr, query.revision))
> >
> > [Severity: Critical]
> > Does this unconditionally reject older userspace applications that pass a
> > smaller uattr_size? The BPF syscall generally maintains backward
> > compatibility by allowing older binaries to pass smaller structures.
> >
> > By rejecting the call entirely at the top-level dispatcher, wouldn't this
> > break ABI for all attachment types (like BPF_FLOW_DISSECTOR), even if those
> > specific handlers never attempt to write to query.revision?
>
> bot is correct. this is too broad.
>
> > Could we instead pass uattr_size down to the lower-level query handlers and
> > conditionally skip writing the revision field to user space when the provided
> > buffer size is insufficient?
>
> ok. let's propagate uattr_size all the way to __cgroup_bpf_query() and do:
>
> if (uattr_size >= offsetofend(union bpf_attr, query.revision) &&
> copy_to_user(&uattr->query.revision, &revision, sizeof(revision)))
> return -EFAULT;
>
> With single fixes tag:
> Fixes: 120933984460 ("bpf: Implement mprog API on top of existing cgroup progs")
>
> and explain in commit log like:
> "
> query.revision in bpf_mprog_query is structurally identical to the cgroup case: a late tail field, written unconditionally.
>
> But the backward-compat hazard is not the same
>
> The min-historical-size test is per command, and bpf_mprog_query only serves attach types that were born with revision in the struct:
>
> - tcx_prog_query → BPF_TCX_INGRESS/EGRESS
> - netkit_prog_query → BPF_NETKIT_PRIMARY/PEER
>
> tcx, netkit, the revision field, and bpf_mprog_query itself all landed in the same v6.6 merge window (053c8e1f235d added the mprog query API + revision; tcx in
> e420bed02507, netkit in 35dfaad7188c). There has never been a tcx/netkit BPF_PROG_QUERY userspace that doesn't know about revision. So for these commands the
> minimum legitimate struct already covers offset 56–64 — no old binary can be broken here.
>
> Contrast with cgroup: BPF_PROG_QUERY on cgroup attach types shipped in 2017; revision write-back was bolted on years later (120933984460). That path has a real
> population of pre-revision callers.
> "
Thanks for the detailed review again. I will send patch v3 based on
these suggestions.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-05-31 3:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-31 0:47 [PATCH bpf-next v2 0/2] bpf: Align syscall writeback behavior with user-declared size Yuyang Huang
2026-05-31 0:47 ` [PATCH bpf-next v2 1/2] bpf: reject BPF_PROG_QUERY with short uattr size Yuyang Huang
2026-05-31 0:59 ` sashiko-bot
2026-05-31 3:01 ` Alexei Starovoitov
2026-05-31 3:12 ` Yuyang Huang
2026-05-31 0:47 ` [PATCH bpf-next v2 2/2] selftests/bpf: add verification for BPF_PROG_QUERY attr size boundaries Yuyang Huang
2026-05-31 1:11 ` sashiko-bot
2026-05-31 1:44 ` Yuyang Huang
2026-05-31 1:28 ` bot+bpf-ci
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox