* [PATCH v9 0/4] Reduce overhead of LSMs with static calls
@ 2024-02-07 12:49 KP Singh
2024-02-07 12:49 ` [PATCH v9 1/4] kernel: Add helper macros for loop unrolling KP Singh
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: KP Singh @ 2024-02-07 12:49 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: paul, keescook, casey, song, daniel, ast, pabeni, andrii, kpsingh
# Background
LSM hooks (callbacks) are currently invoked as indirect function calls. These
callbacks are registered into a linked list at boot time as the order of the
LSMs can be configured on the kernel command line with the "lsm=" command line
parameter.
Indirect function calls have a high overhead due to retpoline mitigation for
various speculative execution attacks.
Retpolines remain relevant even with newer generation CPUs as recently
discovered speculative attacks, like Spectre BHB need Retpolines to mitigate
against branch history injection and still need to be used in combination with
newer mitigation features like eIBRS.
This overhead is especially significant for the "bpf" LSM which allows the user
to implement LSM functionality with eBPF program. In order to facilitate this
the "bpf" LSM provides a default callback for all LSM hooks. When enabled,
the "bpf" LSM incurs an unnecessary / avoidable indirect call. This is
especially bad in OS hot paths (e.g. in the networking stack).
This overhead prevents the adoption of bpf LSM on performance critical
systems, and also, in general, slows down all LSMs.
Since we know the address of the enabled LSM callbacks at compile time and only
the order is determined at boot time, the LSM framework can allocate static
calls for each of the possible LSM callbacks and these calls can be updated once
the order is determined at boot.
This series is a respin of the RFC proposed by Paul Renauld (renauld@google.com)
and Brendan Jackman (jackmanb@google.com) [1]
# Performance improvement
With this patch-set some syscalls with lots of LSM hooks in their path
benefitted at an average of ~3% and I/O and Pipe based system calls benefitting
the most.
Here are the results of the relevant Unixbench system benchmarks with BPF LSM
and SELinux enabled with default policies enabled with and without these
patches.
Benchmark Delta(%): (+ is better)
===============================================================================
Execl Throughput +1.9356
File Write 1024 bufsize 2000 maxblocks +6.5953
Pipe Throughput +9.5499
Pipe-based Context Switching +3.0209
Process Creation +2.3246
Shell Scripts (1 concurrent) +1.4975
System Call Overhead +2.7815
System Benchmarks Index Score (Partial Only): +3.4859
In the best case, some syscalls like eventfd_create benefitted to about ~10%.
The full analysis can be viewed at https://kpsingh.ch/lsm-perf
[1] https://lore.kernel.org/linux-security-module/20200820164753.3256899-1-jackmanb@chromium.org/
# BPF LSM Side effects
Patch 4 of the series also addresses the issues with the side effects of the
default value return values of the BPF LSM callbacks and also removes the
overheads associated with them making it deployable at hyperscale.
# v8 to v9
Paul, I removed the 5th patch about CONFIG_SECURITY_HOOK_LIKELY and went through
all the feedback. I believe it all should be addressed now.
But, please let me know if I missed anything.
The patches are based on https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
(next branch as of 2024-02-07) and resolved a bunch of conflicts.
I also added Andrii's series ack to indidividual patches.
# v7 to v8
* Addressed Andrii's feedback
* Rebased (this seems to have removed the syscall changes). v7 has the required
conflict resolution incase the conflicts need to be resolved again.
# v6 -> v7
* Rebased with latest LSM id changes merged
NOTE: The warning shown by the kernel test bot is spurious, there is no flex array
and it seems to come from an older tool chain.
https://lore.kernel.org/bpf/202310111711.wLbijitj-lkp@intel.com/
# v5 -> v6
* Fix a bug in BPF LSM hook toggle logic.
# v4 -> v5
* Rebase to linux-next/master
* Fixed the case where MAX_LSM_COUNT comes to zero when just CONFIG_SECURITY
is compiled in without any other LSM enabled as reported here:
https://lore.kernel.org/bpf/202309271206.d7fb60f9-oliver.sang@intel.com
# v3 -> v4
* Refactor LSM count macros to use COUNT_ARGS
* Change CONFIG_SECURITY_HOOK_LIKELY likely's default value to be based on
the LSM enabled and have it depend on CONFIG_EXPERT. There are a lot of subtle
options behind CONFIG_EXPERT and this should, hopefully alleviate concerns
about yet another knob.
* __randomize_layout for struct lsm_static_call and, in addition to the cover
letter add performance numbers to 3rd patch and some minor commit message
updates.
* Rebase to linux-next.
# v2 -> v3
* Fixed a build issue on archs which don't have static calls and enable
CONFIG_SECURITY.
* Updated the LSM_COUNT macros based on Andrii's suggestions.
* Changed the security_ prefix to lsm_prefix based on Casey's suggestion.
* Inlined static_branch_maybe into lsm_for_each_hook on Kees' feedback.
# v1 -> v2 (based on linux-next, next-20230614)
* Incorporated suggestions from Kees
* Changed the way MAX_LSMs are counted from a binary based generator to a clever header.
* Add CONFIG_SECURITY_HOOK_LIKELY to configure the likelihood of LSM hooks.
KP Singh (4):
kernel: Add helper macros for loop unrolling
security: Count the LSMs enabled at compile time
security: Replace indirect LSM hook calls with static calls
bpf: Only enable BPF LSM hooks when an LSM program is attached
include/linux/bpf_lsm.h | 5 +
include/linux/lsm_count.h | 114 ++++++++++++++++++
include/linux/lsm_hooks.h | 81 +++++++++++--
include/linux/unroll.h | 36 ++++++
kernel/bpf/trampoline.c | 24 ++++
security/bpf/hooks.c | 25 +++-
security/security.c | 245 ++++++++++++++++++++++++--------------
7 files changed, 431 insertions(+), 99 deletions(-)
create mode 100644 include/linux/lsm_count.h
create mode 100644 include/linux/unroll.h
--
2.43.0.594.gd9cf4e227d-goog
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v9 1/4] kernel: Add helper macros for loop unrolling 2024-02-07 12:49 [PATCH v9 0/4] Reduce overhead of LSMs with static calls KP Singh @ 2024-02-07 12:49 ` KP Singh 2024-02-07 12:49 ` [PATCH v9 2/4] security: Count the LSMs enabled at compile time KP Singh ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: KP Singh @ 2024-02-07 12:49 UTC (permalink / raw) To: bpf, linux-security-module Cc: paul, keescook, casey, song, daniel, ast, pabeni, andrii, kpsingh This helps in easily initializing blocks of code (e.g. static calls and keys). UNROLL(N, MACRO, __VA_ARGS__) calls MACRO N times with the first argument as the index of the iteration. This allows string pasting to create unique tokens for variable names, function calls etc. As an example: #include <linux/unroll.h> #define MACRO(N, a, b) \ int add_##N(int a, int b) \ { \ return a + b + N; \ } UNROLL(2, MACRO, x, y) expands to: int add_0(int x, int y) { return x + y + 0; } int add_1(int x, int y) { return x + y + 1; } Reviewed-by: Kees Cook <keescook@chromium.org> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> Acked-by: Song Liu <song@kernel.org> Acked-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: KP Singh <kpsingh@kernel.org> --- include/linux/unroll.h | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 include/linux/unroll.h diff --git a/include/linux/unroll.h b/include/linux/unroll.h new file mode 100644 index 000000000000..d42fd6366373 --- /dev/null +++ b/include/linux/unroll.h @@ -0,0 +1,36 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * Copyright (C) 2023 Google LLC. + */ + +#ifndef __UNROLL_H +#define __UNROLL_H + +#include <linux/args.h> + +#define UNROLL(N, MACRO, args...) CONCATENATE(__UNROLL_, N)(MACRO, args) + +#define __UNROLL_0(MACRO, args...) +#define __UNROLL_1(MACRO, args...) __UNROLL_0(MACRO, args) MACRO(0, args) +#define __UNROLL_2(MACRO, args...) __UNROLL_1(MACRO, args) MACRO(1, args) +#define __UNROLL_3(MACRO, args...) __UNROLL_2(MACRO, args) MACRO(2, args) +#define __UNROLL_4(MACRO, args...) __UNROLL_3(MACRO, args) MACRO(3, args) +#define __UNROLL_5(MACRO, args...) __UNROLL_4(MACRO, args) MACRO(4, args) +#define __UNROLL_6(MACRO, args...) __UNROLL_5(MACRO, args) MACRO(5, args) +#define __UNROLL_7(MACRO, args...) __UNROLL_6(MACRO, args) MACRO(6, args) +#define __UNROLL_8(MACRO, args...) __UNROLL_7(MACRO, args) MACRO(7, args) +#define __UNROLL_9(MACRO, args...) __UNROLL_8(MACRO, args) MACRO(8, args) +#define __UNROLL_10(MACRO, args...) __UNROLL_9(MACRO, args) MACRO(9, args) +#define __UNROLL_11(MACRO, args...) __UNROLL_10(MACRO, args) MACRO(10, args) +#define __UNROLL_12(MACRO, args...) __UNROLL_11(MACRO, args) MACRO(11, args) +#define __UNROLL_13(MACRO, args...) __UNROLL_12(MACRO, args) MACRO(12, args) +#define __UNROLL_14(MACRO, args...) __UNROLL_13(MACRO, args) MACRO(13, args) +#define __UNROLL_15(MACRO, args...) __UNROLL_14(MACRO, args) MACRO(14, args) +#define __UNROLL_16(MACRO, args...) __UNROLL_15(MACRO, args) MACRO(15, args) +#define __UNROLL_17(MACRO, args...) __UNROLL_16(MACRO, args) MACRO(16, args) +#define __UNROLL_18(MACRO, args...) __UNROLL_17(MACRO, args) MACRO(17, args) +#define __UNROLL_19(MACRO, args...) __UNROLL_18(MACRO, args) MACRO(18, args) +#define __UNROLL_20(MACRO, args...) __UNROLL_19(MACRO, args) MACRO(19, args) + +#endif /* __UNROLL_H */ -- 2.43.0.594.gd9cf4e227d-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v9 2/4] security: Count the LSMs enabled at compile time 2024-02-07 12:49 [PATCH v9 0/4] Reduce overhead of LSMs with static calls KP Singh 2024-02-07 12:49 ` [PATCH v9 1/4] kernel: Add helper macros for loop unrolling KP Singh @ 2024-02-07 12:49 ` KP Singh 2024-02-07 12:49 ` [PATCH v9 3/4] security: Replace indirect LSM hook calls with static calls KP Singh 2024-02-07 12:49 ` [PATCH v9 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached KP Singh 3 siblings, 0 replies; 10+ messages in thread From: KP Singh @ 2024-02-07 12:49 UTC (permalink / raw) To: bpf, linux-security-module Cc: paul, keescook, casey, song, daniel, ast, pabeni, andrii, kpsingh, Kui-Feng Lee These macros are a clever trick to determine a count of the number of LSMs that are enabled in the config to ascertain the maximum number of static calls that need to be configured per LSM hook. Without this one would need to generate static calls for the total number of LSMs in the kernel (even if they are not compiled) times the number of LSM hooks which ends up being quite wasteful. Suggested-by: Kui-Feng Lee <sinquersw@gmail.com> Suggested-by: Andrii Nakryiko <andrii@kernel.org> Acked-by: Song Liu <song@kernel.org> Acked-by: Andrii Nakryiko <andrii@kernel.org> Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: KP Singh <kpsingh@kernel.org> --- include/linux/lsm_count.h | 114 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 include/linux/lsm_count.h diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h new file mode 100644 index 000000000000..dbb3c8573959 --- /dev/null +++ b/include/linux/lsm_count.h @@ -0,0 +1,114 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * Copyright (C) 2023 Google LLC. + */ + +#ifndef __LINUX_LSM_COUNT_H +#define __LINUX_LSM_COUNT_H + +#include <linux/args.h> + +#ifdef CONFIG_SECURITY + +/* + * Macros to count the number of LSMs enabled in the kernel at compile time. + */ + +/* + * Capabilities is enabled when CONFIG_SECURITY is enabled. + */ +#if IS_ENABLED(CONFIG_SECURITY) +#define CAPABILITIES_ENABLED 1, +#else +#define CAPABILITIES_ENABLED +#endif + +#if IS_ENABLED(CONFIG_SECURITY_SELINUX) +#define SELINUX_ENABLED 1, +#else +#define SELINUX_ENABLED +#endif + +#if IS_ENABLED(CONFIG_SECURITY_SMACK) +#define SMACK_ENABLED 1, +#else +#define SMACK_ENABLED +#endif + +#if IS_ENABLED(CONFIG_SECURITY_APPARMOR) +#define APPARMOR_ENABLED 1, +#else +#define APPARMOR_ENABLED +#endif + +#if IS_ENABLED(CONFIG_SECURITY_TOMOYO) +#define TOMOYO_ENABLED 1, +#else +#define TOMOYO_ENABLED +#endif + +#if IS_ENABLED(CONFIG_SECURITY_YAMA) +#define YAMA_ENABLED 1, +#else +#define YAMA_ENABLED +#endif + +#if IS_ENABLED(CONFIG_SECURITY_LOADPIN) +#define LOADPIN_ENABLED 1, +#else +#define LOADPIN_ENABLED +#endif + +#if IS_ENABLED(CONFIG_SECURITY_LOCKDOWN_LSM) +#define LOCKDOWN_ENABLED 1, +#else +#define LOCKDOWN_ENABLED +#endif + +#if IS_ENABLED(CONFIG_SECURITY_SAFESETID) +#define SAFESETID_ENABLED 1, +#else +#define SAFESETID_ENABLED +#endif + +#if IS_ENABLED(CONFIG_BPF_LSM) +#define BPF_LSM_ENABLED 1, +#else +#define BPF_LSM_ENABLED +#endif + +#if IS_ENABLED(CONFIG_SECURITY_LANDLOCK) +#define LANDLOCK_ENABLED 1, +#else +#define LANDLOCK_ENABLED +#endif + +/* + * There is a trailing comma that we need to be accounted for. This is done by + * using a skipped argument in __COUNT_LSMS + */ +#define __COUNT_LSMS(skipped_arg, args...) COUNT_ARGS(args...) +#define COUNT_LSMS(args...) __COUNT_LSMS(args) + +#define MAX_LSM_COUNT \ + COUNT_LSMS( \ + CAPABILITIES_ENABLED \ + SELINUX_ENABLED \ + SMACK_ENABLED \ + APPARMOR_ENABLED \ + TOMOYO_ENABLED \ + YAMA_ENABLED \ + LOADPIN_ENABLED \ + LOCKDOWN_ENABLED \ + SAFESETID_ENABLED \ + BPF_LSM_ENABLED \ + LANDLOCK_ENABLED) + +#else + +#define MAX_LSM_COUNT 0 + +#endif /* CONFIG_SECURITY */ + +#endif /* __LINUX_LSM_COUNT_H */ -- 2.43.0.594.gd9cf4e227d-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v9 3/4] security: Replace indirect LSM hook calls with static calls 2024-02-07 12:49 [PATCH v9 0/4] Reduce overhead of LSMs with static calls KP Singh 2024-02-07 12:49 ` [PATCH v9 1/4] kernel: Add helper macros for loop unrolling KP Singh 2024-02-07 12:49 ` [PATCH v9 2/4] security: Count the LSMs enabled at compile time KP Singh @ 2024-02-07 12:49 ` KP Singh 2024-04-11 0:38 ` Paul Moore 2024-02-07 12:49 ` [PATCH v9 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached KP Singh 3 siblings, 1 reply; 10+ messages in thread From: KP Singh @ 2024-02-07 12:49 UTC (permalink / raw) To: bpf, linux-security-module Cc: paul, keescook, casey, song, daniel, ast, pabeni, andrii, kpsingh 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: 0xffffffff818f0ca0 <+0>: endbr64 0xffffffff818f0ca4 <+4>: nopl 0x0(%rax,%rax,1) 0xffffffff818f0ca9 <+9>: push %rbp 0xffffffff818f0caa <+10>: push %r14 0xffffffff818f0cac <+12>: push %rbx 0xffffffff818f0cad <+13>: mov %rdx,%rbx 0xffffffff818f0cb0 <+16>: mov %esi,%ebp 0xffffffff818f0cb2 <+18>: mov %rdi,%r14 0xffffffff818f0cb5 <+21>: jmp 0xffffffff818f0cc7 <security_file_ioctl+39> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Static key enabled for SELinux 0xffffffff818f0cb7 <+23>: jmp 0xffffffff818f0cde <security_file_ioctl+62> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Static key enabled for BPF LSM. This is something that is changed to default to false to avoid the existing side effect issues of BPF LSM [1] in a subsequent patch. 0xffffffff818f0cb9 <+25>: xor %eax,%eax 0xffffffff818f0cbb <+27>: xchg %ax,%ax 0xffffffff818f0cbd <+29>: pop %rbx 0xffffffff818f0cbe <+30>: pop %r14 0xffffffff818f0cc0 <+32>: pop %rbp 0xffffffff818f0cc1 <+33>: cs jmp 0xffffffff82c00000 <__x86_return_thunk> 0xffffffff818f0cc7 <+39>: endbr64 0xffffffff818f0ccb <+43>: mov %r14,%rdi 0xffffffff818f0cce <+46>: mov %ebp,%esi 0xffffffff818f0cd0 <+48>: mov %rbx,%rdx 0xffffffff818f0cd3 <+51>: call 0xffffffff81903230 <selinux_file_ioctl> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Direct call to SELinux. 0xffffffff818f0cd8 <+56>: test %eax,%eax 0xffffffff818f0cda <+58>: jne 0xffffffff818f0cbd <security_file_ioctl+29> 0xffffffff818f0cdc <+60>: jmp 0xffffffff818f0cb7 <security_file_ioctl+23> 0xffffffff818f0cde <+62>: endbr64 0xffffffff818f0ce2 <+66>: mov %r14,%rdi 0xffffffff818f0ce5 <+69>: mov %ebp,%esi 0xffffffff818f0ce7 <+71>: mov %rbx,%rdx 0xffffffff818f0cea <+74>: call 0xffffffff8141e220 <bpf_lsm_file_ioctl> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Direct call to BPF LSM. 0xffffffff818f0cef <+79>: test %eax,%eax 0xffffffff818f0cf1 <+81>: jne 0xffffffff818f0cbd <security_file_ioctl+29> 0xffffffff818f0cf3 <+83>: jmp 0xffffffff818f0cb9 <security_file_ioctl+25> 0xffffffff818f0cf5 <+85>: endbr64 0xffffffff818f0cf9 <+89>: mov %r14,%rdi 0xffffffff818f0cfc <+92>: mov %ebp,%esi 0xffffffff818f0cfe <+94>: mov %rbx,%rdx 0xffffffff818f0d01 <+97>: pop %rbx 0xffffffff818f0d02 <+98>: pop %r14 0xffffffff818f0d04 <+100>: pop %rbp 0xffffffff818f0d05 <+101>: ret 0xffffffff818f0d06 <+102>: int3 0xffffffff818f0d07 <+103>: int3 0xffffffff818f0d08 <+104>: int3 0xffffffff818f0d09 <+105>: int3 While this patch uses static_branch_unlikely indicating that an LSM hook is likely to be not present, a subsequent makes it configurable. In most cases this is still a better choice as even when an LSM with one hook is added, empty slots are created for all LSM hooks (especially when many LSMs that do not initialize most hooks are present on the system). 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. Below are results of the relevant Unixbench system benchmarks with BPF LSM and SELinux enabled with default policies enabled with and without these patches. Benchmark Delta(%): (+ is better) =============================================================================== Execl Throughput +1.9356 File Write 1024 bufsize 2000 maxblocks +6.5953 Pipe Throughput +9.5499 Pipe-based Context Switching +3.0209 Process Creation +2.3246 Shell Scripts (1 concurrent) +1.4975 System Call Overhead +2.7815 System Benchmarks Index Score (Partial Only): +3.4859 In the best case, some syscalls like eventfd_create benefitted to about ~10%. Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> Reviewed-by: Kees Cook <keescook@chromium.org> Acked-by: Song Liu <song@kernel.org> Acked-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: KP Singh <kpsingh@kernel.org> --- include/linux/lsm_hooks.h | 70 +++++++++-- security/security.c | 244 ++++++++++++++++++++++++-------------- 2 files changed, 216 insertions(+), 98 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index a2ade0ffe9e7..ba63d8b54448 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -30,16 +30,63 @@ #include <linux/init.h> #include <linux/rculist.h> #include <linux/xattr.h> +#include <linux/static_call.h> +#include <linux/unroll.h> +#include <linux/jump_label.h> +#include <linux/lsm_count.h> + +#define SECURITY_HOOK_ACTIVE_KEY(HOOK, IDX) security_hook_active_##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_LOOP_UNROLL(M, ...) \ +do { \ + UNROLL(MAX_LSM_COUNT, M, __VA_ARGS__) \ +} while (0) + +#define LSM_DEFINE_UNROLL(M, ...) UNROLL(MAX_LSM_COUNT, M, __VA_ARGS__) 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. + * @active: 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; + /* this needs to be true or false based on what the key defaults to */ + struct static_key_false *active; +} __randomize_layout; + +/* + * 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; @@ -58,10 +105,14 @@ struct lsm_id { /* * 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 struct lsm_id *lsmid; } __randomize_layout; @@ -110,10 +161,12 @@ static inline struct xattr *lsm_get_xattr_slot(struct xattr *xattrs, * 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, @@ -151,5 +204,6 @@ extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[]; __aligned(sizeof(unsigned long)) 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 */ diff --git a/security/security.c b/security/security.c index 3aaad75c9ce8..e05d2157c95a 100644 --- a/security/security.c +++ b/security/security.c @@ -30,6 +30,8 @@ #include <linux/string.h> #include <linux/msg.h> #include <net/flow.h> +#include <linux/static_call.h> +#include <linux/jump_label.h> /* How many LSMs were built into the kernel? */ #define LSM_COUNT (__end_lsm_info - __start_lsm_info) @@ -91,7 +93,6 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = { [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality", }; -struct security_hook_heads security_hook_heads __ro_after_init; static BLOCKING_NOTIFIER_HEAD(blocking_lsm_notifier_chain); static struct kmem_cache *lsm_file_cache; @@ -110,6 +111,51 @@ static __initconst const char *const builtin_lsm_order = CONFIG_LSM; static __initdata struct lsm_info **ordered_lsms; static __initdata struct lsm_info *exclusive; + +#ifdef CONFIG_HAVE_STATIC_CALL +#define LSM_HOOK_TRAMP(NAME, NUM) \ + &STATIC_CALL_TRAMP(LSM_STATIC_CALL(NAME, NUM)) +#else +#define LSM_HOOK_TRAMP(NAME, NUM) NULL +#endif + +/* + * Define static calls and static keys for each LSM hook. + */ + +#define DEFINE_LSM_STATIC_CALL(NUM, NAME, RET, ...) \ + DEFINE_STATIC_CALL_NULL(LSM_STATIC_CALL(NAME, NUM), \ + *((RET(*)(__VA_ARGS__))NULL)); \ + DEFINE_STATIC_KEY_FALSE(SECURITY_HOOK_ACTIVE_KEY(NAME, NUM)); + +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \ + LSM_DEFINE_UNROLL(DEFINE_LSM_STATIC_CALL, NAME, RET, __VA_ARGS__) +#include <linux/lsm_hook_defs.h> +#undef LSM_HOOK +#undef DEFINE_LSM_STATIC_CALL + +/* + * Initialise a table of static calls for each LSM hook. + * DEFINE_STATIC_CALL_NULL invocation above generates a key (STATIC_CALL_KEY) + * and a trampoline (STATIC_CALL_TRAMP) which are used to call + * __static_call_update when updating the static call. + */ +struct lsm_static_calls_table static_calls_table __ro_after_init = { +#define INIT_LSM_STATIC_CALL(NUM, NAME) \ + (struct lsm_static_call) { \ + .key = &STATIC_CALL_KEY(LSM_STATIC_CALL(NAME, NUM)), \ + .trampoline = LSM_HOOK_TRAMP(NAME, NUM), \ + .active = &SECURITY_HOOK_ACTIVE_KEY(NAME, NUM), \ + }, +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \ + .NAME = { \ + LSM_DEFINE_UNROLL(INIT_LSM_STATIC_CALL, NAME) \ + }, +#include <linux/lsm_hook_defs.h> +#undef LSM_HOOK +#undef INIT_LSM_STATIC_CALL +}; + static __initdata bool debug; #define init_debug(...) \ do { \ @@ -170,7 +216,7 @@ static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from) if (exists_ordered_lsm(lsm)) return; - if (WARN(last_lsm == LSM_COUNT, "%s: out of LSM slots!?\n", from)) + if (WARN(last_lsm == LSM_COUNT, "%s: out of LSM static calls!?\n", from)) return; /* Enable this LSM, if it is not already set. */ @@ -349,6 +395,25 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) kfree(sep); } +static void __init lsm_static_call_init(struct security_hook_list *hl) +{ + struct lsm_static_call *scall = hl->scalls; + int i; + + for (i = 0; i < MAX_LSM_COUNT; i++) { + /* Update the first static call that is not used yet */ + if (!scall->hl) { + __static_call_update(scall->key, scall->trampoline, + hl->hook.lsm_callback); + scall->hl = hl; + static_branch_enable(scall->active); + return; + } + scall++; + } + panic("%s - Ran out of static slots.\n", __func__); +} + static void __init lsm_early_cred(struct cred *cred); static void __init lsm_early_task(struct task_struct *task); @@ -428,11 +493,6 @@ int __init early_security_init(void) { struct lsm_info *lsm; -#define LSM_HOOK(RET, DEFAULT, NAME, ...) \ - INIT_HLIST_HEAD(&security_hook_heads.NAME); -#include "linux/lsm_hook_defs.h" -#undef LSM_HOOK - for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) { if (!lsm->enabled) lsm->enabled = &lsm_enabled_true; @@ -560,7 +620,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, for (i = 0; i < count; i++) { hooks[i].lsmid = lsmid; - hlist_add_tail_rcu(&hooks[i].list, hooks[i].head); + lsm_static_call_init(&hooks[i]); } /* @@ -846,29 +906,41 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len, * call_int_hook: * This is a hook that returns a value. */ +#define __CALL_STATIC_VOID(NUM, HOOK, ...) \ +do { \ + if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) { \ + static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__); \ + } \ +} while (0); -#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__); \ +#define call_void_hook(FUNC, ...) \ + do { \ + LSM_LOOP_UNROLL(__CALL_STATIC_VOID, FUNC, __VA_ARGS__); \ } while (0) -#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; \ +#define __CALL_STATIC_INT(NUM, R, HOOK, LABEL, ...) \ +do { \ + if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) { \ + R = static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__); \ + if (R != 0) \ + goto LABEL; \ + } \ +} while (0); + +#define call_int_hook(FUNC, IRC, ...) \ +({ \ + __label__ out; \ + int RC = IRC; \ + LSM_LOOP_UNROLL(__CALL_STATIC_INT, RC, FUNC, out, __VA_ARGS__); \ +out: \ + RC; \ }) +#define lsm_for_each_hook(scall, NAME) \ + for (scall = static_calls_table.NAME; \ + scall - static_calls_table.NAME < MAX_LSM_COUNT; scall++) \ + if (static_key_enabled(&scall->active->key)) + /* Security operations */ /** @@ -1104,7 +1176,7 @@ int security_settime64(const struct timespec64 *ts, const struct timezone *tz) */ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int cap_sys_admin = 1; int rc; @@ -1115,8 +1187,8 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) * agree that it should be set it will. If any module * thinks it should not be set it won't. */ - hlist_for_each_entry(hp, &security_hook_heads.vm_enough_memory, list) { - rc = hp->hook.vm_enough_memory(mm, pages); + lsm_for_each_hook(scall, vm_enough_memory) { + rc = scall->hl->hook.vm_enough_memory(mm, pages); if (rc <= 0) { cap_sys_admin = 0; break; @@ -1268,13 +1340,12 @@ int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc) int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int trc; int rc = -ENOPARAM; - hlist_for_each_entry(hp, &security_hook_heads.fs_context_parse_param, - list) { - trc = hp->hook.fs_context_parse_param(fc, param); + lsm_for_each_hook(scall, fs_context_parse_param) { + trc = scall->hl->hook.fs_context_parse_param(fc, param); if (trc == 0) rc = 0; else if (trc != -ENOPARAM) @@ -1637,19 +1708,19 @@ int security_dentry_init_security(struct dentry *dentry, int mode, const char **xattr_name, void **ctx, u32 *ctxlen) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int rc; /* * Only one module will provide a security context. */ - hlist_for_each_entry(hp, &security_hook_heads.dentry_init_security, - list) { - rc = hp->hook.dentry_init_security(dentry, mode, name, + lsm_for_each_hook(scall, dentry_init_security) { + rc = scall->hl->hook.dentry_init_security(dentry, mode, name, xattr_name, ctx, ctxlen); if (rc != LSM_RET_DEFAULT(dentry_init_security)) return rc; } + return LSM_RET_DEFAULT(dentry_init_security); } EXPORT_SYMBOL(security_dentry_init_security); @@ -1709,7 +1780,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, const initxattrs initxattrs, void *fs_data) { - struct security_hook_list *hp; + struct lsm_static_call *scall; struct xattr *new_xattrs = NULL; int ret = -EOPNOTSUPP, xattr_count = 0; @@ -1727,9 +1798,8 @@ int security_inode_init_security(struct inode *inode, struct inode *dir, return -ENOMEM; } - hlist_for_each_entry(hp, &security_hook_heads.inode_init_security, - list) { - ret = hp->hook.inode_init_security(inode, dir, qstr, new_xattrs, + lsm_for_each_hook(scall, inode_init_security) { + ret = scall->hl->hook.inode_init_security(inode, dir, qstr, new_xattrs, &xattr_count); if (ret && ret != -EOPNOTSUPP) goto out; @@ -2489,7 +2559,7 @@ int security_inode_getsecurity(struct mnt_idmap *idmap, struct inode *inode, const char *name, void **buffer, bool alloc) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int rc; if (unlikely(IS_PRIVATE(inode))) @@ -2497,9 +2567,8 @@ int security_inode_getsecurity(struct mnt_idmap *idmap, /* * Only one module will provide an attribute with a given name. */ - hlist_for_each_entry(hp, &security_hook_heads.inode_getsecurity, list) { - rc = hp->hook.inode_getsecurity(idmap, inode, name, buffer, - alloc); + lsm_for_each_hook(scall, inode_getsecurity) { + rc = scall->hl->hook.inode_getsecurity(idmap, inode, name, buffer, alloc); if (rc != LSM_RET_DEFAULT(inode_getsecurity)) return rc; } @@ -2524,7 +2593,7 @@ int security_inode_getsecurity(struct mnt_idmap *idmap, int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int rc; if (unlikely(IS_PRIVATE(inode))) @@ -2532,9 +2601,8 @@ int security_inode_setsecurity(struct inode *inode, const char *name, /* * Only one module will provide an attribute with a given name. */ - hlist_for_each_entry(hp, &security_hook_heads.inode_setsecurity, list) { - rc = hp->hook.inode_setsecurity(inode, name, value, size, - flags); + lsm_for_each_hook(scall, inode_setsecurity) { + rc = scall->hl->hook.inode_setsecurity(inode, name, value, size, flags); if (rc != LSM_RET_DEFAULT(inode_setsecurity)) return rc; } @@ -2608,7 +2676,7 @@ EXPORT_SYMBOL(security_inode_copy_up); */ int security_inode_copy_up_xattr(const char *name) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int rc; /* @@ -2616,9 +2684,8 @@ int security_inode_copy_up_xattr(const char *name) * xattr), -EOPNOTSUPP if it does not know anything about the xattr or * any other error code in case of an error. */ - hlist_for_each_entry(hp, - &security_hook_heads.inode_copy_up_xattr, list) { - rc = hp->hook.inode_copy_up_xattr(name); + lsm_for_each_hook(scall, inode_copy_up_xattr) { + rc = scall->hl->hook.inode_copy_up_xattr(name); if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr)) return rc; } @@ -3510,10 +3577,10 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3, { int thisrc; int rc = LSM_RET_DEFAULT(task_prctl); - struct security_hook_list *hp; + struct lsm_static_call *scall; - hlist_for_each_entry(hp, &security_hook_heads.task_prctl, list) { - thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4, arg5); + lsm_for_each_hook(scall, task_prctl) { + thisrc = scall->hl->hook.task_prctl(option, arg2, arg3, arg4, arg5); if (thisrc != LSM_RET_DEFAULT(task_prctl)) { rc = thisrc; if (thisrc != 0) @@ -3919,7 +3986,7 @@ EXPORT_SYMBOL(security_d_instantiate); int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, size_t __user *size, u32 flags) { - struct security_hook_list *hp; + struct lsm_static_call *scall; struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; u8 __user *base = (u8 __user *)uctx; size_t total = 0; @@ -3957,13 +4024,13 @@ int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, * In the usual case gather all the data from the LSMs. * In the single case only get the data from the LSM specified. */ - hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) { - if (single && lctx.id != hp->lsmid->id) + lsm_for_each_hook(scall, getselfattr) { + if (single && lctx.id != scall->hl->lsmid->id) continue; entrysize = left; if (base) uctx = (struct lsm_ctx __user *)(base + total); - rc = hp->hook.getselfattr(attr, uctx, &entrysize, flags); + rc = scall->hl->hook.getselfattr(attr, uctx, &entrysize, flags); if (rc == -EOPNOTSUPP) { rc = 0; continue; @@ -4012,7 +4079,7 @@ int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, int security_setselfattr(unsigned int attr, struct lsm_ctx __user *uctx, size_t size, u32 flags) { - struct security_hook_list *hp; + struct lsm_static_call *scall; struct lsm_ctx *lctx; int rc = LSM_RET_DEFAULT(setselfattr); @@ -4033,9 +4100,9 @@ int security_setselfattr(unsigned int attr, struct lsm_ctx __user *uctx, goto free_out; } - hlist_for_each_entry(hp, &security_hook_heads.setselfattr, list) - if ((hp->lsmid->id) == lctx->id) { - rc = hp->hook.setselfattr(attr, lctx, size, flags); + lsm_for_each_hook(scall, setselfattr) + if ((scall->hl->lsmid->id) == lctx->id) { + rc = scall->hl->hook.setselfattr(attr, lctx, size, flags); break; } @@ -4058,12 +4125,12 @@ int security_setselfattr(unsigned int attr, struct lsm_ctx __user *uctx, int security_getprocattr(struct task_struct *p, int lsmid, const char *name, char **value) { - struct security_hook_list *hp; + struct lsm_static_call *scall; - hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) { - if (lsmid != 0 && lsmid != hp->lsmid->id) + lsm_for_each_hook(scall, getprocattr) { + if (lsmid != 0 && lsmid != scall->hl->lsmid->id) continue; - return hp->hook.getprocattr(p, name, value); + return scall->hl->hook.getprocattr(p, name, value); } return LSM_RET_DEFAULT(getprocattr); } @@ -4082,12 +4149,12 @@ int security_getprocattr(struct task_struct *p, int lsmid, const char *name, */ int security_setprocattr(int lsmid, const char *name, void *value, size_t size) { - struct security_hook_list *hp; + struct lsm_static_call *scall; - hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) { - if (lsmid != 0 && lsmid != hp->lsmid->id) + lsm_for_each_hook(scall, setprocattr) { + if (lsmid != 0 && lsmid != scall->hl->lsmid->id) continue; - return hp->hook.setprocattr(name, value, size); + return scall->hl->hook.setprocattr(name, value, size); } return LSM_RET_DEFAULT(setprocattr); } @@ -4139,15 +4206,15 @@ EXPORT_SYMBOL(security_ismaclabel); */ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int rc; /* * Currently, only one LSM can implement secid_to_secctx (i.e this * LSM hook is not "stackable"). */ - hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) { - rc = hp->hook.secid_to_secctx(secid, secdata, seclen); + lsm_for_each_hook(scall, secid_to_secctx) { + rc = scall->hl->hook.secid_to_secctx(secid, secdata, seclen); if (rc != LSM_RET_DEFAULT(secid_to_secctx)) return rc; } @@ -4255,14 +4322,14 @@ EXPORT_SYMBOL(security_inode_setsecctx); */ int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int rc; /* * Only one module will provide a security context. */ - hlist_for_each_entry(hp, &security_hook_heads.inode_getsecctx, list) { - rc = hp->hook.inode_getsecctx(inode, ctx, ctxlen); + lsm_for_each_hook(scall, inode_getsecctx) { + rc = scall->hl->hook.inode_getsecctx(inode, ctx, ctxlen); if (rc != LSM_RET_DEFAULT(inode_getsecctx)) return rc; } @@ -4624,15 +4691,14 @@ EXPORT_SYMBOL(security_sock_rcv_skb); int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval, sockptr_t optlen, unsigned int len) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int rc; /* * Only one module will provide a security context. */ - hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream, - list) { - rc = hp->hook.socket_getpeersec_stream(sock, optval, optlen, + lsm_for_each_hook(scall, socket_getpeersec_stream) { + rc = scall->hl->hook.socket_getpeersec_stream(sock, optval, optlen, len); if (rc != LSM_RET_DEFAULT(socket_getpeersec_stream)) return rc; @@ -4657,15 +4723,14 @@ int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval, int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int rc; /* * Only one module will provide a security context. */ - hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_dgram, - list) { - rc = hp->hook.socket_getpeersec_dgram(sock, skb, secid); + lsm_for_each_hook(scall, socket_getpeersec_dgram) { + rc = scall->hl->hook.socket_getpeersec_dgram(sock, skb, secid); if (rc != LSM_RET_DEFAULT(socket_getpeersec_dgram)) return rc; } @@ -5225,7 +5290,7 @@ int security_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *xp, const struct flowi_common *flic) { - struct security_hook_list *hp; + struct lsm_static_call *scall; int rc = LSM_RET_DEFAULT(xfrm_state_pol_flow_match); /* @@ -5237,9 +5302,8 @@ int security_xfrm_state_pol_flow_match(struct xfrm_state *x, * For speed optimization, we explicitly break the loop rather than * using the macro */ - hlist_for_each_entry(hp, &security_hook_heads.xfrm_state_pol_flow_match, - list) { - rc = hp->hook.xfrm_state_pol_flow_match(x, xp, flic); + lsm_for_each_hook(scall, xfrm_state_pol_flow_match) { + rc = scall->hl->hook.xfrm_state_pol_flow_match(x, xp, flic); break; } return rc; -- 2.43.0.594.gd9cf4e227d-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/4] security: Replace indirect LSM hook calls with static calls 2024-02-07 12:49 ` [PATCH v9 3/4] security: Replace indirect LSM hook calls with static calls KP Singh @ 2024-04-11 0:38 ` Paul Moore 2024-04-11 7:12 ` KP Singh 0 siblings, 1 reply; 10+ messages in thread From: Paul Moore @ 2024-04-11 0:38 UTC (permalink / raw) To: KP Singh, bpf, linux-security-module Cc: keescook, casey, song, daniel, ast, pabeni, andrii, kpsingh On Feb 7, 2024 KP Singh <kpsingh@kernel.org> wrote: > > 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> Generally I fix these up, but since there are quite a few long-ish lines in the description, and a respin is probably a good idea to reduce the merge fuzz, it would be good if you could manage the line lengths a bit better. Aim to have the no wrapped lines in the commit description when you run 'git log' on a 80-char wide terminal. I'm guessing that (re)formatting the assembly to something like this will solve most of the problems: 0xff...0360: jmp 0xff...47c4 <__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: > 0xffffffff818f0ca0 <+0>: endbr64 > 0xffffffff818f0ca4 <+4>: nopl 0x0(%rax,%rax,1) > 0xffffffff818f0ca9 <+9>: push %rbp > 0xffffffff818f0caa <+10>: push %r14 > 0xffffffff818f0cac <+12>: push %rbx > 0xffffffff818f0cad <+13>: mov %rdx,%rbx > 0xffffffff818f0cb0 <+16>: mov %esi,%ebp > 0xffffffff818f0cb2 <+18>: mov %rdi,%r14 > 0xffffffff818f0cb5 <+21>: jmp 0xffffffff818f0cc7 <security_file_ioctl+39> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Static key enabled for SELinux > > 0xffffffff818f0cb7 <+23>: jmp 0xffffffff818f0cde <security_file_ioctl+62> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > Static key enabled for BPF LSM. This is something that is changed to > default to false to avoid the existing side effect issues of BPF LSM > [1] in a subsequent patch. > > 0xffffffff818f0cb9 <+25>: xor %eax,%eax > 0xffffffff818f0cbb <+27>: xchg %ax,%ax > 0xffffffff818f0cbd <+29>: pop %rbx > 0xffffffff818f0cbe <+30>: pop %r14 > 0xffffffff818f0cc0 <+32>: pop %rbp > 0xffffffff818f0cc1 <+33>: cs jmp 0xffffffff82c00000 <__x86_return_thunk> > 0xffffffff818f0cc7 <+39>: endbr64 > 0xffffffff818f0ccb <+43>: mov %r14,%rdi > 0xffffffff818f0cce <+46>: mov %ebp,%esi > 0xffffffff818f0cd0 <+48>: mov %rbx,%rdx > 0xffffffff818f0cd3 <+51>: call 0xffffffff81903230 <selinux_file_ioctl> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Direct call to SELinux. > > 0xffffffff818f0cd8 <+56>: test %eax,%eax > 0xffffffff818f0cda <+58>: jne 0xffffffff818f0cbd <security_file_ioctl+29> > 0xffffffff818f0cdc <+60>: jmp 0xffffffff818f0cb7 <security_file_ioctl+23> > 0xffffffff818f0cde <+62>: endbr64 > 0xffffffff818f0ce2 <+66>: mov %r14,%rdi > 0xffffffff818f0ce5 <+69>: mov %ebp,%esi > 0xffffffff818f0ce7 <+71>: mov %rbx,%rdx > 0xffffffff818f0cea <+74>: call 0xffffffff8141e220 <bpf_lsm_file_ioctl> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Direct call to BPF LSM. > > 0xffffffff818f0cef <+79>: test %eax,%eax > 0xffffffff818f0cf1 <+81>: jne 0xffffffff818f0cbd <security_file_ioctl+29> > 0xffffffff818f0cf3 <+83>: jmp 0xffffffff818f0cb9 <security_file_ioctl+25> > 0xffffffff818f0cf5 <+85>: endbr64 > 0xffffffff818f0cf9 <+89>: mov %r14,%rdi > 0xffffffff818f0cfc <+92>: mov %ebp,%esi > 0xffffffff818f0cfe <+94>: mov %rbx,%rdx > 0xffffffff818f0d01 <+97>: pop %rbx > 0xffffffff818f0d02 <+98>: pop %r14 > 0xffffffff818f0d04 <+100>: pop %rbp > 0xffffffff818f0d05 <+101>: ret > 0xffffffff818f0d06 <+102>: int3 > 0xffffffff818f0d07 <+103>: int3 > 0xffffffff818f0d08 <+104>: int3 > 0xffffffff818f0d09 <+105>: int3 > > While this patch uses static_branch_unlikely indicating that an LSM hook > is likely to be not present, a subsequent makes it configurable. I believe the comment above needs to be updated. > In most > cases this is still a better choice as even when an LSM with one hook is > added, empty slots are created for all LSM hooks (especially when many > LSMs that do not initialize most hooks are present on the system). > > 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. The security_for_each_hook() macro is not present in this patch. Beyond that, let's find a way to use static calls in the LSM hooks which don't use the call_{int,void}_hook() macros. If we're going to do this to help close some attack vectors, let's make sure we do the conversion everywhere. > Below are results of the relevant Unixbench system benchmarks with BPF LSM > and SELinux enabled with default policies enabled with and without these > patches. > > Benchmark Delta(%): (+ is better) > =============================================================================== > Execl Throughput +1.9356 > File Write 1024 bufsize 2000 maxblocks +6.5953 > Pipe Throughput +9.5499 > Pipe-based Context Switching +3.0209 > Process Creation +2.3246 > Shell Scripts (1 concurrent) +1.4975 > System Call Overhead +2.7815 > System Benchmarks Index Score (Partial Only): +3.4859 > > In the best case, some syscalls like eventfd_create benefitted to about ~10%. > > Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > Acked-by: Song Liu <song@kernel.org> > Acked-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: KP Singh <kpsingh@kernel.org> > --- > include/linux/lsm_hooks.h | 70 +++++++++-- > security/security.c | 244 ++++++++++++++++++++++++-------------- > 2 files changed, 216 insertions(+), 98 deletions(-) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index a2ade0ffe9e7..ba63d8b54448 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -30,16 +30,63 @@ > #include <linux/init.h> > #include <linux/rculist.h> > #include <linux/xattr.h> > +#include <linux/static_call.h> > +#include <linux/unroll.h> > +#include <linux/jump_label.h> > +#include <linux/lsm_count.h> > + > +#define SECURITY_HOOK_ACTIVE_KEY(HOOK, IDX) security_hook_active_##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_LOOP_UNROLL(M, ...) \ > +do { \ > + UNROLL(MAX_LSM_COUNT, M, __VA_ARGS__) \ > +} while (0) > + > +#define LSM_DEFINE_UNROLL(M, ...) UNROLL(MAX_LSM_COUNT, M, __VA_ARGS__) > > 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; > }; It took me a little while to figure out what you were doing with the lsm_callback field above, can we get rid of the "callback" bit and go with something to indicate this is a generic function address? How about "lsm_func_addr" or similar (bikeshedding, I know ...)? I'd also like to see a one line comment in there too. > -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. > + * @active: 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; > + /* this needs to be true or false based on what the key defaults to */ Isn't this "true or false based on if @hl is valid or not"? > + struct static_key_false *active; > +} __randomize_layout; > + > +/* > + * 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; > > @@ -58,10 +105,14 @@ struct lsm_id { > /* > * Security module hook list structure. > * For use with generic list macros for common operations. > + * > + * struct security_hook_list - Contents of a cacheable, mappable object. The comment above looks odd ... can you explain this a bit more and what your intention was with that line? > + * @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 struct lsm_id *lsmid; > } __randomize_layout; > @@ -110,10 +161,12 @@ static inline struct xattr *lsm_get_xattr_slot(struct xattr *xattrs, > * 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 } \ > + } Unless there is something that I'm missing, please just stick with the existing "HOOK" name instead of "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, > @@ -151,5 +204,6 @@ extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[]; > __aligned(sizeof(unsigned long)) > > 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 */ > diff --git a/security/security.c b/security/security.c > index 3aaad75c9ce8..e05d2157c95a 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -30,6 +30,8 @@ > #include <linux/string.h> > #include <linux/msg.h> > #include <net/flow.h> > +#include <linux/static_call.h> > +#include <linux/jump_label.h> > > /* How many LSMs were built into the kernel? */ > #define LSM_COUNT (__end_lsm_info - __start_lsm_info) > @@ -91,7 +93,6 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = { > [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality", > }; > > -struct security_hook_heads security_hook_heads __ro_after_init; > static BLOCKING_NOTIFIER_HEAD(blocking_lsm_notifier_chain); > > static struct kmem_cache *lsm_file_cache; > @@ -110,6 +111,51 @@ static __initconst const char *const builtin_lsm_order = CONFIG_LSM; > static __initdata struct lsm_info **ordered_lsms; > static __initdata struct lsm_info *exclusive; > > + > +#ifdef CONFIG_HAVE_STATIC_CALL > +#define LSM_HOOK_TRAMP(NAME, NUM) \ > + &STATIC_CALL_TRAMP(LSM_STATIC_CALL(NAME, NUM)) > +#else > +#define LSM_HOOK_TRAMP(NAME, NUM) NULL > +#endif > + > +/* > + * Define static calls and static keys for each LSM hook. > + */ > + > +#define DEFINE_LSM_STATIC_CALL(NUM, NAME, RET, ...) \ > + DEFINE_STATIC_CALL_NULL(LSM_STATIC_CALL(NAME, NUM), \ > + *((RET(*)(__VA_ARGS__))NULL)); \ > + DEFINE_STATIC_KEY_FALSE(SECURITY_HOOK_ACTIVE_KEY(NAME, NUM)); > + > +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \ > + LSM_DEFINE_UNROLL(DEFINE_LSM_STATIC_CALL, NAME, RET, __VA_ARGS__) > +#include <linux/lsm_hook_defs.h> > +#undef LSM_HOOK > +#undef DEFINE_LSM_STATIC_CALL > + > +/* > + * Initialise a table of static calls for each LSM hook. > + * DEFINE_STATIC_CALL_NULL invocation above generates a key (STATIC_CALL_KEY) > + * and a trampoline (STATIC_CALL_TRAMP) which are used to call > + * __static_call_update when updating the static call. > + */ > +struct lsm_static_calls_table static_calls_table __ro_after_init = { > +#define INIT_LSM_STATIC_CALL(NUM, NAME) \ > + (struct lsm_static_call) { \ > + .key = &STATIC_CALL_KEY(LSM_STATIC_CALL(NAME, NUM)), \ > + .trampoline = LSM_HOOK_TRAMP(NAME, NUM), \ > + .active = &SECURITY_HOOK_ACTIVE_KEY(NAME, NUM), \ > + }, > +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \ > + .NAME = { \ > + LSM_DEFINE_UNROLL(INIT_LSM_STATIC_CALL, NAME) \ > + }, > +#include <linux/lsm_hook_defs.h> > +#undef LSM_HOOK > +#undef INIT_LSM_STATIC_CALL > +}; > + > static __initdata bool debug; > #define init_debug(...) \ > do { \ > @@ -170,7 +216,7 @@ static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from) > if (exists_ordered_lsm(lsm)) > return; > > - if (WARN(last_lsm == LSM_COUNT, "%s: out of LSM slots!?\n", from)) > + if (WARN(last_lsm == LSM_COUNT, "%s: out of LSM static calls!?\n", from)) > return; > > /* Enable this LSM, if it is not already set. */ > @@ -349,6 +395,25 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) > kfree(sep); > } > > +static void __init lsm_static_call_init(struct security_hook_list *hl) > +{ > + struct lsm_static_call *scall = hl->scalls; > + int i; > + > + for (i = 0; i < MAX_LSM_COUNT; i++) { > + /* Update the first static call that is not used yet */ > + if (!scall->hl) { > + __static_call_update(scall->key, scall->trampoline, > + hl->hook.lsm_callback); > + scall->hl = hl; > + static_branch_enable(scall->active); > + return; > + } > + scall++; > + } > + panic("%s - Ran out of static slots.\n", __func__); > +} > + > static void __init lsm_early_cred(struct cred *cred); > static void __init lsm_early_task(struct task_struct *task); > > @@ -428,11 +493,6 @@ int __init early_security_init(void) > { > struct lsm_info *lsm; > > -#define LSM_HOOK(RET, DEFAULT, NAME, ...) \ > - INIT_HLIST_HEAD(&security_hook_heads.NAME); > -#include "linux/lsm_hook_defs.h" > -#undef LSM_HOOK > - > for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) { > if (!lsm->enabled) > lsm->enabled = &lsm_enabled_true; > @@ -560,7 +620,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, > > for (i = 0; i < count; i++) { > hooks[i].lsmid = lsmid; > - hlist_add_tail_rcu(&hooks[i].list, hooks[i].head); > + lsm_static_call_init(&hooks[i]); > } > > /* > @@ -846,29 +906,41 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len, > * call_int_hook: > * This is a hook that returns a value. > */ > +#define __CALL_STATIC_VOID(NUM, HOOK, ...) \ > +do { \ > + if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) { \ I'm not a fan of the likely()/unlikely() style markings/macros in cases like this as it can vary tremendously. Drop the likely()/unlikely() checks and just do a static_call(). > + static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__); \ > + } \ > +} while (0); > > -#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__); \ > +#define call_void_hook(FUNC, ...) \ > + do { \ > + LSM_LOOP_UNROLL(__CALL_STATIC_VOID, FUNC, __VA_ARGS__); \ > } while (0) > > -#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; \ > +#define __CALL_STATIC_INT(NUM, R, HOOK, LABEL, ...) \ > +do { \ > + if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) { \ See my comments in the void sister function. > + R = static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__); \ > + if (R != 0) \ > + goto LABEL; \ > + } \ > +} while (0); > + > +#define call_int_hook(FUNC, IRC, ...) \ > +({ \ > + __label__ out; \ > + int RC = IRC; \ > + LSM_LOOP_UNROLL(__CALL_STATIC_INT, RC, FUNC, out, __VA_ARGS__); \ > +out: \ > + RC; \ > }) > > +#define lsm_for_each_hook(scall, NAME) \ > + for (scall = static_calls_table.NAME; \ > + scall - static_calls_table.NAME < MAX_LSM_COUNT; scall++) \ > + if (static_key_enabled(&scall->active->key)) > + > /* Security operations */ > -- paul-moore.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/4] security: Replace indirect LSM hook calls with static calls 2024-04-11 0:38 ` Paul Moore @ 2024-04-11 7:12 ` KP Singh 2024-04-12 15:39 ` Paul Moore 0 siblings, 1 reply; 10+ messages in thread From: KP Singh @ 2024-04-11 7:12 UTC (permalink / raw) To: Paul Moore Cc: bpf, linux-security-module, Kees Cook, Casey Schaufler, song, daniel, ast, pabeni, andrii > On 11 Apr 2024, at 02:38, Paul Moore <paul@paul-moore.com> wrote: > > On Feb 7, 2024 KP Singh <kpsingh@kernel.org> wrote: >> >> 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> > > Generally I fix these up, but since there are quite a few long-ish lines > in the description, and a respin is probably a good idea to reduce the > merge fuzz, it would be good if you could manage the line lengths a bit > better. Aim to have the no wrapped lines in the commit description when > you run 'git log' on a 80-char wide terminal. I'm guessing that > (re)formatting the assembly to something like this will solve most of > the problems: > > 0xff...0360: jmp 0xff...47c4 <__x86_return_thunk> Good idea. Will do. > >> 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 [...] >> 0xffffffff818f0d07 <+103>: int3 >> 0xffffffff818f0d08 <+104>: int3 >> 0xffffffff818f0d09 <+105>: int3 >> >> While this patch uses static_branch_unlikely indicating that an LSM hook >> is likely to be not present, a subsequent makes it configurable. > > I believe the comment above needs to be updated. Done. > >> In most >> cases this is still a better choice as even when an LSM with one hook is >> added, empty slots are created for all LSM hooks (especially when many >> LSMs that do not initialize most hooks are present on the system). >> >> 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. > > The security_for_each_hook() macro is not present in this patch. Yeah, it was renamed to lsm_for_each_hook based on Casey's suggestion, I missed updating the message. Updated. > > Beyond that, let's find a way to use static calls in the LSM hooks > which don't use the call_{int,void}_hook() macros. If we're going to do > this to help close some attack vectors, let's make sure we do the > conversion everywhere. This is surely doable, We can unroll the loop individually in these separate hooks. It would need separate LSM_LOOP_UNROLL(__CALL_STATIC_xfrm_state_pol_flow_match, x, xp file) Would you be okay if we do it in a follow up series? These are special hooks and I don't want to introduce any subtle logical bugs when fixing potential speculative side channels (Which could be fixed with retpolines, proper flushing at privilege changes etc). > >> Below are results of the relevant Unixbench system benchmarks with BPF LSM >> and SELinux enabled with default policies enabled with and without these >> patches. >> >> Benchmark Delta(%): (+ is better) >> =============================================================================== >> Execl Throughput +1.9356 >> File Write 1024 bufsize 2000 maxblocks +6.5953 >> Pipe Throughput +9.5499 >> Pipe-based Context Switching +3.0209 >> Process Creation +2.3246 >> Shell Scripts (1 concurrent) +1.4975 >> System Call Overhead +2.7815 >> System Benchmarks Index Score (Partial Only): +3.4859 >> >> In the best case, some syscalls like eventfd_create benefitted to about ~10%. >> >> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> >> Reviewed-by: Kees Cook <keescook@chromium.org> >> Acked-by: Song Liu <song@kernel.org> >> Acked-by: Andrii Nakryiko <andrii@kernel.org> >> Signed-off-by: KP Singh <kpsingh@kernel.org> >> --- >> include/linux/lsm_hooks.h | 70 +++++++++-- >> security/security.c | 244 ++++++++++++++++++++++++-------------- >> 2 files changed, 216 insertions(+), 98 deletions(-) >> [...] >> #undef LSM_HOOK >> + void *lsm_callback; >> }; > > It took me a little while to figure out what you were doing with the > lsm_callback field above, can we get rid of the "callback" bit and go > with something to indicate this is a generic function address? How > about "lsm_func_addr" or similar (bikeshedding, I know ...)? > > I'd also like to see a one line comment in there too. lsm_func_addr is actually better. Thanks. > >> -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. >> + * @active: 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; >> + /* this needs to be true or false based on what the key defaults to */ > > Isn't this "true or false based on if @hl is valid or not"? See below, we are trying to avoid surplus branches and loads. > >> + struct static_key_false *active; >> +} __randomize_layout; >> + >> +/* >> + * 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; >> >> @@ -58,10 +105,14 @@ struct lsm_id { >> /* >> * Security module hook list structure. >> * For use with generic list macros for common operations. >> + * >> + * struct security_hook_list - Contents of a cacheable, mappable object. > > The comment above looks odd ... can you explain this a bit more and what > your intention was with that line? It's odd indeed. Been a while since I wrote it, I don't think that comemnt is actually needed, since the comment above it is explantory. Will delete it. > >> + * @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 struct lsm_id *lsmid; >> } __randomize_layout; >> @@ -110,10 +161,12 @@ static inline struct xattr *lsm_get_xattr_slot(struct xattr *xattrs, >> * 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 } \ >> + } > > Unless there is something that I'm missing, please just stick with the > existing "HOOK" name instead of "CALLBACK". > fair, updated. >> -extern struct security_hook_heads security_hook_heads; >> extern char *lsm_names; >> >> extern void security_add_hooks(struct security_hook_list *hooks, int count, >> @@ -151,5 +204,6 @@ extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[]; >> __aligned(sizeof(unsigned long)) >> >> extern int lsm_inode_alloc(struct inode *inode); >> +extern struct lsm_static_calls_table static_calls_table __ro_after_init; >> [...] >> /* >> @@ -846,29 +906,41 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len, >> * call_int_hook: >> * This is a hook that returns a value. >> */ >> +#define __CALL_STATIC_VOID(NUM, HOOK, ...) \ >> +do { \ >> + if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) { \ > > I'm not a fan of the likely()/unlikely() style markings/macros in cases > like this as it can vary tremendously. Drop the likely()/unlikely() > checks and just do a static_call(). > These are actually not the the classical likely, unlikely macros which are just hints to the compiler: #define likely(x) __builtin_expect(!!(x), 1) #define unlikely(x) __builtin_expect(!!(x), 0 but a part of the static keys API which generates jump tables and the code generated depends on the (default state, likelyhood). It could have been named better, all we need is to have a jump table so that we can optimize this extra branch in hotpaths, in one direction. https://www.kernel.org/doc/Documentation/static-keys.txt If you want I can put this behind a macro: #define LSM_HOOK_ACTIVE(HOOK, NUM) static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM) the static_branch_likely / static_branch_unlikey actually does not matter much here, because without this we have a conditional branch and an extra load. >> + static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__); \ >> + } \ >> +} while (0); >> >> -#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__); \ >> +#define call_void_hook(FUNC, ...) \ >> + do { \ >> + LSM_LOOP_UNROLL(__CALL_STATIC_VOID, FUNC, __VA_ARGS__); \ >> } while (0) >> >> -#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; \ >> +#define __CALL_STATIC_INT(NUM, R, HOOK, LABEL, ...) \ >> +do { \ >> + if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) { \ > > See my comments in the void sister function. See above. > >> + R = static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__); \ >> + if (R != 0) \ >> + goto LABEL; \ >> + } \ >> +} while (0); >> + >> +#define call_int_hook(FUNC, IRC, ...) \ >> +({ \ >> + __label__ out; \ >> + int RC = IRC; \ >> + LSM_LOOP_UNROLL(__CALL_STATIC_INT, RC, FUNC, out, __VA_ARGS__); \ >> +out: \ >> + RC; \ >> }) >> >> +#define lsm_for_each_hook(scall, NAME) \ >> + for (scall = static_calls_table.NAME; \ >> + scall - static_calls_table.NAME < MAX_LSM_COUNT; scall++) \ >> + if (static_key_enabled(&scall->active->key)) >> + >> /* Security operations */ >> > > -- > paul-moore.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/4] security: Replace indirect LSM hook calls with static calls 2024-04-11 7:12 ` KP Singh @ 2024-04-12 15:39 ` Paul Moore 0 siblings, 0 replies; 10+ messages in thread From: Paul Moore @ 2024-04-12 15:39 UTC (permalink / raw) To: KP Singh Cc: bpf, linux-security-module, Kees Cook, Casey Schaufler, song, daniel, ast, pabeni, andrii On Thu, Apr 11, 2024 at 3:12 AM KP Singh <kpsingh@kernel.org> wrote: > > On 11 Apr 2024, at 02:38, Paul Moore <paul@paul-moore.com> wrote: > > On Feb 7, 2024 KP Singh <kpsingh@kernel.org> wrote: > >> > >> 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: ... > > Beyond that, let's find a way to use static calls in the LSM hooks > > which don't use the call_{int,void}_hook() macros. If we're going to do > > this to help close some attack vectors, let's make sure we do the > > conversion everywhere. > > This is surely doable, We can unroll the loop individually in these separate hooks. It would need separate > > LSM_LOOP_UNROLL(__CALL_STATIC_xfrm_state_pol_flow_match, x, xp file) > > Would you be okay if we do it in a follow up series? These are special hooks and I don't want to introduce any subtle logical bugs when fixing potential speculative side channels (Which could be fixed with retpolines, proper flushing at privilege changes etc). I'm okay if you want to do it in a separate patch, but I would like to see it included in the same patchset. The good news is that recent commits have significantly reduced the number of cases where we aren't using the macros. > >> @@ -846,29 +906,41 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len, > >> * call_int_hook: > >> * This is a hook that returns a value. > >> */ > >> +#define __CALL_STATIC_VOID(NUM, HOOK, ...) \ > >> +do { \ > >> + if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) { \ > > > > I'm not a fan of the likely()/unlikely() style markings/macros in cases > > like this as it can vary tremendously. Drop the likely()/unlikely() > > checks and just do a static_call(). > > > > These are actually not the the classical likely, unlikely macros which are just hints to the compiler: > > #define likely(x) __builtin_expect(!!(x), 1) > #define unlikely(x) __builtin_expect(!!(x), 0 > > > but a part of the static keys API which generates jump tables and the code generated depends on the (default state, likelyhood). It could have been named better, all we need is to have a jump table so that we can optimize this extra branch in hotpaths, in one direction. > > https://www.kernel.org/doc/Documentation/static-keys.txt > > > If you want I can put this behind a macro: > > > #define LSM_HOOK_ACTIVE(HOOK, NUM) static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM) > > the static_branch_likely / static_branch_unlikey actually does not matter much here, because without this we have a conditional branch and an extra load. Fair enough, leave it as-is. Thanks for the explanation. -- paul-moore.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v9 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached 2024-02-07 12:49 [PATCH v9 0/4] Reduce overhead of LSMs with static calls KP Singh ` (2 preceding siblings ...) 2024-02-07 12:49 ` [PATCH v9 3/4] security: Replace indirect LSM hook calls with static calls KP Singh @ 2024-02-07 12:49 ` KP Singh 2024-04-11 0:38 ` Paul Moore 3 siblings, 1 reply; 10+ messages in thread From: KP Singh @ 2024-02-07 12:49 UTC (permalink / raw) To: bpf, linux-security-module Cc: paul, keescook, casey, song, daniel, ast, pabeni, andrii, kpsingh, Jiri Olsa BPF LSM hooks have side-effects (even when a default value is returned), as some hooks end up behaving differently due to the very presence of the hook. The static keys guarding the BPF LSM hooks are disabled by default and enabled only when a BPF program is attached implementing the hook logic. This avoids the issue of the side-effects and also the minor overhead associated with the empty callback. security_file_ioctl: 0xffffffff818f0e30 <+0>: endbr64 0xffffffff818f0e34 <+4>: nopl 0x0(%rax,%rax,1) 0xffffffff818f0e39 <+9>: push %rbp 0xffffffff818f0e3a <+10>: push %r14 0xffffffff818f0e3c <+12>: push %rbx 0xffffffff818f0e3d <+13>: mov %rdx,%rbx 0xffffffff818f0e40 <+16>: mov %esi,%ebp 0xffffffff818f0e42 <+18>: mov %rdi,%r14 0xffffffff818f0e45 <+21>: jmp 0xffffffff818f0e57 <security_file_ioctl+39> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Static key enabled for SELinux 0xffffffff818f0e47 <+23>: xchg %ax,%ax ^^^^^^^^^^^^^^ Static key disabled for BPF. This gets patched when a BPF LSM program is attached 0xffffffff818f0e49 <+25>: xor %eax,%eax 0xffffffff818f0e4b <+27>: xchg %ax,%ax 0xffffffff818f0e4d <+29>: pop %rbx 0xffffffff818f0e4e <+30>: pop %r14 0xffffffff818f0e50 <+32>: pop %rbp 0xffffffff818f0e51 <+33>: cs jmp 0xffffffff82c00000 <__x86_return_thunk> 0xffffffff818f0e57 <+39>: endbr64 0xffffffff818f0e5b <+43>: mov %r14,%rdi 0xffffffff818f0e5e <+46>: mov %ebp,%esi 0xffffffff818f0e60 <+48>: mov %rbx,%rdx 0xffffffff818f0e63 <+51>: call 0xffffffff819033c0 <selinux_file_ioctl> 0xffffffff818f0e68 <+56>: test %eax,%eax 0xffffffff818f0e6a <+58>: jne 0xffffffff818f0e4d <security_file_ioctl+29> 0xffffffff818f0e6c <+60>: jmp 0xffffffff818f0e47 <security_file_ioctl+23> 0xffffffff818f0e6e <+62>: endbr64 0xffffffff818f0e72 <+66>: mov %r14,%rdi 0xffffffff818f0e75 <+69>: mov %ebp,%esi 0xffffffff818f0e77 <+71>: mov %rbx,%rdx 0xffffffff818f0e7a <+74>: call 0xffffffff8141e3b0 <bpf_lsm_file_ioctl> 0xffffffff818f0e7f <+79>: test %eax,%eax 0xffffffff818f0e81 <+81>: jne 0xffffffff818f0e4d <security_file_ioctl+29> 0xffffffff818f0e83 <+83>: jmp 0xffffffff818f0e49 <security_file_ioctl+25> 0xffffffff818f0e85 <+85>: endbr64 0xffffffff818f0e89 <+89>: mov %r14,%rdi 0xffffffff818f0e8c <+92>: mov %ebp,%esi 0xffffffff818f0e8e <+94>: mov %rbx,%rdx 0xffffffff818f0e91 <+97>: pop %rbx 0xffffffff818f0e92 <+98>: pop %r14 0xffffffff818f0e94 <+100>: pop %rbp 0xffffffff818f0e95 <+101>: ret Reviewed-by: Kees Cook <keescook@chromium.org> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> Acked-by: Song Liu <song@kernel.org> Acked-by: Jiri Olsa <jolsa@kernel.org> Acked-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: KP Singh <kpsingh@kernel.org> --- include/linux/bpf_lsm.h | 5 +++++ include/linux/lsm_hooks.h | 13 ++++++++++++- kernel/bpf/trampoline.c | 24 ++++++++++++++++++++++++ security/bpf/hooks.c | 25 ++++++++++++++++++++++++- security/security.c | 3 ++- 5 files changed, 67 insertions(+), 3 deletions(-) diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h index 1de7ece5d36d..5bbc31ac948c 100644 --- a/include/linux/bpf_lsm.h +++ b/include/linux/bpf_lsm.h @@ -29,6 +29,7 @@ int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, bool bpf_lsm_is_sleepable_hook(u32 btf_id); bool bpf_lsm_is_trusted(const struct bpf_prog *prog); +void bpf_lsm_toggle_hook(void *addr, bool value); static inline struct bpf_storage_blob *bpf_inode( const struct inode *inode) @@ -78,6 +79,10 @@ static inline void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, { } +static inline void bpf_lsm_toggle_hook(void *addr, bool value) +{ +} + #endif /* CONFIG_BPF_LSM */ #endif /* _LINUX_BPF_LSM_H */ diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index ba63d8b54448..e95f0a5cb409 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -110,11 +110,14 @@ struct lsm_id { * @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. + * @default_state: The state of the LSM hook when initialized. If set to false, + * the static key guarding the hook will be set to disabled. */ struct security_hook_list { struct lsm_static_call *scalls; union security_list_options hook; const struct lsm_id *lsmid; + bool default_enabled; } __randomize_layout; /* @@ -164,7 +167,15 @@ static inline struct xattr *lsm_get_xattr_slot(struct xattr *xattrs, #define LSM_HOOK_INIT(NAME, CALLBACK) \ { \ .scalls = static_calls_table.NAME, \ - .hook = { .NAME = CALLBACK } \ + .hook = { .NAME = CALLBACK }, \ + .default_enabled = true \ + } + +#define LSM_HOOK_INIT_DISABLED(NAME, CALLBACK) \ + { \ + .scalls = static_calls_table.NAME, \ + .hook = { .NAME = CALLBACK }, \ + .default_enabled = false \ } extern char *lsm_names; diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c index d382f5ebe06c..5281c3338e19 100644 --- a/kernel/bpf/trampoline.c +++ b/kernel/bpf/trampoline.c @@ -13,6 +13,7 @@ #include <linux/bpf_verifier.h> #include <linux/bpf_lsm.h> #include <linux/delay.h> +#include <linux/bpf_lsm.h> /* dummy _ops. The verifier will operate on target program's ops. */ const struct bpf_verifier_ops bpf_extension_verifier_ops = { @@ -521,6 +522,21 @@ static enum bpf_tramp_prog_type bpf_attach_type_to_tramp(struct bpf_prog *prog) } } +static void bpf_trampoline_toggle_lsm(struct bpf_trampoline *tr, + enum bpf_tramp_prog_type kind) +{ + struct bpf_tramp_link *link; + bool found = false; + + hlist_for_each_entry(link, &tr->progs_hlist[kind], tramp_hlist) { + if (link->link.prog->type == BPF_PROG_TYPE_LSM) { + found = true; + break; + } + } + bpf_lsm_toggle_hook(tr->func.addr, found); +} + static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr) { enum bpf_tramp_prog_type kind; @@ -560,6 +576,10 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]); tr->progs_cnt[kind]++; + + if (link->link.prog->type == BPF_PROG_TYPE_LSM) + bpf_trampoline_toggle_lsm(tr, kind); + err = bpf_trampoline_update(tr, true /* lock_direct_mutex */); if (err) { hlist_del_init(&link->tramp_hlist); @@ -593,6 +613,10 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_ } hlist_del_init(&link->tramp_hlist); tr->progs_cnt[kind]--; + + if (link->link.prog->type == BPF_PROG_TYPE_LSM) + bpf_trampoline_toggle_lsm(tr, kind); + return bpf_trampoline_update(tr, true /* lock_direct_mutex */); } diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c index 57b9ffd53c98..38bedab2b4f9 100644 --- a/security/bpf/hooks.c +++ b/security/bpf/hooks.c @@ -9,7 +9,7 @@ static struct security_hook_list bpf_lsm_hooks[] __ro_after_init = { #define LSM_HOOK(RET, DEFAULT, NAME, ...) \ - LSM_HOOK_INIT(NAME, bpf_lsm_##NAME), + LSM_HOOK_INIT_DISABLED(NAME, bpf_lsm_##NAME), #include <linux/lsm_hook_defs.h> #undef LSM_HOOK LSM_HOOK_INIT(inode_free_security, bpf_inode_storage_free), @@ -39,3 +39,26 @@ DEFINE_LSM(bpf) = { .init = bpf_lsm_init, .blobs = &bpf_lsm_blob_sizes }; + +void bpf_lsm_toggle_hook(void *addr, bool enable) +{ + struct lsm_static_call *scalls; + struct security_hook_list *h; + int i, j; + + for (i = 0; i < ARRAY_SIZE(bpf_lsm_hooks); i++) { + h = &bpf_lsm_hooks[i]; + if (h->hook.lsm_callback != addr) + continue; + + for (j = 0; j < MAX_LSM_COUNT; j++) { + scalls = &h->scalls[j]; + if (scalls->hl != &bpf_lsm_hooks[i]) + continue; + if (enable) + static_branch_enable(scalls->active); + else + static_branch_disable(scalls->active); + } + } +} diff --git a/security/security.c b/security/security.c index e05d2157c95a..40d83da87f68 100644 --- a/security/security.c +++ b/security/security.c @@ -406,7 +406,8 @@ static void __init lsm_static_call_init(struct security_hook_list *hl) __static_call_update(scall->key, scall->trampoline, hl->hook.lsm_callback); scall->hl = hl; - static_branch_enable(scall->active); + if (hl->default_enabled) + static_branch_enable(scall->active); return; } scall++; -- 2.43.0.594.gd9cf4e227d-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v9 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached 2024-02-07 12:49 ` [PATCH v9 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached KP Singh @ 2024-04-11 0:38 ` Paul Moore 2024-05-05 16:25 ` KP Singh 0 siblings, 1 reply; 10+ messages in thread From: Paul Moore @ 2024-04-11 0:38 UTC (permalink / raw) To: KP Singh, bpf, linux-security-module Cc: keescook, casey, song, daniel, ast, pabeni, andrii, kpsingh, Jiri Olsa On Feb 7, 2024 KP Singh <kpsingh@kernel.org> wrote: > > BPF LSM hooks have side-effects (even when a default value is returned), > as some hooks end up behaving differently due to the very presence of > the hook. > > The static keys guarding the BPF LSM hooks are disabled by default and > enabled only when a BPF program is attached implementing the hook > logic. This avoids the issue of the side-effects and also the minor > overhead associated with the empty callback. > > security_file_ioctl: > 0xffffffff818f0e30 <+0>: endbr64 > 0xffffffff818f0e34 <+4>: nopl 0x0(%rax,%rax,1) > 0xffffffff818f0e39 <+9>: push %rbp > 0xffffffff818f0e3a <+10>: push %r14 > 0xffffffff818f0e3c <+12>: push %rbx > 0xffffffff818f0e3d <+13>: mov %rdx,%rbx > 0xffffffff818f0e40 <+16>: mov %esi,%ebp > 0xffffffff818f0e42 <+18>: mov %rdi,%r14 > 0xffffffff818f0e45 <+21>: jmp 0xffffffff818f0e57 <security_file_ioctl+39> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > Static key enabled for SELinux > > 0xffffffff818f0e47 <+23>: xchg %ax,%ax > ^^^^^^^^^^^^^^ > > Static key disabled for BPF. This gets patched when a BPF LSM program > is attached > > 0xffffffff818f0e49 <+25>: xor %eax,%eax > 0xffffffff818f0e4b <+27>: xchg %ax,%ax > 0xffffffff818f0e4d <+29>: pop %rbx > 0xffffffff818f0e4e <+30>: pop %r14 > 0xffffffff818f0e50 <+32>: pop %rbp > 0xffffffff818f0e51 <+33>: cs jmp 0xffffffff82c00000 <__x86_return_thunk> > 0xffffffff818f0e57 <+39>: endbr64 > 0xffffffff818f0e5b <+43>: mov %r14,%rdi > 0xffffffff818f0e5e <+46>: mov %ebp,%esi > 0xffffffff818f0e60 <+48>: mov %rbx,%rdx > 0xffffffff818f0e63 <+51>: call 0xffffffff819033c0 <selinux_file_ioctl> > 0xffffffff818f0e68 <+56>: test %eax,%eax > 0xffffffff818f0e6a <+58>: jne 0xffffffff818f0e4d <security_file_ioctl+29> > 0xffffffff818f0e6c <+60>: jmp 0xffffffff818f0e47 <security_file_ioctl+23> > 0xffffffff818f0e6e <+62>: endbr64 > 0xffffffff818f0e72 <+66>: mov %r14,%rdi > 0xffffffff818f0e75 <+69>: mov %ebp,%esi > 0xffffffff818f0e77 <+71>: mov %rbx,%rdx > 0xffffffff818f0e7a <+74>: call 0xffffffff8141e3b0 <bpf_lsm_file_ioctl> > 0xffffffff818f0e7f <+79>: test %eax,%eax > 0xffffffff818f0e81 <+81>: jne 0xffffffff818f0e4d <security_file_ioctl+29> > 0xffffffff818f0e83 <+83>: jmp 0xffffffff818f0e49 <security_file_ioctl+25> > 0xffffffff818f0e85 <+85>: endbr64 > 0xffffffff818f0e89 <+89>: mov %r14,%rdi > 0xffffffff818f0e8c <+92>: mov %ebp,%esi > 0xffffffff818f0e8e <+94>: mov %rbx,%rdx > 0xffffffff818f0e91 <+97>: pop %rbx > 0xffffffff818f0e92 <+98>: pop %r14 > 0xffffffff818f0e94 <+100>: pop %rbp > 0xffffffff818f0e95 <+101>: ret > > Reviewed-by: Kees Cook <keescook@chromium.org> > Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> > Acked-by: Song Liu <song@kernel.org> > Acked-by: Jiri Olsa <jolsa@kernel.org> > Acked-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: KP Singh <kpsingh@kernel.org> > --- > include/linux/bpf_lsm.h | 5 +++++ > include/linux/lsm_hooks.h | 13 ++++++++++++- > kernel/bpf/trampoline.c | 24 ++++++++++++++++++++++++ > security/bpf/hooks.c | 25 ++++++++++++++++++++++++- > security/security.c | 3 ++- > 5 files changed, 67 insertions(+), 3 deletions(-) > > diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h > index 1de7ece5d36d..5bbc31ac948c 100644 > --- a/include/linux/bpf_lsm.h > +++ b/include/linux/bpf_lsm.h > @@ -29,6 +29,7 @@ int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, > > bool bpf_lsm_is_sleepable_hook(u32 btf_id); > bool bpf_lsm_is_trusted(const struct bpf_prog *prog); > +void bpf_lsm_toggle_hook(void *addr, bool value); > > static inline struct bpf_storage_blob *bpf_inode( > const struct inode *inode) > @@ -78,6 +79,10 @@ static inline void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, > { > } > > +static inline void bpf_lsm_toggle_hook(void *addr, bool value) > +{ > +} > + > #endif /* CONFIG_BPF_LSM */ > > #endif /* _LINUX_BPF_LSM_H */ > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index ba63d8b54448..e95f0a5cb409 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -110,11 +110,14 @@ struct lsm_id { > * @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. > + * @default_state: The state of the LSM hook when initialized. If set to false, > + * the static key guarding the hook will be set to disabled. > */ > struct security_hook_list { > struct lsm_static_call *scalls; > union security_list_options hook; > const struct lsm_id *lsmid; > + bool default_enabled; Ugh. We've already got an lsm_static_call::active field, I don't want to see another enable/active/etc. flag unless there is absolutely no way this works otherwise. > } __randomize_layout; > > /* > @@ -164,7 +167,15 @@ static inline struct xattr *lsm_get_xattr_slot(struct xattr *xattrs, > #define LSM_HOOK_INIT(NAME, CALLBACK) \ > { \ > .scalls = static_calls_table.NAME, \ > - .hook = { .NAME = CALLBACK } \ > + .hook = { .NAME = CALLBACK }, \ > + .default_enabled = true \ > + } > + > +#define LSM_HOOK_INIT_DISABLED(NAME, CALLBACK) \ > + { \ > + .scalls = static_calls_table.NAME, \ > + .hook = { .NAME = CALLBACK }, \ > + .default_enabled = false \ > } > > extern char *lsm_names; > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c > index d382f5ebe06c..5281c3338e19 100644 > --- a/kernel/bpf/trampoline.c > +++ b/kernel/bpf/trampoline.c > @@ -13,6 +13,7 @@ > #include <linux/bpf_verifier.h> > #include <linux/bpf_lsm.h> > #include <linux/delay.h> > +#include <linux/bpf_lsm.h> > > /* dummy _ops. The verifier will operate on target program's ops. */ > const struct bpf_verifier_ops bpf_extension_verifier_ops = { > @@ -521,6 +522,21 @@ static enum bpf_tramp_prog_type bpf_attach_type_to_tramp(struct bpf_prog *prog) > } > } > > +static void bpf_trampoline_toggle_lsm(struct bpf_trampoline *tr, > + enum bpf_tramp_prog_type kind) > +{ > + struct bpf_tramp_link *link; > + bool found = false; > + > + hlist_for_each_entry(link, &tr->progs_hlist[kind], tramp_hlist) { > + if (link->link.prog->type == BPF_PROG_TYPE_LSM) { > + found = true; > + break; > + } > + } > + bpf_lsm_toggle_hook(tr->func.addr, found); > +} > + > static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr) > { > enum bpf_tramp_prog_type kind; > @@ -560,6 +576,10 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr > > hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]); > tr->progs_cnt[kind]++; > + > + if (link->link.prog->type == BPF_PROG_TYPE_LSM) > + bpf_trampoline_toggle_lsm(tr, kind); > + > err = bpf_trampoline_update(tr, true /* lock_direct_mutex */); > if (err) { > hlist_del_init(&link->tramp_hlist); > @@ -593,6 +613,10 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_ > } > hlist_del_init(&link->tramp_hlist); > tr->progs_cnt[kind]--; > + > + if (link->link.prog->type == BPF_PROG_TYPE_LSM) > + bpf_trampoline_toggle_lsm(tr, kind); > + > return bpf_trampoline_update(tr, true /* lock_direct_mutex */); > } > > diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c > index 57b9ffd53c98..38bedab2b4f9 100644 > --- a/security/bpf/hooks.c > +++ b/security/bpf/hooks.c > @@ -9,7 +9,7 @@ > > static struct security_hook_list bpf_lsm_hooks[] __ro_after_init = { > #define LSM_HOOK(RET, DEFAULT, NAME, ...) \ > - LSM_HOOK_INIT(NAME, bpf_lsm_##NAME), > + LSM_HOOK_INIT_DISABLED(NAME, bpf_lsm_##NAME), > #include <linux/lsm_hook_defs.h> > #undef LSM_HOOK > LSM_HOOK_INIT(inode_free_security, bpf_inode_storage_free), > @@ -39,3 +39,26 @@ DEFINE_LSM(bpf) = { > .init = bpf_lsm_init, > .blobs = &bpf_lsm_blob_sizes > }; > + > +void bpf_lsm_toggle_hook(void *addr, bool enable) > +{ > + struct lsm_static_call *scalls; > + struct security_hook_list *h; > + int i, j; > + > + for (i = 0; i < ARRAY_SIZE(bpf_lsm_hooks); i++) { > + h = &bpf_lsm_hooks[i]; > + if (h->hook.lsm_callback != addr) > + continue; > + > + for (j = 0; j < MAX_LSM_COUNT; j++) { > + scalls = &h->scalls[j]; > + if (scalls->hl != &bpf_lsm_hooks[i]) > + continue; > + if (enable) > + static_branch_enable(scalls->active); > + else > + static_branch_disable(scalls->active); > + } > + } > +} More ugh. If we're going to solve things this way, let's make it a proper LSM interface and not a BPF LSM specific hack; I *really* don't want to see individual LSMs managing the lsm_static_call or security_hook_list entries. > diff --git a/security/security.c b/security/security.c > index e05d2157c95a..40d83da87f68 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -406,7 +406,8 @@ static void __init lsm_static_call_init(struct security_hook_list *hl) > __static_call_update(scall->key, scall->trampoline, > hl->hook.lsm_callback); > scall->hl = hl; > - static_branch_enable(scall->active); > + if (hl->default_enabled) > + static_branch_enable(scall->active); > return; > } > scall++; > -- > 2.43.0.594.gd9cf4e227d-goog -- paul-moore.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached 2024-04-11 0:38 ` Paul Moore @ 2024-05-05 16:25 ` KP Singh 0 siblings, 0 replies; 10+ messages in thread From: KP Singh @ 2024-05-05 16:25 UTC (permalink / raw) To: Paul Moore Cc: bpf, linux-security-module, keescook, casey, song, daniel, ast, pabeni, andrii, Jiri Olsa On Thu, Apr 11, 2024 at 2:38 AM Paul Moore <paul@paul-moore.com> wrote: > > On Feb 7, 2024 KP Singh <kpsingh@kernel.org> wrote: > > > > BPF LSM hooks have side-effects (even when a default value is returned), > > as some hooks end up behaving differently due to the very presence of > > the hook. > > > > The static keys guarding the BPF LSM hooks are disabled by default and > > enabled only when a BPF program is attached implementing the hook > > logic. This avoids the issue of the side-effects and also the minor > > overhead associated with the empty callback. > > > > security_file_ioctl: > > 0xffffffff818f0e30 <+0>: endbr64 > > 0xffffffff818f0e34 <+4>: nopl 0x0(%rax,%rax,1) > > 0xffffffff818f0e39 <+9>: push %rbp > > 0xffffffff818f0e3a <+10>: push %r14 > > 0xffffffff818f0e3c <+12>: push %rbx > > 0xffffffff818f0e3d <+13>: mov %rdx,%rbx > > 0xffffffff818f0e40 <+16>: mov %esi,%ebp > > 0xffffffff818f0e42 <+18>: mov %rdi,%r14 > > 0xffffffff818f0e45 <+21>: jmp 0xffffffff818f0e57 <security_file_ioctl+39> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > Static key enabled for SELinux > > > > 0xffffffff818f0e47 <+23>: xchg %ax,%ax > > ^^^^^^^^^^^^^^ > > > > Static key disabled for BPF. This gets patched when a BPF LSM program > > is attached > > > > 0xffffffff818f0e49 <+25>: xor %eax,%eax > > 0xffffffff818f0e4b <+27>: xchg %ax,%ax > > 0xffffffff818f0e4d <+29>: pop %rbx > > 0xffffffff818f0e4e <+30>: pop %r14 > > 0xffffffff818f0e50 <+32>: pop %rbp > > 0xffffffff818f0e51 <+33>: cs jmp 0xffffffff82c00000 <__x86_return_thunk> > > 0xffffffff818f0e57 <+39>: endbr64 > > 0xffffffff818f0e5b <+43>: mov %r14,%rdi > > 0xffffffff818f0e5e <+46>: mov %ebp,%esi > > 0xffffffff818f0e60 <+48>: mov %rbx,%rdx > > 0xffffffff818f0e63 <+51>: call 0xffffffff819033c0 <selinux_file_ioctl> > > 0xffffffff818f0e68 <+56>: test %eax,%eax > > 0xffffffff818f0e6a <+58>: jne 0xffffffff818f0e4d <security_file_ioctl+29> > > 0xffffffff818f0e6c <+60>: jmp 0xffffffff818f0e47 <security_file_ioctl+23> > > 0xffffffff818f0e6e <+62>: endbr64 > > 0xffffffff818f0e72 <+66>: mov %r14,%rdi > > 0xffffffff818f0e75 <+69>: mov %ebp,%esi > > 0xffffffff818f0e77 <+71>: mov %rbx,%rdx > > 0xffffffff818f0e7a <+74>: call 0xffffffff8141e3b0 <bpf_lsm_file_ioctl> > > 0xffffffff818f0e7f <+79>: test %eax,%eax > > 0xffffffff818f0e81 <+81>: jne 0xffffffff818f0e4d <security_file_ioctl+29> > > 0xffffffff818f0e83 <+83>: jmp 0xffffffff818f0e49 <security_file_ioctl+25> > > 0xffffffff818f0e85 <+85>: endbr64 > > 0xffffffff818f0e89 <+89>: mov %r14,%rdi > > 0xffffffff818f0e8c <+92>: mov %ebp,%esi > > 0xffffffff818f0e8e <+94>: mov %rbx,%rdx > > 0xffffffff818f0e91 <+97>: pop %rbx > > 0xffffffff818f0e92 <+98>: pop %r14 > > 0xffffffff818f0e94 <+100>: pop %rbp > > 0xffffffff818f0e95 <+101>: ret > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> > > Acked-by: Song Liu <song@kernel.org> > > Acked-by: Jiri Olsa <jolsa@kernel.org> > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > Signed-off-by: KP Singh <kpsingh@kernel.org> > > --- > > include/linux/bpf_lsm.h | 5 +++++ > > include/linux/lsm_hooks.h | 13 ++++++++++++- > > kernel/bpf/trampoline.c | 24 ++++++++++++++++++++++++ > > security/bpf/hooks.c | 25 ++++++++++++++++++++++++- > > security/security.c | 3 ++- > > 5 files changed, 67 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h > > index 1de7ece5d36d..5bbc31ac948c 100644 > > --- a/include/linux/bpf_lsm.h > > +++ b/include/linux/bpf_lsm.h > > @@ -29,6 +29,7 @@ int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, > > > > bool bpf_lsm_is_sleepable_hook(u32 btf_id); > > bool bpf_lsm_is_trusted(const struct bpf_prog *prog); > > +void bpf_lsm_toggle_hook(void *addr, bool value); > > > > static inline struct bpf_storage_blob *bpf_inode( > > const struct inode *inode) > > @@ -78,6 +79,10 @@ static inline void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, > > { > > } > > > > +static inline void bpf_lsm_toggle_hook(void *addr, bool value) > > +{ > > +} > > + > > #endif /* CONFIG_BPF_LSM */ > > > > #endif /* _LINUX_BPF_LSM_H */ > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > > index ba63d8b54448..e95f0a5cb409 100644 > > --- a/include/linux/lsm_hooks.h > > +++ b/include/linux/lsm_hooks.h > > @@ -110,11 +110,14 @@ struct lsm_id { > > * @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. > > + * @default_state: The state of the LSM hook when initialized. If set to false, > > + * the static key guarding the hook will be set to disabled. > > */ > > struct security_hook_list { > > struct lsm_static_call *scalls; > > union security_list_options hook; > > const struct lsm_id *lsmid; > > + bool default_enabled; > > Ugh. We've already got an lsm_static_call::active field, I don't want > to see another enable/active/etc. flag unless there is absolutely no way > this works otherwise. The field default_enabled is used at the time of initialization. The lsm_static_call::active is a static key which we really cannot use at initialization time from the various LSMs directly. I don't see a way out of this one IMHO. > > > } __randomize_layout; > > > > /* > > @@ -164,7 +167,15 @@ static inline struct xattr *lsm_get_xattr_slot(struct xattr *xattrs, > > #define LSM_HOOK_INIT(NAME, CALLBACK) \ > > { \ > > .scalls = static_calls_table.NAME, \ > > - .hook = { .NAME = CALLBACK } \ > > + .hook = { .NAME = CALLBACK }, \ > > + .default_enabled = true \ > > + } > > + > > +#define LSM_HOOK_INIT_DISABLED(NAME, CALLBACK) \ > > + { \ > > + .scalls = static_calls_table.NAME, \ > > + .hook = { .NAME = CALLBACK }, \ [...] static_branch_disable(scalls->active); > > + } > > + } > > +} > > More ugh. If we're going to solve things this way, let's make it a > proper LSM interface and not a BPF LSM specific hack; I *really* don't > want to see individual LSMs managing the lsm_static_call or > security_hook_list entries. > Fair, and that makes the implementation much simpler too. I created a security_hook_toggle function in security.c which implements this functionality. - KP > > diff --git a/security/security.c b/security/security.c > > index e05d2157c95a..40d83da87f68 100644 > > --- a/security/security.c > > +++ b/security/security.c > > @@ -406,7 +406,8 @@ static void __init lsm_static_call_init(struct security_hook_list *hl) > > __static_call_update(scall->key, scall->trampoline, > > hl->hook.lsm_callback); > > scall->hl = hl; > > - static_branch_enable(scall->active); > > + if (hl->default_enabled) > > + static_branch_enable(scall->active); > > return; > > } > > scall++; > > -- > > 2.43.0.594.gd9cf4e227d-goog > > -- > paul-moore.com ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-05-05 16:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-07 12:49 [PATCH v9 0/4] Reduce overhead of LSMs with static calls KP Singh 2024-02-07 12:49 ` [PATCH v9 1/4] kernel: Add helper macros for loop unrolling KP Singh 2024-02-07 12:49 ` [PATCH v9 2/4] security: Count the LSMs enabled at compile time KP Singh 2024-02-07 12:49 ` [PATCH v9 3/4] security: Replace indirect LSM hook calls with static calls KP Singh 2024-04-11 0:38 ` Paul Moore 2024-04-11 7:12 ` KP Singh 2024-04-12 15:39 ` Paul Moore 2024-02-07 12:49 ` [PATCH v9 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached KP Singh 2024-04-11 0:38 ` Paul Moore 2024-05-05 16:25 ` KP Singh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).