Re: [PATCH bpf-next v2 1/2] bpf: reject BPF_PROG_QUERY with short uattr size

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

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.
"