linux-kernel - Re: [PATCH bpf-next v5 4/9] bpf: Add syscall common attributes support for prog

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzZboCfG_DTnJkdi8+VSV14fm==w4kh9zacmyqjHMtm=DQ@mail.gmail.com>
Date: Fri, 16 Jan 2026 14:29:56 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Leon Hwang <leon.hwang@...ux.dev>
Cc: bpf@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>, 
	Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Shuah Khan <shuah@...nel.org>, Christian Brauner <brauner@...nel.org>, 
	Seth Forshee <sforshee@...nel.org>, Yuichiro Tsuji <yuichtsu@...zon.com>, 
	Andrey Albershteyn <aalbersh@...hat.com>, Willem de Bruijn <willemb@...gle.com>, 
	Jason Xing <kerneljasonxing@...il.com>, Tao Chen <chen.dylane@...ux.dev>, 
	Mykyta Yatsenko <yatsenko@...a.com>, Kumar Kartikeya Dwivedi <memxor@...il.com>, 
	Anton Protopopov <a.s.protopopov@...il.com>, Amery Hung <ameryhung@...il.com>, 
	Rong Tao <rongtao@...tc.cn>, linux-kernel@...r.kernel.org, linux-api@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, kernel-patches-bot@...com
Subject: Re: [PATCH bpf-next v5 4/9] bpf: Add syscall common attributes
 support for prog_load

On Fri, Jan 16, 2026 at 6:10 AM Leon Hwang <leon.hwang@...ux.dev> wrote:
>
>
>
> On 2026/1/16 08:54, Andrii Nakryiko wrote:
> > On Mon, Jan 12, 2026 at 6:59 AM Leon Hwang <leon.hwang@...ux.dev> wrote:
> >>
> >> 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.

>
> >>
> >> Signed-off-by: Leon Hwang <leon.hwang@...ux.dev>
> >> ---
> >>  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
>
> [...]
>