Re: [PATCH bpf-next v14 1/8] bpf: Extend BPF syscall with common attributes support

[Leon Hwang <leon.hwang@linux.dev>]

On 2026/5/14 06:48, sashiko-bot@kernel.org wrote:
[...]
>>  
>> +struct bpf_common_attr {
>> +	__aligned_u64 log_buf;
>> +	__u32 log_size;
>> +	__u32 log_level;
>> +	__u32 log_true_size;
>> +};
> 
> Does this UAPI structure contain implicit trailing padding? With one
> __aligned_u64 (8 bytes) and three __u32 fields (12 bytes), there are 20
> bytes of data. Because of the 8-byte alignment, the compiler will pad the
> structure to 24 bytes.
> 
> 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.
> 
> Should this include an explicit __u32 pad; field, and should the kernel
> explicitly validate that attr_common.pad == 0?
> 

This concern seems valid to me.

Propose my fix here:

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6600e126fbfb..83de8fb9b9aa 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -6278,7 +6278,9 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t
uattr, unsigned int size,

        memset(&attr_common, 0, sizeof(attr_common));
        if (cmd & BPF_COMMON_ATTRS) {
-               err = bpf_check_uarg_tail_zero(uattr_common,
sizeof(attr_common), size_common);
+               err = bpf_check_uarg_tail_zero(uattr_common,
+                                              offsetofend(struct
bpf_common_attr, log_true_size),
+                                              size_common);
                if (err)
                        return err;

This is to avoid adding an explicit __u32 pad in struct bpf_common_attr.

> [ ... ]
> 
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
> 
> [ ... ]
> 
>> @@ -6226,6 +6228,20 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
>>  	if (copy_from_bpfptr(&attr, uattr, size) != 0)
>>  		return -EFAULT;
>>  
>> +	memset(&attr_common, 0, sizeof(attr_common));
>> +	if (cmd & BPF_COMMON_ATTRS) {
>> +		err = bpf_check_uarg_tail_zero(uattr_common, sizeof(attr_common), size_common);
>> +		if (err)
>> +			return err;
>> +
>> +		cmd &= ~BPF_COMMON_ATTRS;
> 
> By unconditionally stripping BPF_COMMON_ATTRS from the command here, won't any
> BPF command (such as BPF_MAP_FREEZE) successfully accept the flag but silently
> ignore the provided common attributes?
> 

Correct.

> This might prevent userspace from correctly probing whether the running kernel
> actually supports common attributes for a given command.
> 
> Should __sys_bpf() enforce a whitelist of supported commands before stripping
> the flag, or explicitly reject the flag for commands that do not support it?
> 

If users want to probe for common attributes feature, they can do it by
the way of patch #2.

Thanks,
Leon

[...]