On Fri, Jan 16, 2026 at 6:10 AM Leon Hwang [off-list ref] wrote:

On 2026/1/16 08:54, Andrii Nakryiko wrote:

quoted

On Mon, Jan 12, 2026 at 6:59 AM Leon Hwang [off-list ref] wrote:

quoted

The log buffer of common attributes would be confusing with the one in
'union bpf_attr' for BPF_PROG_LOAD.

In order to clarify the usage of these two log buffers, they both can be
used for logging if:

* They are same, including 'log_buf', 'log_level' and 'log_size'.
* One of them is missing, then another one will be used for logging.

If they both have 'log_buf' but they are not same totally, return -EUSERS.

why use this special error code that we don't seem to use in BPF
subsystem at all? What's wrong with -EINVAL. This shouldn't be an easy
mistake to do, tbh.

-EUSERS was suggested by Alexei.

However, I agree with you that it is better to use -EINVAL here.

I don't know what the context was, if you can find it that would be
great. Maybe special error makes sense for what Alexei had in mind.

quoted

quoted

Signed-off-by: Leon Hwang <redacted>
---
 include/linux/bpf_verifier.h |  4 +++-
 kernel/bpf/log.c             | 29 ++++++++++++++++++++++++++---
 kernel/bpf/syscall.c         |  9 ++++++---
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 4c9632c40059..da2d37ca60e7 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h

@@ -637,9 +637,11 @@ struct bpf_log_attr {
        u32 log_level;
        struct bpf_attrs *attrs;
        u32 offsetof_log_true_size;
+       struct bpf_attrs *attrs_common;
 };

-int bpf_prog_load_log_attr_init(struct bpf_log_attr *log_attr, struct bpf_attrs *attrs);
+int bpf_prog_load_log_attr_init(struct bpf_log_attr *log_attr, struct bpf_attrs *attrs,
+                               struct bpf_attrs *attrs_common);
 int bpf_log_attr_finalize(struct bpf_log_attr *log_attr, struct bpf_verifier_log *log);

 #define BPF_MAX_SUBPROGS 256

diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
index 457b724c4176..eba60a13e244 100644
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c

@@ -865,23 +865,41 @@ void print_insn_state(struct bpf_verifier_env *env, const struct bpf_verifier_st
 }

 static int bpf_log_attr_init(struct bpf_log_attr *log_attr, struct bpf_attrs *attrs, u64 log_buf,
-                            u32 log_size, u32 log_level, int offsetof_log_true_size)
+                            u32 log_size, u32 log_level, int offsetof_log_true_size,
+                            struct bpf_attrs *attrs_common)
 {
+       const struct bpf_common_attr *common_attr = attrs_common ? attrs_common->attr : NULL;
+

There is something to be said about naming choices here :) it's easy
to get lost in attrs_common being actually bpf_attrs, which contains
attr field, which is actually of bpf_common_attr type... It's a bit
disorienting. :)

I see your point about the naming being confusing.

The original intent of 'struct bpf_attrs' was to provide a shared
wrapper for both 'union bpf_attr' and 'struct bpf_common_attr'. However,
I agree that using 'attrs_common' here makes the layering harder to follow.

If that approach is undesirable, how about introducing a dedicated
structure instead, e.g.:

struct bpf_common_attrs {
        const struct bpf_common_attr *attr;
        bpfptr_t uattr;
        u32 size;
};

This should make the ownership and intent clearer.

I don't know and it's not that important, as it's pretty content. But
I'd just try to shorten some names, maybe just "common" for internal
helpers would make sense. common->log_buf, seems to work.

Thanks,
Leon

[...]