security: Replace indirect LSM hook calls with static calls

LSM hooks are currently invoked from a linked list as indirect calls which are invoked using retpolines as a mitigation for speculative attacks (Branch History / Target injection) and add extra overhead which is especially bad in kernel hot paths: security_file_ioctl: 0xffffffff814f0320 <+0>: endbr64 0xffffffff814f0324 <+4>: push %rbp 0xffffffff814f0325 <+5>: push %r15 0xffffffff814f0327 <+7>: push %r14 0xffffffff814f0329 <+9>: push %rbx 0xffffffff814f032a <+10>: mov %rdx,%rbx 0xffffffff814f032d <+13>: mov %esi,%ebp 0xffffffff814f032f <+15>: mov %rdi,%r14 0xffffffff814f0332 <+18>: mov $0xffffffff834a7030,%r15 0xffffffff814f0339 <+25>: mov (%r15),%r15 0xffffffff814f033c <+28>: test %r15,%r15 0xffffffff814f033f <+31>: je 0xffffffff814f0358 <security_file_ioctl+56> 0xffffffff814f0341 <+33>: mov 0x18(%r15),%r11 0xffffffff814f0345 <+37>: mov %r14,%rdi 0xffffffff814f0348 <+40>: mov %ebp,%esi 0xffffffff814f034a <+42>: mov %rbx,%rdx >>> 0xffffffff814f034d <+45>: call 0xffffffff81f742e0 <__x86_indirect_thunk_array+352> Indirect calls that use retpolines leading to overhead, not just due to extra instruction but also branch misses. 0xffffffff814f0352 <+50>: test %eax,%eax 0xffffffff814f0354 <+52>: je 0xffffffff814f0339 <security_file_ioctl+25> 0xffffffff814f0356 <+54>: jmp 0xffffffff814f035a <security_file_ioctl+58> 0xffffffff814f0358 <+56>: xor %eax,%eax 0xffffffff814f035a <+58>: pop %rbx 0xffffffff814f035b <+59>: pop %r14 0xffffffff814f035d <+61>: pop %r15 0xffffffff814f035f <+63>: pop %rbp 0xffffffff814f0360 <+64>: jmp 0xffffffff81f747c4 <__x86_return_thunk> The indirect calls are not really needed as one knows the addresses of enabled LSM callbacks at boot time and only the order can possibly change at boot time with the lsm= kernel command line parameter. An array of static calls is defined per LSM hook and the static calls are updated at boot time once the order has been determined. A static key guards whether an LSM static call is enabled or not, without this static key, for LSM hooks that return an int, the presence of the hook that returns a default value can create side-effects which has resulted in bugs [1]. With the hook now exposed as a static call, one can see that the retpolines are no longer there and the LSM callbacks are invoked directly: security_file_ioctl: 0xffffffff814f0dd0 <+0>: endbr64 0xffffffff814f0dd4 <+4>: push %rbp 0xffffffff814f0dd5 <+5>: push %r14 0xffffffff814f0dd7 <+7>: push %rbx 0xffffffff814f0dd8 <+8>: mov %rdx,%rbx 0xffffffff814f0ddb <+11>: mov %esi,%ebp 0xffffffff814f0ddd <+13>: mov %rdi,%r14 >>> 0xffffffff814f0de0 <+16>: jmp 0xffffffff814f0df1 <security_file_ioctl+33> Static key enabled for selinux_file_ioctl >>> 0xffffffff814f0de2 <+18>: jmp 0xffffffff814f0e08 <security_file_ioctl+56> Static key enabled for bpf_lsm_file_ioctl. This is something that is changed to default to false to avoid the existing side effect issues of BPF LSM [1] 0xffffffff814f0de4 <+20>: xor %eax,%eax 0xffffffff814f0de6 <+22>: xchg %ax,%ax 0xffffffff814f0de8 <+24>: pop %rbx 0xffffffff814f0de9 <+25>: pop %r14 0xffffffff814f0deb <+27>: pop %rbp 0xffffffff814f0dec <+28>: jmp 0xffffffff81f767c4 <__x86_return_thunk> 0xffffffff814f0df1 <+33>: endbr64 0xffffffff814f0df5 <+37>: mov %r14,%rdi 0xffffffff814f0df8 <+40>: mov %ebp,%esi 0xffffffff814f0dfa <+42>: mov %rbx,%rdx >>> 0xffffffff814f0dfd <+45>: call 0xffffffff814fe820 <selinux_file_ioctl> Direct call to SELinux. 0xffffffff814f0e02 <+50>: test %eax,%eax 0xffffffff814f0e04 <+52>: jne 0xffffffff814f0de8 <security_file_ioctl+24> 0xffffffff814f0e06 <+54>: jmp 0xffffffff814f0de2 <security_file_ioctl+18> 0xffffffff814f0e08 <+56>: endbr64 0xffffffff814f0e0c <+60>: mov %r14,%rdi 0xffffffff814f0e0f <+63>: mov %ebp,%esi 0xffffffff814f0e11 <+65>: mov %rbx,%rdx >>> 0xffffffff814f0e14 <+68>: call 0xffffffff8123b7d0 <bpf_lsm_file_ioctl> Direct call to bpf_lsm_file_ioctl 0xffffffff814f0e19 <+73>: test %eax,%eax 0xffffffff814f0e1b <+75>: jne 0xffffffff814f0de8 <security_file_ioctl+24> 0xffffffff814f0e1d <+77>: jmp 0xffffffff814f0de4 <security_file_ioctl+20> 0xffffffff814f0e1f <+79>: endbr64 0xffffffff814f0e23 <+83>: mov %r14,%rdi 0xffffffff814f0e26 <+86>: mov %ebp,%esi 0xffffffff814f0e28 <+88>: mov %rbx,%rdx 0xffffffff814f0e2b <+91>: pop %rbx 0xffffffff814f0e2c <+92>: pop %r14 0xffffffff814f0e2e <+94>: pop %rbp 0xffffffff814f0e2f <+95>: ret 0xffffffff814f0e30 <+96>: int3 0xffffffff814f0e31 <+97>: int3 0xffffffff814f0e32 <+98>: int3 0xffffffff814f0e33 <+99>: int3 There are some hooks that don't use the call_int_hook and call_void_hook. These hooks are updated to use a new macro called security_for_each_hook where the lsm_callback is directly invoked as an indirect call. Currently, there are no performance sensitive hooks that use the security_for_each_hook macro. However, if, some performance sensitive hooks are discovered, these can be updated to use static calls with loop unrolling as well using a custom macro. [1] https://lore.kernel.org/linux-security-module/20220609234601.2026362-1-kpsingh@kernel.org/ Signed-off-by: KP Singh <kpsingh@kernel.org>
intel-lab-lkp · Jan 20, 2023 · 1dfb908 · 1dfb908
1 parent 831b062
commit 1dfb908
Show file tree

