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

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

On Mon May 11, 2026 at 9:19 AM PDT, bot+bpf-ci wrote:
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index 8433430dedb7..c15a4c26a43b 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>
> [ ... ]
>
>> diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
>> index fd12ad5a0338..62fe6ed18374 100644
>> --- a/kernel/bpf/log.c
>> +++ b/kernel/bpf/log.c
>
> [ ... ]
>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 2fa05ba8f161..54b43e16848a 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>
> [ ... ]
>
>> @@ -1611,6 +1632,31 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr)
>>  	return err;
>>  }
>>
>> +static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_attr *attr_common,
>> +		      bpfptr_t uattr_common, u32 size_common)
>> +{
>> +	struct bpf_verifier_log *log;
>> +	struct bpf_log_attr attr_log;
>> +	int err, ret;
>> +
>> +	log = bpf_log_attr_create_vlog(&attr_log, attr_common, uattr_common, size_common);
>> +	if (IS_ERR(log))
>> +		return PTR_ERR(log);
>> +
>> +	err = __map_create(attr, uattr, log);
>> +
>> +	ret = bpf_log_attr_finalize(&attr_log, log);
>> +	if (ret) {
>> +		if (err >= 0)
>> +			close_fd(err);
>> +		err = ret;
>
> When bpf_log_attr_finalize() returns an error (say -ENOSPC for log buffer
> truncation or -EFAULT for copy_to_user failure) after __map_create() already
> returned a negative error (for example -EINVAL from CHECK_ATTR or -EPERM from
> security checks), does it make sense to overwrite the original map creation
> error with the log finalization error?
>
> The result is that userspace receives -ENOSPC or -EFAULT instead of the
> actual reason the map creation failed. A user with an undersized log buffer
> might retry with a larger buffer expecting success, when the real issue is
> that the map creation itself was rejected for a different reason.
>
> Looking at bpf_check() in kernel/bpf/verifier.c (around the verifier log
> finalization), it uses similar logic with 'err = ret;' reassignment. However,
> that doesn't appear to have the preserve-original-error comment that would
> clarify the intended behavior for this scenario.

I guess sinle line comment wouldn't hurt.

>
>> +	}
>> +
>> +free:
>
> Does anything use this free: label? The only exit path before it is the early
> return for IS_ERR(log), which correctly returns directly since log is an error
> pointer and doesn't need cleanup. There doesn't seem to be any 'goto free;'
> statement in the function.
>
> With -Wunused-label enabled by default in the kernel build (part of -Wall),
> this will generate a compiler warning. When building with -Werror (common in
> CI configurations and allmodconfig builds), this would break the build.

good point. pls fix.