linux-kernel - Re: [PATCH bpf-next v5 5/7] bpf: lsm: Initialize the BPF LSM hooks

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 23 Mar 2020 20:47:59 +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>,
        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 v5 5/7] bpf: lsm: Initialize the BPF LSM hooks

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@...gle.com>
> > 
> > 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@...gle.com>
> 
> Nice! This is super clean on the LSM side of things. :)
> 
> One note below...
> 
> > Reviewed-by: Brendan Jackman <jackmanb@...gle.com>

[...]

> > +
> > +/*
> > + * 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