Hide file tree

Showing 2 changed files with 211 additions and 88 deletions.
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
@@ -28,6 +28,26 @@
 #include <linux/security.h>
 #include <linux/init.h>
 #include <linux/rculist.h>
+#include <linux/static_call.h>
+#include <linux/unroll.h>
+#include <linux/jump_label.h>
+
+/* Include the generated MAX_LSM_COUNT */
+#include <generated/lsm_count.h>
+
+#define SECURITY_HOOK_ENABLED_KEY(HOOK, IDX) security_enabled_key_##HOOK##_##IDX
+
+/*
+ * Identifier for the LSM static calls.
+ * HOOK is an LSM hook as defined in linux/lsm_hookdefs.h
+ * IDX is the index of the static call. 0 <= NUM < MAX_LSM_COUNT
+ */
+#define LSM_STATIC_CALL(HOOK, IDX) lsm_static_call_##HOOK##_##IDX
+
+/*
+ * Call the macro M for each LSM hook MAX_LSM_COUNT times.
+ */
+#define LSM_UNROLL(M, ...) UNROLL(MAX_LSM_COUNT, M, __VA_ARGS__)
 
 /**
  * union security_list_options - Linux Security Module hook function list
@@ -1657,21 +1677,48 @@ union security_list_options {
 	#define LSM_HOOK(RET, DEFAULT, NAME, ...) RET (*NAME)(__VA_ARGS__);
 	#include "lsm_hook_defs.h"
 	#undef LSM_HOOK
+	void *lsm_callback;
 };
 
-struct security_hook_heads {
-	#define LSM_HOOK(RET, DEFAULT, NAME, ...) struct hlist_head NAME;
-	#include "lsm_hook_defs.h"
+/*
+ * @key: static call key as defined by STATIC_CALL_KEY
+ * @trampoline: static call trampoline as defined by STATIC_CALL_TRAMP
+ * @hl: The security_hook_list as initialized by the owning LSM.
+ * @enabled_key: Enabled when the static call has an LSM hook associated.
+ */
+struct lsm_static_call {
+	struct static_call_key *key;
+	void *trampoline;
+	struct security_hook_list *hl;
+	struct static_key *enabled_key;
+};
+
+/*
+ * Table of the static calls for each LSM hook.
+ * Once the LSMs are initialized, their callbacks will be copied to these
+ * tables such that the calls are filled backwards (from last to first).
+ * This way, we can jump directly to the first used static call, and execute
+ * all of them after. This essentially makes the entry point
+ * dynamic to adapt the number of static calls to the number of callbacks.
+ */
+struct lsm_static_calls_table {
+	#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
+		struct lsm_static_call NAME[MAX_LSM_COUNT];
+	#include <linux/lsm_hook_defs.h>
 	#undef LSM_HOOK
 } __randomize_layout;
 
 /*
  * Security module hook list structure.
  * For use with generic list macros for common operations.
+ *
+ * struct security_hook_list - Contents of a cacheable, mappable object.
+ * @scalls: The beginning of the array of static calls assigned to this hook.
+ * @hook: The callback for the hook.
+ * @lsm: The name of the lsm that owns this hook.
  */
 struct security_hook_list {
-	struct hlist_node		list;
-	struct hlist_head		*head;
+	struct lsm_static_call	*scalls;
 	union security_list_options	hook;
 	const char			*lsm;
 } __randomize_layout;
