Re: [PATCH bpf-next v12 6/8] bpf: Add syscall common attributes support for map_create

[Leon Hwang]

On Mon, 20 Apr 2026 14:56:33 +0000 (UTC), bot+bpf-ci@xxxxxxxxxx wrote:
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index dbe321aa8591..34500f81db6e 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -710,6 +710,9 @@ struct bpf_log_attr {
>> int bpf_log_attr_init(struct bpf_log_attr *log, u64 log_buf, u32 log_size, u32 log_level,
>> u32 offsetof_log_true_size, bpfptr_t uattr, struct bpf_common_attr *common,
>> bpfptr_t uattr_common, u32 size_common);
>> +struct bpf_verifier_log *bpf_log_attr_create_vlog(struct bpf_log_attr *attr_log,
>> +						  struct bpf_common_attr *common, bpfptr_t uattr,
>> +						  u32 size);
>> int bpf_log_attr_finalize(struct bpf_log_attr *attr, struct bpf_verifier_log *log);
>>
>> #define BPF_MAX_SUBPROGS 256
>> diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
>> index dffe3a0180a7..5a977a52ef61 100644
>> --- a/kernel/bpf/log.c
>> +++ b/kernel/bpf/log.c
>> @@ -860,6 +860,36 @@ int bpf_log_attr_init(struct bpf_log_attr *log, u64 log_buf, u32 log_size, u32 l
>> return 0;
>> }
>>
>> +

NIT: should drop this dup blank line.

>> +struct bpf_verifier_log *bpf_log_attr_create_vlog(struct bpf_log_attr *attr_log,
>> +						  struct bpf_common_attr *common, bpfptr_t uattr,
>> +						  u32 size)
>> +{
>> +	struct bpf_verifier_log *log;
>> +	int err;
>> +
>> +	memset(attr_log, 0, sizeof(*attr_log));
>> +	attr_log->uattr = uattr;
>> +	if (size >= offsetofend(struct bpf_common_attr, log_true_size))
>> +		attr_log->offsetof_true_size = offsetof(struct bpf_common_attr, log_true_size);
>> +
>> +	if (!common->log_buf)
>> +		return NULL;
>
> Does this skip validation that bpf_log_attr_init() performs?
>
> The sibling helper bpf_log_attr_init() validates struct bpf_common_attr
> fields via bpf_verifier_log_attr_valid() unconditionally:
>
> kernel/bpf/log.c:bpf_log_attr_init() {
>     if (!bpf_verifier_log_attr_valid(common->log_level, ubuf_common, common->log_size) ||
>         !bpf_verifier_log_attr_valid(log_level, ubuf, log_size))
>         return -EINVAL;
> }
>
> This returns -EINVAL for combinations like (log_buf==0 && log_size!=0)
> or (log_buf==0 && log_level!=0), enforced by:
>
> kernel/bpf/log.c:bpf_verifier_log_attr_valid() {
>     /* ubuf and len_total should both be specified (or not) together */
>     if (!!log_buf != !!log_size)
>         return false;
>     if (log_buf && log_level == 0)
>         return false;
>     if (log_level & ~BPF_LOG_MASK)
>         return false;
>     if (log_size > UINT_MAX >> 2)
>         return false;
> }
>
> When bpf_log_attr_create_vlog() returns early because common->log_buf == 0,
> these four checks are skipped entirely. When common->log_buf != 0, the
> checks run later via bpf_vlog_init().
>
> This creates an ABI validation inconsistency: BPF_PROG_LOAD and BPF_BTF_LOAD
> reject (log_buf==0 && log_size!=0) with -EINVAL, but BPF_MAP_CREATE
> silently accepts it. Userspace passing the same invalid attribute
> combination receives different results depending on the syscall command.
>
> Should bpf_log_attr_create_vlog() call
> bpf_verifier_log_attr_valid(common->log_level, u64_to_user_ptr(common->log_buf), common->log_size)
> before the early return, and return ERR_PTR(-EINVAL) on failure?
>

It was intentional.

Like the bpf_log(log, ...) use in this patch, if log_buf is not provided,
there's nowhere to store the log msg. Thereafter, no need to alloc the log
object.

It seems cheap to alloc the 1KiB+ log object. So it would be fine to drop
this early check, and let bpf_vlog_init() validate them instead.

Thanks,
Leon

>> +
[...]