On 12/2/26 06:08, Andrii Nakryiko wrote:
> On Wed, Feb 11, 2026 at 7:13 AM Leon Hwang <[email protected]> wrote:
>>
[...]
>> diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
>> index e31747b84fe2..a2b41bf5e9cb 100644
>> --- a/kernel/bpf/log.c
>> +++ b/kernel/bpf/log.c
>> @@ -864,14 +864,43 @@ void print_insn_state(struct bpf_verifier_env *env,
>> const struct bpf_verifier_st
>> print_verifier_state(env, vstate, frameno, false);
>> }
>>
>> +static bool bpf_log_attrs_set(u64 log_buf, u32 log_size, u32 log_level)
>> +{
>> + return log_buf && log_size && log_level;
>> +}
>> +
>> +static bool bpf_log_attrs_diff(struct bpf_common_attr *common, u64 log_buf,
>> u32 log_size,
>> + u32 log_level)
>> +{
>> + return bpf_log_attrs_set(log_buf, log_size, log_level) &&
>> + bpf_log_attrs_set(common->log_buf, common->log_size,
>> common->log_level) &&
>> + (log_buf != common->log_buf || log_size != common->log_size
>> ||
>> + log_level != common->log_level);
>> +}
>> +
>
> I'm not sure this check is doing what we discussed previously?... If
> log_buf is set, but log_size or log_level is zero, you'll just ignore
> log_buf here...
>
> Maybe let's keep it super simple:
>
> if (log_buf && common->log_buf && log_buf != common->log_buf)
> return -EINVAL;
> /* same for log_size, log_level, log_true_size */
>
> and then below just
>
> log->log_buf = u64_to_user_ptr(log_buf ?: common->log_buf);
> log->log_size = log_size ?: common->log_size;
>
> and so on
>
>
> We can be stricter than that, of course (as in, all triplets have to
> be completely set in either/both common_attr and attr, and they should
> completely match), but it's just more code for little benefit.
>
We cannot mix fields across the two sources. For example, using log_buf
from attr together with common->log_size when log_size is zero would mix
the configuration and make the effective log setup ambiguous.
The intent is to align strictly with the semantics enforced by
bpf_verifier_log_attr_valid():
* log_buf and log_size must be specified together.
* A non-NULL log_buf requires log_level != 0.
* All values must pass basic sanity checks.
Given that contract, we should:
1. Validate the log attributes from attr and common independently using
the same helper.
2. if both sides provide log buffers, require the tuples to match
exactly.
3. select either the attr tuple or the common tuple as a whole — never
mix fields across the two.
The patch below implements this by reusing bpf_verifier_log_attr_valid()
for both sources and resolving conflicts before selecting the effective
log configuration.
>
>> int bpf_log_attr_init(struct bpf_log_attr *log, u64 log_buf, u32 log_size,
>> u32 log_level,
>> - u32 __user *log_true_size)
>> + u32 __user *log_true_size, struct bpf_common_attr
>> *common, bpfptr_t uattr,
>> + u32 size)
>> {
>> + if (bpf_log_attrs_diff(common, log_buf, log_size, log_level))
>> + return -EINVAL;
>> +
>> memset(log, 0, sizeof(*log));
>> log->log_buf = u64_to_user_ptr(log_buf);
>> log->log_size = log_size;
>> log->log_level = log_level;
>> log->log_true_size = log_true_size;
>> +
>> + if (!log_buf && common->log_buf) {
>> + log->log_buf = u64_to_user_ptr(common->log_buf);
>> + log->log_size = common->log_size;
>> + log->log_level = common->log_level;
>> + if (size >= offsetofend(struct bpf_common_attr,
>> log_true_size))
>> + log->log_true_size = uattr.user +
>> + offsetof(struct bpf_common_attr,
>> log_true_size);
>> + else
>> + log->log_true_size = NULL;
>
> why not treat log_true_size same as log_buf/log_level/log_size? If
> both are provided, they should match, and then we don't have a
> possibility of inconsistency?
>
log_true_size is different from log_buf/log_size/log_level.
It is not a regular attribute stored in either union bpf_attr or
struct bpf_common_attr. Instead, it is a user pointer derived from
uattr.user + offset.
As a result, the computed log_true_size pointer for union bpf_attr
and for struct bpf_common_attr will always differ, because they are
based on different base user pointers (uattr.user vs
uattr_common.user).
So unlike the other log attributes, pointer equality is not a
meaningful consistency check for log_true_size. The only sensible
rule is that whichever side provides the effective log triplet also
determines the write-back destination.
Thanks,
Leon
---
Based-on commit 19de32d4cb58 ("selftests/bpf: Migrate align.c tests to
test_loader framework").
>From 32ec02c06d2abacbde17a45edbda46ef8a16fa2d Mon Sep 17 00:00:00 2001
From: Leon Hwang <[email protected]>
Date: Wed, 11 Feb 2026 23:11:11 +0800
Subject: [PATCH bpf-next v11 4/8] bpf: Add syscall common attributes support
for prog_load
BPF_PROG_LOAD can now take log parameters from both union bpf_attr and
struct bpf_common_attr. The merge rules are:
- if both sides provide a complete log tuple (buf/size/level) and they
match, use it;
- if only one side provides log parameters, use that one;
- if both sides provide complete tuples but they differ, return -EINVAL.
Signed-off-by: Leon Hwang <[email protected]>
---
include/linux/bpf_verifier.h | 3 ++-
kernel/bpf/log.c | 38 ++++++++++++++++++++++++++++--------
kernel/bpf/syscall.c | 2 +-
3 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index dbd9bdb955b3..34f28d40022a 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -643,7 +643,8 @@ struct bpf_log_attr {
};
int bpf_log_attr_init(struct bpf_log_attr *log, u64 log_buf, u32
log_size, u32 log_level,
- u32 __user *log_true_size);
+ u32 __user *log_true_size, 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 e31747b84fe2..47bf496b673e 100644
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -13,17 +13,17 @@
#define verbose(env, fmt, args...) bpf_verifier_log_write(env, fmt, ##args)
-static bool bpf_verifier_log_attr_valid(const struct bpf_verifier_log *log)
+static bool bpf_verifier_log_attr_valid(u32 log_level, char __user
*log_buf, u32 log_size)
{
/* ubuf and len_total should both be specified (or not) together */
- if (!!log->ubuf != !!log->len_total)
+ if (!!log_buf != !!log_size)
return false;
/* log buf without log_level is meaningless */
- if (log->ubuf && log->level == 0)
+ if (log_buf && log_level == 0)
return false;
- if (log->level & ~BPF_LOG_MASK)
+ if (log_level & ~BPF_LOG_MASK)
return false;
- if (log->len_total > UINT_MAX >> 2)
+ if (log_size > UINT_MAX >> 2)
return false;
return true;
}
@@ -36,7 +36,7 @@ int bpf_vlog_init(struct bpf_verifier_log *log, u32
log_level,
log->len_total = log_size;
/* log attributes have to be sane */
- if (!bpf_verifier_log_attr_valid(log))
+ if (!bpf_verifier_log_attr_valid(log_level, log_buf, log_size))
return -EINVAL;
return 0;
@@ -865,13 +865,35 @@ void print_insn_state(struct bpf_verifier_env
*env, const struct bpf_verifier_st
}
int bpf_log_attr_init(struct bpf_log_attr *log, u64 log_buf, u32
log_size, u32 log_level,
- u32 __user *log_true_size)
+ u32 __user *log_true_size, struct bpf_common_attr *common,
bpfptr_t uattr,
+ u32 size)
{
+ 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;
+
memset(log, 0, sizeof(*log));
- log->log_buf = u64_to_user_ptr(log_buf);
+ log->log_buf = ubuf;
log->log_size = log_size;
log->log_level = log_level;
log->log_true_size = log_true_size;
+
+ if (!ubuf && ubuf_common) {
+ log->log_buf = ubuf_common;
+ log->log_size = common->log_size;
+ log->log_level = common->log_level;
+ log->log_true_size = NULL;
+ if (size >= offsetofend(struct bpf_common_attr, log_true_size))
+ log->log_true_size = uattr.user +
+ offsetof(struct bpf_common_attr, log_true_size);
+ }
return 0;
}
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e86674811996..17116603ff51 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -6247,7 +6247,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t
uattr, unsigned int size,
if (from_user && size >= offsetofend(union bpf_attr,
log_true_size))
log_true_size = uattr.user + offsetof(union bpf_attr,
log_true_size);
err = bpf_log_attr_init(&attr_log, attr.log_buf, attr.log_size,
attr.log_level,
- log_true_size);
+ log_true_size, &attr_common,
uattr_common, size_common);
err = err ?: bpf_prog_load(&attr, uattr, &attr_log);
break;
case BPF_OBJ_PIN:
--
2.52.0
