Re: [RFC PATCH bpf-next v2 1/6] bpf: Extend bpf syscall with common attributes support

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

On 2025/9/17 08:06, Andrii Nakryiko wrote:
> On Thu, Sep 11, 2025 at 9:33 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> This patch extends the 'bpf()' syscall to support a set of common
>> attributes shared across all BPF commands:
>>
>> 1. 'log_buf': User-provided buffer for storing logs
>> 2. 'log_size': Size of the log buffer
>> 3. 'log_level': Log verbosity level
>>
>> These common attributes are passed as the 4th argument to the 'bpf()'
>> syscall, with the 5th argument specifying the size of this structure.
>>
>> To indicate the use of these common attributes from userspace, a new flag
>> 'BPF_COMMON_ATTRS' ('1 << 16') is introduced. This flag is OR-ed into the
>> 'cmd' field of the syscall.
>>
>> When 'cmd & BPF_COMMON_ATTRS' is set, the kernel will copy the common
>> attributes from userspace into kernel space for use.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>>  include/linux/syscalls.h       |  3 ++-
>>  include/uapi/linux/bpf.h       |  7 +++++++
>>  kernel/bpf/syscall.c           | 19 +++++++++++++++----
>>  tools/include/uapi/linux/bpf.h |  7 +++++++
>>  4 files changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index 77f45e5d44139..94408575dc49b 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -933,7 +933,8 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
>>  asmlinkage long sys_getrandom(char __user *buf, size_t count,
>>                               unsigned int flags);
>>  asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
>> -asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size);
>> +asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size,
>> +                       struct bpf_common_attr __user *attr_common, unsigned int size_common);
>>  asmlinkage long sys_execveat(int dfd, const char __user *filename,
>>                         const char __user *const __user *argv,
>>                         const char __user *const __user *envp, int flags);
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 233de8677382e..5014baccf065f 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -1474,6 +1474,13 @@ struct bpf_stack_build_id {
>>         };
>>  };
>>
>> +struct bpf_common_attr {
>> +       __u64 log_buf;
>> +       __u32 log_size;
>> +       __u32 log_level;
>> +};
>> +
>> +#define BPF_COMMON_ATTRS (1 << 16)
>
> add this into enum bpf_cmd after __MAX_BPF_CMD (with a small comment
> about the purpose of this)? That will keep everything cmd-related in
> one place
>

Ack.

>>  #define BPF_OBJ_NAME_LEN 16U
>>
>>  enum {
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 3f178a0f8eb12..d49f822ceea12 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -5987,8 +5987,10 @@ static int prog_stream_read(union bpf_attr *attr)
>>         return ret;
>>  }
>>
>> -static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
>> +static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
>> +                    bpfptr_t uattr_common, unsigned int size_common)
>>  {
>> +       struct bpf_common_attr common_attrs;
>>         union bpf_attr attr;
>>         int err;
>>
>> @@ -6002,6 +6004,14 @@ 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(&common_attrs, 0, sizeof(common_attrs));
>> +       if (cmd & BPF_COMMON_ATTRS) {
>> +               cmd &= ~BPF_COMMON_ATTRS;
>> +               size_common = min_t(u32, size_common, sizeof(common_attrs));
>> +               if (uattr_common.user && copy_from_bpfptr(&common_attrs, uattr_common, size_common))
>> +                       return -EFAULT;
>
> use bpf_check_uarg_tail_zero() for extra checks, just like we do for uattr
>

Ack.

Thanks,
Leon