Re: BPF LSM and fexit [was: [PATCH bpf-next v3 04/10] bpf: lsm: Add mutable hooks list for the BPF LSM]
From: Alexei Starovoitov <hidden>
Date: 2020-02-12 02:45:49
Also in:
bpf, lkml
Subsystem:
security subsystem, the rest · Maintainers:
Paul Moore, James Morris, "Serge E. Hallyn", Linus Torvalds
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.
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.