On 23-Mär 12:44, Kees Cook wrote:
> On Mon, Mar 23, 2020 at 05:44:13PM +0100, KP Singh wrote:
> > From: KP Singh <kpsingh@xxxxxxxxxx>
> >
> > The bpf_lsm_ nops are initialized into the LSM framework like any other
> > LSM. Some LSM hooks do not have 0 as their default return value. The
> > __weak symbol for these hooks is overridden by a corresponding
> > definition in security/bpf/hooks.c
> >
> > The LSM can be enabled / disabled with CONFIG_LSM.
> >
> > Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx>
>
> Nice! This is super clean on the LSM side of things. :)
>
> One note below...
>
> > Reviewed-by: Brendan Jackman <jackmanb@xxxxxxxxxx>
[...]
> > +
> > +/*
> > + * Copyright (C) 2020 Google LLC.
> > + */
> > +#include <linux/lsm_hooks.h>
> > +#include <linux/bpf_lsm.h>
> > +
> > +/* Some LSM hooks do not have 0 as their default return values. Override the
> > + * __weak definitons generated by default for these hooks
>
> If you wanted to avoid this, couldn't you make the default return value
> part of lsm_hooks.h?
>
> e.g.:
>
> LSM_HOOK(int, -EOPNOTSUPP, inode_getsecurity, struct inode *inode,
> const char *name, void **buffer, bool alloc)
>
> ...
>
> #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> LSM_HOOK_##RET(NAME, DEFAULT, __VA_ARGS__)
> ...
> #define LSM_HOOK_int(NAME, DEFAULT, ...) \
> noinline int bpf_lsm_##NAME(__VA_ARGS__) \
> { \
> return (DEFAULT); \
> }
>
> Then all the __weak stuff is gone, and the following 4 functions don't
> need to be written out, and the information is available to the macros
> if anyone else might ever want it.
Thanks, I like it!
If no-one objects, I will update it in the next revision.
- KP
>
> -Kees
>
> > + */
> > +noinline int bpf_lsm_inode_getsecurity(struct inode *inode, const char *name,
> > + void **buffer, bool alloc)
> > +};
[...]
> > --
> > 2.20.1
> >
>
> --
> Kees Cook
