On Wed, Feb 11, 2026 at 9:50 PM Leon Hwang <[email protected]> wrote:
>
>
>
> 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.
yeah, you are right, I forgot that log_true_size is not a pointer
itself, it's just a field in user-provided attrs. I'll check what you
did in v11, let's continue there.
>
> 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
>
>
