* Re: [PATCH bpf-next v13 4/8] bpf: Add syscall common attributes support for prog_load
[not found] ` <20260512221819.7EA2FC2BCB0@smtp.kernel.org>
@ 2026-05-13 10:44 ` Leon Hwang
0 siblings, 0 replies; only message in thread
From: Leon Hwang @ 2026-05-13 10:44 UTC (permalink / raw)
To: sashiko; +Cc: bpf
On 13/5/26 06:18, sashiko-bot@kernel.org wrote:
[...]
>> 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)
>> + u32 offsetof_log_true_size, bpfptr_t uattr, struct bpf_common_attr *common,
>> + bpfptr_t uattr_common, u32 size_common)
>> {
>> + char __user *ubuf_common = u64_to_user_ptr(common->log_buf);
>> char __user *ubuf = u64_to_user_ptr(log_buf);
>>
>> + 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;
>> +
>> + if (ubuf && ubuf_common && (ubuf != ubuf_common || log_size != common->log_size ||
>> + log_level != common->log_level))
>> + return -EINVAL;
>
> If a user specifies log_level != 0 but leaves log_buf == NULL to probe for
> log_true_size, will these checks bypass conflicts?
>
> If one side provides a complete tuple and the other provides only log_level,
> or if both provide mismatched log_levels without buffers, it appears the
> conflict checks are skipped and no -EINVAL is returned.
>
Correct.
However, if users want to probe for log_true_size, they should not
provide common attrs. It is compatible for bpf libraries to probe for
log_true_size using union bpf_attr without providing common attrs.
>> +
>> memset(log, 0, sizeof(*log));
>> log->ubuf = ubuf;
>> log->size = log_size;
>> log->level = log_level;
>> log->offsetof_true_size = offsetof_log_true_size;
>> log->uattr = uattr;
>> +
>> + if (!ubuf && ubuf_common) {
>
> Does this condition silently ignore valid common->log_level configurations?
>
> If the user provides only common->log_level via bpf_common_attr for a log
> size dry-run, ubuf_common evaluates to NULL. The condition here would
> evaluate to false, and common->log_level would be ignored instead of
> applied.
>
The user should probe for log size using union bpf_attr instead of
common attrs.
> Also, if both specific and common attributes provide identical log
> parameters, the code appears to unconditionally use uattr for the
> log_true_size writeback since ubuf is not NULL.
>
> If the original uattr uses an older layout without space for log_true_size,
> log->offsetof_true_size will be set to 0. Does this mean log_true_size
> writeback will be lost even if the user provided a modern attr_common
> struct with sufficient space to receive it?
>
This was intentional.
If the user uses union bpf_attr to receive log, he should provide the
space for log_true_size.
IOW, if the user uses common attrs to receive log, he should provide the
space in common attrs for log_true_size.
Thanks,
Leon
>> + log->ubuf = ubuf_common;
>> + log->size = common->log_size;
>> + log->level = common->log_level;
>> + log->uattr = uattr_common;
>> + log->offsetof_true_size = 0;
>> + if (size_common >= offsetofend(struct bpf_common_attr, log_true_size))
>> + log->offsetof_true_size = offsetof(struct bpf_common_attr, log_true_size);
>> + }
>> return 0;
>> }
>
^ permalink raw reply [flat|nested] only message in thread