@@ -1701,10 +1748,12 @@ struct lsm_blob_sizes {
  * care of the common case and reduces the amount of
  * text involved.
  */
-#define LSM_HOOK_INIT(HEAD, HOOK) \
-	{ .head = &security_hook_heads.HEAD, .hook = { .HEAD = HOOK } }
+#define LSM_HOOK_INIT(NAME, CALLBACK)			\
+	{						\
+		.scalls = static_calls_table.NAME,	\
+		.hook = { .NAME = CALLBACK }		\
+	}
 
-extern struct security_hook_heads security_hook_heads;
 extern char *lsm_names;
 
 extern void security_add_hooks(struct security_hook_list *hooks, int count,
@@ -1756,10 +1805,21 @@ extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
 static inline void security_delete_hooks(struct security_hook_list *hooks,
 						int count)
 {
-	int i;
+	struct lsm_static_call *scalls;
+	int i, j;
+
+	for (i = 0; i < count; i++) {
+		scalls = hooks[i].scalls;
+		for (j = 0; j < MAX_LSM_COUNT; j++) {
+			if (scalls[j].hl != &hooks[i])
+				continue;
 
-	for (i = 0; i < count; i++)
-		hlist_del_rcu(&hooks[i].list);
+			static_key_disable(scalls[j].enabled_key);
+			__static_call_update(scalls[j].key,
+					     scalls[j].trampoline, NULL);
+			scalls[j].hl = NULL;
+		}
+	}
 }
 #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
 
@@ -1771,5 +1831,6 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
 #endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
 
 extern int lsm_inode_alloc(struct inode *inode);
+extern struct lsm_static_calls_table static_calls_table __ro_after_init;
 
 #endif /* ! __LINUX_LSM_HOOKS_H */