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

[Kees Cook]

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...

-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