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

[bot+bpf-ci]

> 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;
>  }
>
> +
> +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?

> +
> +	log = kzalloc_obj(*log, GFP_KERNEL);
> +	if (!log)
> +		return ERR_PTR(-ENOMEM);
> +
> +	err = bpf_vlog_init(log, common->log_level, u64_to_user_ptr(common->log_buf),
> +			    common->log_size);
> +	if (err) {
> +		kfree(log);
> +		return ERR_PTR(err);
> +	}
> +
> +	return log;
> +}
> +
>  int bpf_log_attr_finalize(struct bpf_log_attr *attr, struct bpf_verifier_log *log)
>  {
>  	u32 log_true_size;

[ ... ]


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