[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bee0fd08-b9f2-83e4-2882-475b81c74303@schaufler-ca.com>
Date: Wed, 12 Feb 2020 07:52:09 -0800
From: Casey Schaufler <casey@...aufler-ca.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>,
Daniel Borkmann <daniel@...earbox.net>
Cc: Jann Horn <jannh@...gle.com>, KP Singh <kpsingh@...omium.org>,
kernel list <linux-kernel@...r.kernel.org>,
bpf <bpf@...r.kernel.org>,
linux-security-module <linux-security-module@...r.kernel.org>,
Brendan Jackman <jackmanb@...gle.com>,
Florent Revest <revest@...gle.com>,
Thomas Garnier <thgarnie@...gle.com>,
Alexei Starovoitov <ast@...nel.org>,
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>,
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>,
"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>,
Kernel Team <kernel-team@...com>,
Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: BPF LSM and fexit [was: [PATCH bpf-next v3 04/10] bpf: lsm: Add
mutable hooks list for the BPF LSM]
On 2/11/2020 6:45 PM, Alexei Starovoitov wrote:
> On Wed, Feb 12, 2020 at 01:09:07AM +0100, Daniel Borkmann wrote:
>> Another approach could be to have a special nop inside call_int_hook()
>> macro which would then get patched to avoid these situations. Somewhat
>> similar like static keys where it could be defined anywhere in text but
>> with updating of call_int_hook()'s RC for the verdict.
Tell me again why you can't register your BPF hooks like all the
other security modules do? You keep reintroducing BPF as a special
case, and I don't see why.
> Sounds nice in theory. I couldn't quite picture how that would look
> in the code, so I hacked:
> diff --git a/security/security.c b/security/security.c
> index 565bc9b67276..ce4bc1e5e26c 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -28,6 +28,7 @@
> #include <linux/string.h>
> #include <linux/msg.h>
> #include <net/flow.h>
> +#include <linux/jump_label.h>
>
> #define MAX_LSM_EVM_XATTR 2
>
> @@ -678,12 +679,26 @@ static void __init lsm_early_task(struct task_struct *task)
> * This is a hook that returns a value.
> */
>
> +#define LSM_HOOK_NAME(FUNC) \
> + DEFINE_STATIC_KEY_FALSE(bpf_lsm_key_##FUNC);
> +#include <linux/lsm_hook_names.h>
> +#undef LSM_HOOK_NAME
> +__diag_push();
> +__diag_ignore(GCC, 8, "-Wstrict-prototypes", "");
> +#define LSM_HOOK_NAME(FUNC) \
> + int bpf_lsm_call_##FUNC() {return 0;}
> +#include <linux/lsm_hook_names.h>
> +#undef LSM_HOOK_NAME
> +__diag_pop();
> +
> #define call_void_hook(FUNC, ...) \
> do { \
> struct security_hook_list *P; \
> \
> hlist_for_each_entry(P, &security_hook_heads.FUNC, list) \
> P->hook.FUNC(__VA_ARGS__); \
> + if (static_branch_unlikely(&bpf_lsm_key_##FUNC)) \
> + (void)bpf_lsm_call_##FUNC(__VA_ARGS__); \
> } while (0)
>
> #define call_int_hook(FUNC, IRC, ...) ({ \
> @@ -696,6 +711,8 @@ static void __init lsm_early_task(struct task_struct *task)
> if (RC != 0) \
> break; \
> } \
> + if (RC == IRC && static_branch_unlikely(&bpf_lsm_key_##FUNC)) \
> + RC = bpf_lsm_call_##FUNC(__VA_ARGS__); \
> } while (0); \
> RC; \
> })
>
> The assembly looks good from correctness and performance points.
> union security_list_options can be split into lsm_hook_names.h too
> to avoid __diag_ignore. Is that what you have in mind?
> I don't see how one can improve call_int_hook() macro without
> full refactoring of linux/lsm_hooks.h
> imo static_key doesn't have to be there in the first set. We can add this
> optimization later.
Powered by blists - more mailing lists