lkml.org 
[lkml]   [2020]   [Feb]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: BPF LSM and fexit [was: [PATCH bpf-next v3 04/10] bpf: lsm: Add mutable hooks list for the BPF LSM]
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.

\
 
 \ /
  Last update: 2020-02-12 03:47    [W:0.105 / U:6.764 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site