linux-kernel - Re: [PATCH bpf-next v2 04/10] bpf: lsm: Add mutable hooks list for the BPF LSM

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: <20200116094847.GB240584@google.com>
Date:   Thu, 16 Jan 2020 10:48:47 +0100
From:   KP Singh <kpsingh@...omium.org>
To:     Stephen Smalley <sds@...ho.nsa.gov>
Cc:     KP Singh <kpsingh@...omium.org>, linux-kernel@...r.kernel.org,
        bpf@...r.kernel.org, linux-security-module@...r.kernel.org,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        James Morris <jmorris@...ei.org>,
        Kees Cook <keescook@...omium.org>,
        Thomas Garnier <thgarnie@...omium.org>,
        Michael Halcrow <mhalcrow@...gle.com>,
        Paul Turner <pjt@...gle.com>,
        Brendan Gregg <brendan.d.gregg@...il.com>,
        Jann Horn <jannh@...gle.com>,
        Matthew Garrett <mjg59@...gle.com>,
        Christian Brauner <christian@...uner.io>,
        Mickaël Salaün <mic@...ikod.net>,
        Florent Revest <revest@...omium.org>,
        Brendan Jackman <jackmanb@...omium.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Stanislav Fomichev <sdf@...gle.com>,
        Quentin Monnet <quentin.monnet@...ronome.com>,
        Andrey Ignatov <rdna@...com>, Joe Stringer <joe@...d.net.nz>
Subject: Re: [PATCH bpf-next v2 04/10] bpf: lsm: Add mutable hooks list for
 the BPF LSM

On 15-Jan 12:30, Stephen Smalley wrote:
> On 1/15/20 12:13 PM, KP Singh wrote:
> > From: KP Singh <kpsingh@...gle.com>
> > 
> > - The list of hooks registered by an LSM is currently immutable as they
> >    are declared with __lsm_ro_after_init and they are attached to a
> >    security_hook_heads struct.
> > - For the BPF LSM we need to de/register the hooks at runtime. Making
> >    the existing security_hook_heads mutable broadens an
> >    attack vector, so a separate security_hook_heads is added for only
> >    those that ~must~ be mutable.
> > - These mutable hooks are run only after all the static hooks have
> >    successfully executed.
> > 
> > This is based on the ideas discussed in:
> > 
> >    https://lore.kernel.org/lkml/20180408065916.GA2832@ircssh-2.c.rugged-nimbus-611.internal
> > 
> > Signed-off-by: KP Singh <kpsingh@...gle.com>
> > ---
> [...]
> > diff --git a/security/security.c b/security/security.c
> > index cd2d18d2d279..4a2eb4c089b2 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -652,20 +653,21 @@ static void __init lsm_early_task(struct task_struct *task)
> >   								\
> >   		hlist_for_each_entry(P, &security_hook_heads.FUNC, list) \
> >   			P->hook.FUNC(__VA_ARGS__);		\
> > +		CALL_BPF_LSM_VOID_HOOKS(FUNC, __VA_ARGS__);	\
> >   	} while (0)
> > -#define call_int_hook(FUNC, IRC, ...) ({			\
> > -	int RC = IRC;						\
> > -	do {							\
> > -		struct security_hook_list *P;			\
> > -								\
> > +#define call_int_hook(FUNC, IRC, ...) ({				\
> > +	int RC = IRC;							\
> > +	do {								\
> > +		struct security_hook_list *P;				\
> >   		hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \
> > -			RC = P->hook.FUNC(__VA_ARGS__);		\
> > -			if (RC != 0)				\
> > -				break;				\
> > -		}						\
> > -	} while (0);						\
> > -	RC;							\
> > +			RC = P->hook.FUNC(__VA_ARGS__);			\
> > +			if (RC != 0)					\
> > +				break;					\
> > +		}							\
> > +		RC = CALL_BPF_LSM_INT_HOOKS(RC, FUNC, __VA_ARGS__);	\
> 
> Let's not clobber the return code from the other LSMs with the bpf one.

Good catch and thanks for pointing it out. Should be fixed in v3.

- KP


> 
> > +	} while (0);							\
> > +	RC;								\
> >   })
> >   /* Security operations */
> > 
>