Re: [PATCH bpf-next v3 2/3] bpf: Check tail zero of bpf_prog_info

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3949 bytes --]

> commit acd0d2042870d7613c0c27932c41262d1c41b4e1
> Author: Leon Hwang <leon.hwang@linux.dev>
> Commit: Kernel Patches Daemon <kernel-patches-bot@fb.com>
>
>     bpf: Check tail zero of bpf_prog_info
>
>     Since there're 4 bytes padding at the end of struct bpf_prog_info, they
>     won't be checked by bpf_check_uarg_tail_zero().
>
>     pahole -C bpf_prog_info ./vmlinux
>     struct bpf_prog_info {
>             ...
>             __u32                      attach_btf_obj_id;    /*   220     4 */
>             __u32                      attach_btf_id;        /*   224     4 */
>
>             /* size: 232, cachelines: 4, members: 38 */
>             /* sum members: 224 */
>             /* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 31 bits */
>             /* padding: 4 */
>             /* forced alignments: 9 */
>             /* last cacheline: 40 bytes */
>     } __attribute__((__aligned__(8)));
>
>     If a future kernel extension adds a new 4-byte field, older userspace
>     programs allocating this structure on the stack might inadvertently pass
>     uninitialized stack garbage into the new field, permanently breaking
>     backward compatibility. -- sashiko [1]
>
>     Fix it by changing sizeof(info) to
>     offsetofend(struct bpf_prog_info, attach_btf_id).
>
>     And, add "__u32 :32" to the tail of struct bpf_prog_info.
>
>     [1] https://lore.kernel.org/bpf/20260513224823.6494FC19425@smtp.kernel.org/
>
>     Fixes: aba64c7da983 ("bpf: Add verified_insns to bpf_prog_info and fdinfo")

Is the Fixes: tag pointing to the right commit? The padding bug was
actually introduced by commit b79c9fc9551b ("bpf: implement BPF_PROG_QUERY
for BPF_LSM_CGROUP"), which added attach_btf_obj_id and attach_btf_id after
verified_insns, creating the 4 bytes of padding at the end of the structure.
Commit aba64c7da983 added verified_insns as the last field but did not
create padding.

Suggested: Fixes: b79c9fc9551b ("bpf: implement BPF_PROG_QUERY for
BPF_LSM_CGROUP")

>     Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
>     Signed-off-by: Leon Hwang <leon.hwang@linux.dev>

[ ... ]

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 89f020a44fc9..c5d4ae957e87 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -5121,10 +5121,11 @@ static int bpf_prog_get_info_by_fd(struct file *file,
>  	u32 info_len = attr->info.info_len;
>  	struct bpf_prog_kstats stats;
>  	char __user *uinsns;
> -	u32 ulen;
> +	u32 ulen, len;
>  	int err;
>
> -	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), sizeof(info), info_len);
> +	len = offsetofend(struct bpf_prog_info, attach_btf_id);
> +	err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), len, info_len);
>  	if (err)
>  		return err;
>  	info_len = min_t(u32, sizeof(info), info_len);

Does this change break the ABI for existing deployed binaries?

Previously, expected_size was 232 (sizeof(info)). If an existing binary
passes info_len = 232 with uninitialized stack garbage in the 4-byte
padding, bpf_check_uarg_tail_zero() would see actual_size <= expected_size
(232 <= 232) and immediately return 0.

By reducing expected_size to 228, actual_size > expected_size (232 > 228),
so bpf_check_uarg_tail_zero() will actively check the 4 padding bytes and
return -E2BIG if they contain garbage. This rejects previously working
applications.

sashiko-bot and bot+bpf-ci raised this concern in v1 and v2:
https://lore.kernel.org/bpf/20260603151405.6DB111F00893@smtp.kernel.org/

While v3 adds '__u32 :32;' to prevent future field additions in the
padding space (addressing forward compatibility), the backward
compatibility concern about breaking existing binaries remains unresolved.

[ ... ]


---
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/27025811732