linux-kernel - Re: [PATCH bpf-next v6 3/8] bpf: lsm: provide attachment points for BPF LSM programs

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: <20200325201456.GA30568@chromium.org>
Date:   Wed, 25 Mar 2020 21:14:56 +0100
From:   KP Singh <kpsingh@...omium.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Brendan Jackman <jackmanb@...gle.com>,
        Florent Revest <revest@...gle.com>, Yonghong Song <yhs@...com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        James Morris <jmorris@...ei.org>, Paul Turner <pjt@...gle.com>,
        Jann Horn <jannh@...gle.com>,
        Florent Revest <revest@...omium.org>,
        Brendan Jackman <jackmanb@...omium.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH bpf-next v6 3/8] bpf: lsm: provide attachment points for
 BPF LSM programs

On 25-Mär 13:07, Kees Cook wrote:
> On Wed, Mar 25, 2020 at 08:39:56PM +0100, KP Singh wrote:
> > On 25-Mär 12:28, Kees Cook wrote:
> > > On Wed, Mar 25, 2020 at 04:26:24PM +0100, KP Singh wrote:
> > > > +noinline __weak RET bpf_lsm_##NAME(__VA_ARGS__)	\
> > > 
> > > I don't think the __weak is needed any more here?
> > 
> > This was suggested in:
> > 
> >  https://lore.kernel.org/bpf/20200221022537.wbmhdfkdbfvw2pww@ast-mbp/
> > 
> > "I think I saw cases when gcc ignored 'noinline' when function is
> > defined in the same file and still performed inlining while keeping
> > the function body.  To be safe I think __weak is necessary. That will
> > guarantee noinline."
> > 
> > It happened to work nicely with the previous approach for the special
> > hooks but the actual reason for adding the __weak was to guarrantee
> > that these functions don't get inlined.
> 
> Oh, hrm. Well, okay. That rationale would imply that the "noinline"
> macro needs adjustment instead, but that can be separate, something like:
> 
> include/linux/compiler_attributes.h
> 
> -#define noinline __attribute__((__noinline__))
> +#define noinline __attribute__((__noinline__)) __attribute__((__weak__))
> 
> With a comment, etc...

Sounds reasonable, I will drop the __weak from this and send a
separate patch for this.

- KP

> 
> -Kees
> 
> > 
> > > 
> > > > +{						\
> > > > +	return DEFAULT;				\
> > > 
> > > I'm impressed that LSM_RET_VOID actually works. :)
> > 
> > All the credit goes to Andrii :)
> > 
> > - KP
> > 
> > > 
> > > -Kees
> > > 
> > > > +}
> > > > +
> > > > +#include <linux/lsm_hook_defs.h>
> > > > +#undef LSM_HOOK
> > > >  
> > > >  const struct bpf_prog_ops lsm_prog_ops = {
> > > >  };
> > > > -- 
> > > > 2.20.1
> > > > 
> > > 
> > > -- 
> > > Kees Cook
> 
> -- 
> Kees Cook