[PATCH v3 0/5] Reduce overhead of LSMs with static calls

[PATCH v3 0/5] Reduce overhead of LSMs with static calls

[KP Singh]

# 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.

# 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 (5):
  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
  security: Add CONFIG_SECURITY_HOOK_LIKELY

 include/linux/bpf.h       |   1 +
 include/linux/bpf_lsm.h   |   5 +
 include/linux/lsm_count.h | 106 +++++++++++++++++++
 include/linux/lsm_hooks.h |  81 +++++++++++++--
 include/linux/unroll.h    |  36 +++++++
 kernel/bpf/trampoline.c   |  29 +++++-
 security/Kconfig          |  11 ++
 security/bpf/hooks.c      |  25 ++++-
 security/security.c       | 213 +++++++++++++++++++++++++-------------
 9 files changed, 424 insertions(+), 83 deletions(-)
 create mode 100644 include/linux/lsm_count.h
 create mode 100644 include/linux/unroll.h

-- 
2.42.0.459.ge4e396fd5e-goog

[PATCH v3 1/5] kernel: Add helper macros for loop unrolling

[KP Singh]

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;
	}

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.42.0.459.ge4e396fd5e-goog

Re: [PATCH v3 1/5] kernel: Add helper macros for loop unrolling

[Kees Cook]

On Mon, Sep 18, 2023 at 11:24:55PM +0200, KP Singh wrote:
> 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;
> 	}
> 
> Signed-off-by: KP Singh <kpsingh@kernel.org>

A handy bit of macro fun to have. :)

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v3 1/5] kernel: Add helper macros for loop unrolling

[Casey Schaufler]

On 9/18/2023 2:24 PM, KP Singh wrote:
> 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;
> 	}
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>

I confess that I find some of the macros are scary, nonetheless

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>

> ---
>  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 */

Re: [PATCH v3 1/5] kernel: Add helper macros for loop unrolling

[Song Liu]

On Mon, Sep 18, 2023 at 2:25 PM KP Singh <kpsingh@kernel.org> wrote:
>
> 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;
>         }
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>

Acked-by: Song Liu <song@kernel.org>

[PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

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 (number of
possible LSMs * number of LSM hooks) which ends up being quite wasteful
especially when some LSMs are not compiled into the kernel.

Suggested-by: Kui-Feng Lee <sinquersw@gmail.com>
Suggested-by: Andrii Nakryiko <andrii@kernel.org
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/lsm_count.h | 106 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 106 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..0c0ff3c7dddc
--- /dev/null
+++ b/include/linux/lsm_count.h
@@ -0,0 +1,106 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (C) 2023 Google LLC.
+ */
+
+#ifndef __LINUX_LSM_COUNT_H
+#define __LINUX_LSM_COUNT_H
+
+#include <linux/kconfig.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_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
+
+
+#define __COUNT_COMMAS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
+#define COUNT_COMMAS(a, X...) __COUNT_COMMAS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
+#define ___COUNT_COMMAS(args...) COUNT_COMMAS(args)
+
+
+#define MAX_LSM_COUNT			\
+	___COUNT_COMMAS(		\
+		CAPABILITIES_ENABLED	\
+		SELINUX_ENABLED		\
+		SMACK_ENABLED		\
+		APPARMOR_ENABLED	\
+		TOMOYO_ENABLED		\
+		YAMA_ENABLED		\
+		LOADPIN_ENABLED		\
+		LOCKDOWN_ENABLED	\
+		BPF_LSM_ENABLED		\
+		LANDLOCK_ENABLED)
+
+#else
+
+#define MAX_LSM_COUNT 0
+
+#endif /* CONFIG_SECURITY */
+
+#endif  /* __LINUX_LSM_COUNT_H */
-- 
2.42.0.459.ge4e396fd5e-goog

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Kees Cook]

On Mon, Sep 18, 2023 at 11:24:56PM +0200, KP Singh wrote:
> 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 (number of
> possible LSMs * number of LSM hooks) which ends up being quite wasteful
> especially when some LSMs are not compiled into the kernel.
> 
> Suggested-by: Kui-Feng Lee <sinquersw@gmail.com>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org
> Signed-off-by: KP Singh <kpsingh@kernel.org>

I may extract this into a separate header in the future -- I have plans
to make strscpy() take a variable number of arguments. ;) Regardless,
for the LSM usage:

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Casey Schaufler]

On 9/18/2023 2:24 PM, KP Singh wrote:
> 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 (number of
> possible LSMs * number of LSM hooks) which ends up being quite wasteful
> especially when some LSMs are not compiled into the kernel.
>
> Suggested-by: Kui-Feng Lee <sinquersw@gmail.com>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org
> Signed-off-by: KP Singh <kpsingh@kernel.org>

Much better than previous versions.

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>

> ---
>  include/linux/lsm_count.h | 106 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 106 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..0c0ff3c7dddc
> --- /dev/null
> +++ b/include/linux/lsm_count.h
> @@ -0,0 +1,106 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (C) 2023 Google LLC.
> + */
> +
> +#ifndef __LINUX_LSM_COUNT_H
> +#define __LINUX_LSM_COUNT_H
> +
> +#include <linux/kconfig.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_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
> +
> +
> +#define __COUNT_COMMAS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
> +#define COUNT_COMMAS(a, X...) __COUNT_COMMAS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
> +#define ___COUNT_COMMAS(args...) COUNT_COMMAS(args)
> +
> +
> +#define MAX_LSM_COUNT			\
> +	___COUNT_COMMAS(		\
> +		CAPABILITIES_ENABLED	\
> +		SELINUX_ENABLED		\
> +		SMACK_ENABLED		\
> +		APPARMOR_ENABLED	\
> +		TOMOYO_ENABLED		\
> +		YAMA_ENABLED		\
> +		LOADPIN_ENABLED		\
> +		LOCKDOWN_ENABLED	\
> +		BPF_LSM_ENABLED		\
> +		LANDLOCK_ENABLED)
> +
> +#else
> +
> +#define MAX_LSM_COUNT 0
> +
> +#endif /* CONFIG_SECURITY */
> +
> +#endif  /* __LINUX_LSM_COUNT_H */

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Kees Cook]

On 9/18/2023 2:24 PM, KP Singh wrote:
> [...]
> +#define __COUNT_COMMAS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
> +#define COUNT_COMMAS(a, X...) __COUNT_COMMAS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
> +#define ___COUNT_COMMAS(args...) COUNT_COMMAS(args)

Oh! Oops, I missed that this _DOES_ already exist in Linux:

cf14f27f82af ("macro: introduce COUNT_ARGS() macro")

now in include/linux/args.h as COUNT_ARGS():

#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)

I think this can be refactored to use that?

-Kees

-- 
Kees Cook

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

On Wed, Sep 20, 2023 at 9:24 PM Kees Cook <keescook@chromium.org> wrote:
>
> On 9/18/2023 2:24 PM, KP Singh wrote:
> > [...]
> > +#define __COUNT_COMMAS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
> > +#define COUNT_COMMAS(a, X...) __COUNT_COMMAS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
> > +#define ___COUNT_COMMAS(args...) COUNT_COMMAS(args)
>
> Oh! Oops, I missed that this _DOES_ already exist in Linux:
>
> cf14f27f82af ("macro: introduce COUNT_ARGS() macro")
>
> now in include/linux/args.h as COUNT_ARGS():
>
> #define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
> #define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
>
> I think this can be refactored to use that?

Thanks, yeah I was able to do this with:

diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
index 0c0ff3c7dddc..969b6bf60718 100644
--- a/include/linux/lsm_count.h
+++ b/include/linux/lsm_count.h
@@ -7,7 +7,7 @@
 #ifndef __LINUX_LSM_COUNT_H
 #define __LINUX_LSM_COUNT_H

-#include <linux/kconfig.h>
+#include <linux/args.h>

 #ifdef CONFIG_SECURITY

@@ -79,13 +79,15 @@
 #endif


-#define __COUNT_COMMAS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10,
_11, _12, _n, X...) >
-#define COUNT_COMMAS(a, X...) __COUNT_COMMAS(, ##X, 12, 11, 10, 9, 8,
7, 6, 5, 4, 3, 2, >
-#define ___COUNT_COMMAS(args...) COUNT_COMMAS(args)
-
+/*
+ *  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_COMMAS(                \
+       COUNT_LSMS(                     \
                CAPABILITIES_ENABLED    \
                SELINUX_ENABLED         \
                SMACK_ENABLED           \



>
> -Kees
>
> --
> Kees Cook
>

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Song Liu]

On Thu, Sep 21, 2023 at 1:41 AM KP Singh <kpsingh@kernel.org> wrote:
>
> On Wed, Sep 20, 2023 at 9:24 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On 9/18/2023 2:24 PM, KP Singh wrote:
> > > [...]
> > > +#define __COUNT_COMMAS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
> > > +#define COUNT_COMMAS(a, X...) __COUNT_COMMAS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
> > > +#define ___COUNT_COMMAS(args...) COUNT_COMMAS(args)
> >
> > Oh! Oops, I missed that this _DOES_ already exist in Linux:
> >
> > cf14f27f82af ("macro: introduce COUNT_ARGS() macro")
> >
> > now in include/linux/args.h as COUNT_ARGS():
> >
> > #define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
> > #define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
> >
> > I think this can be refactored to use that?

Aha, I noticed the same thing when backporting the set to 6.4 for testing. (Some
dependency of this set uses args.h).

>
> Thanks, yeah I was able to do this with:

With this fixed:

Acked-by: Song Liu <song@kernel.org>


>
> diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
> index 0c0ff3c7dddc..969b6bf60718 100644
> --- a/include/linux/lsm_count.h
> +++ b/include/linux/lsm_count.h
> @@ -7,7 +7,7 @@
>  #ifndef __LINUX_LSM_COUNT_H
>  #define __LINUX_LSM_COUNT_H
>
> -#include <linux/kconfig.h>
> +#include <linux/args.h>
>
>  #ifdef CONFIG_SECURITY
>
> @@ -79,13 +79,15 @@
>  #endif
>
>
> -#define __COUNT_COMMAS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10,
> _11, _12, _n, X...) >
> -#define COUNT_COMMAS(a, X...) __COUNT_COMMAS(, ##X, 12, 11, 10, 9, 8,
> 7, 6, 5, 4, 3, 2, >
> -#define ___COUNT_COMMAS(args...) COUNT_COMMAS(args)
> -
> +/*
> + *  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_COMMAS(                \
> +       COUNT_LSMS(                     \
>                 CAPABILITIES_ENABLED    \
>                 SELINUX_ENABLED         \
>                 SMACK_ENABLED           \
>
>
>
> >
> > -Kees
> >
> > --
> > Kees Cook
> >

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Tetsuo Handa]

On 2023/09/19 6:24, KP Singh wrote:
> 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.

As a LKM-based LSM user, indirect function calls using a linked list have
an advantage which this series kills. There always is a situation where a
LSM cannot be built into vmlinux (and hence has to be loaded as a LKM-based
LSM) due to distributor's support policy. Therefore, honestly speaking,
I don't want LSM infrastructure to define the maximum number of "slots" or
"static calls"...

> 
> Without this one would need to generate static calls for (number of
> possible LSMs * number of LSM hooks) which ends up being quite wasteful
> especially when some LSMs are not compiled into the kernel.

I can't interpret "number of possible LSMs * number of LSM hooks" part.
Is this tokenized as "number of possible LSMs" (an integer) * (multipled by)
"number of LSM hooks" (an integer) ? But the next patch includes

  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;

which seems to me that lsm_static_calls_table will get "number of possible
LSMs" static calls for each LSM hook defined in linux/lsm_hook_defs.h .
How did this patch help reducing static calls? What does "possible LSMs" mean?
Should "number of possible LSMs" be replaced with "number of built-in LSMs" ?

> Suggested-by: Andrii Nakryiko <andrii@kernel.org

Trailing ">" is missing.

> +/*
> + * Macros to count the number of LSMs enabled in the kernel at compile time.
> + */
> +#define MAX_LSM_COUNT			\
> +	___COUNT_COMMAS(		\
> +		CAPABILITIES_ENABLED	\
> +		SELINUX_ENABLED		\
> +		SMACK_ENABLED		\
> +		APPARMOR_ENABLED	\
> +		TOMOYO_ENABLED		\
> +		YAMA_ENABLED		\
> +		LOADPIN_ENABLED		\
> +		LOCKDOWN_ENABLED	\
> +		BPF_LSM_ENABLED		\
> +		LANDLOCK_ENABLED)

Since IS_ENABLED(CONFIG_FOO) is evaluated to either 1 or 0, why can't you directly
do like IS_ENABLED(CONFIG_FOO) + IS_ENABLED(CONFIG_BAR) + IS_ENABLED(CONFIG_BUZ) ?
If you can't do direct "+", can't you still do indirect "+" like something below?

#if IS_ENABLED(CONFIG_FOO)
#define FOO_ENABLED 1
#else
#define FOO_ENABLED 0
#endif

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

On Thu, Sep 21, 2023 at 3:21 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/09/19 6:24, KP Singh wrote:
> > 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.
>
> As a LKM-based LSM user, indirect function calls using a linked list have
> an advantage which this series kills. There always is a situation where a


> LSM cannot be built into vmlinux (and hence has to be loaded as a LKM-based
> LSM) due to distributor's support policy. Therefore, honestly speaking,
> I don't want LSM infrastructure to define the maximum number of "slots" or
> "static calls"...
>

Yeah, LSMs are not meant to be used from a kernel module. The data
structure is actually __ro_after_init. So, I am not even sure how you
are using it in kernel modules (unless you are patching this out).
And, if you are really patching stuff to get your out of tree LSMs to
work, then you might as well add your "custom" LSM config here or just
override this count.

The performance benefits here outweigh the need for a completely
unsupported use case.

> >
> > Without this one would need to generate static calls for (number of
> > possible LSMs * number of LSM hooks) which ends up being quite wasteful
> > especially when some LSMs are not compiled into the kernel.
>
> I can't interpret "number of possible LSMs * number of LSM hooks" part.
> Is this tokenized as "number of possible LSMs" (an integer) * (multipled by)
> "number of LSM hooks" (an integer) ? But the next patch includes
>
>   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;
>
> which seems to me that lsm_static_calls_table will get "number of possible
> LSMs" static calls for each LSM hook defined in linux/lsm_hook_defs.h .
> How did this patch help reducing static calls? What does "possible LSMs" mean?
> Should "number of possible LSMs" be replaced with "number of built-in LSMs" ?
>
> > Suggested-by: Andrii Nakryiko <andrii@kernel.org
>
> Trailing ">" is missing.
>
> > +/*
> > + * Macros to count the number of LSMs enabled in the kernel at compile time.
> > + */
> > +#define MAX_LSM_COUNT                        \
> > +     ___COUNT_COMMAS(                \
> > +             CAPABILITIES_ENABLED    \
> > +             SELINUX_ENABLED         \
> > +             SMACK_ENABLED           \
> > +             APPARMOR_ENABLED        \
> > +             TOMOYO_ENABLED          \
> > +             YAMA_ENABLED            \
> > +             LOADPIN_ENABLED         \
> > +             LOCKDOWN_ENABLED        \
> > +             BPF_LSM_ENABLED         \
> > +             LANDLOCK_ENABLED)
>
> Since IS_ENABLED(CONFIG_FOO) is evaluated to either 1 or 0, why can't you directly
> do like IS_ENABLED(CONFIG_FOO) + IS_ENABLED(CONFIG_BAR) + IS_ENABLED(CONFIG_BUZ) ?

You cannot do this because this is not evaluated in the preprocessor
and is used to generate the variable names. If you have a working
snippet of code, please share.

> If you can't do direct "+", can't you still do indirect "+" like something below?
>
> #if IS_ENABLED(CONFIG_FOO)
> #define FOO_ENABLED 1
> #else
> #define FOO_ENABLED 0
> #endif

How is this an indirect addition? I am not following. The end goal is
that when the preprocessor runs MAX_LSM_COUNT is a constant number and
not an expression like (1 + 1 + 1) if you have ideas please share the
actual code.

- KP
>

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Tetsuo Handa]

On 2023/09/21 22:58, KP Singh wrote:
> On Thu, Sep 21, 2023 at 3:21 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>
>> On 2023/09/19 6:24, KP Singh wrote:
>>> 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.
>>
>> As a LKM-based LSM user, indirect function calls using a linked list have
>> an advantage which this series kills. There always is a situation where a
> 
> 
>> LSM cannot be built into vmlinux (and hence has to be loaded as a LKM-based
>> LSM) due to distributor's support policy. Therefore, honestly speaking,
>> I don't want LSM infrastructure to define the maximum number of "slots" or
>> "static calls"...
>>
> 
> Yeah, LSMs are not meant to be used from a kernel module. The data
> structure is actually __ro_after_init. So, I am not even sure how you
> are using it in kernel modules (unless you are patching this out).
> And, if you are really patching stuff to get your out of tree LSMs to
> work, then you might as well add your "custom" LSM config here or just
> override this count.

I'm using LKM-based LSM with any version between 2.6.0 and 6.6-rc2, without patching
__ro_after_init out. We can load LKM-based LSMs, without patching the original kernel.
And it seems to me that several proprietary security products for Linux are using
this trick, for LSMs for such products cannot be built into distributor's kernels...

----------
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-6.6.0-rc2+ root=/dev/sda1 ro vconsole.keymap=jp106 vconsole.font=latarcyrheb-sun16 security=none sysrq_always_enabled console=ttyS0,115200n8 console=tty0 LANG=en_US.UTF-8 init=/sbin/akari-init
(...snipped...)
[  147.238458] AKARI: 1.0.48   2023/05/27
[  147.244867] Access Keeping And Regulating Instrument registered.
[  147.261232] Calling /sbin/ccs-init to load policy. Please wait.
239 domains. 11807 ACL entries.
1938 KB used by policy.
[  147.768694] CCSecurity: 1.8.9   2021/04/01
[  147.768740] Mandatory Access Control activated.
----------

> 
> The performance benefits here outweigh the need for a completely
> unsupported use case.

LKM-based LSMs are not officially supported since 2.6.24. But people need LKM-based LSMs.
It is very sad that the LSM community is trying to lock out out of tree LSMs
( https://lkml.kernel.org/r/ec37cd2f-24ee-3273-c253-58d480569117@I-love.SAKURA.ne.jp ).
The LSM interface is a common property for *all* Linux users.

I'm not objecting the performance benefits by replacing with static calls.
I'm not happy that the LSM community ignores the Torvald's comment at https://lkml.org/lkml/2007/10/1/192
and does not listen to minority's voices.

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

On Fri, Sep 22, 2023 at 1:25 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/09/21 22:58, KP Singh wrote:
> > On Thu, Sep 21, 2023 at 3:21 PM Tetsuo Handa
> > <penguin-kernel@i-love.sakura.ne.jp> wrote:
> >>
> >> On 2023/09/19 6:24, KP Singh wrote:
> >>> 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.
> >>
> >> As a LKM-based LSM user, indirect function calls using a linked list have
> >> an advantage which this series kills. There always is a situation where a
> >
> >
> >> LSM cannot be built into vmlinux (and hence has to be loaded as a LKM-based
> >> LSM) due to distributor's support policy. Therefore, honestly speaking,
> >> I don't want LSM infrastructure to define the maximum number of "slots" or
> >> "static calls"...
> >>
> >
> > Yeah, LSMs are not meant to be used from a kernel module. The data
> > structure is actually __ro_after_init. So, I am not even sure how you
> > are using it in kernel modules (unless you are patching this out).
> > And, if you are really patching stuff to get your out of tree LSMs to
> > work, then you might as well add your "custom" LSM config here or just
> > override this count.
>
> I'm using LKM-based LSM with any version between 2.6.0 and 6.6-rc2, without patching
> __ro_after_init out. We can load LKM-based LSMs, without patching the original kernel.

Then __ro_after_init is broken in your tree and you are missing some patches.

> And it seems to me that several proprietary security products for Linux are using
> this trick, for LSMs for such products cannot be built into distributor's kernels...
>
> ----------
> [    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-6.6.0-rc2+ root=/dev/sda1 ro vconsole.keymap=jp106 vconsole.font=latarcyrheb-sun16 security=none sysrq_always_enabled console=ttyS0,115200n8 console=tty0 LANG=en_US.UTF-8 init=/sbin/akari-init
> (...snipped...)
> [  147.238458] AKARI: 1.0.48   2023/05/27
> [  147.244867] Access Keeping And Regulating Instrument registered.
> [  147.261232] Calling /sbin/ccs-init to load policy. Please wait.
> 239 domains. 11807 ACL entries.
> 1938 KB used by policy.
> [  147.768694] CCSecurity: 1.8.9   2021/04/01
> [  147.768740] Mandatory Access Control activated.
> ----------
>
> >
> > The performance benefits here outweigh the need for a completely
> > unsupported use case.
>
> LKM-based LSMs are not officially supported since 2.6.24. But people need LKM-based LSMs.
> It is very sad that the LSM community is trying to lock out out of tree LSMs
> ( https://lkml.kernel.org/r/ec37cd2f-24ee-3273-c253-58d480569117@I-love.SAKURA.ne.jp ).
> The LSM interface is a common property for *all* Linux users.

Again, I don't understand how this locks out out-of-tree LSMs. One can
go and patch static calls the same way one hacked around by directly
adding stuff to the security_hook_heads. I am not going to suggest any
hacks here but there are pretty obvious solutions out there.;

My recommendation would be to use BPF LSM for any custom MAC policy
logic. That's the whole goal of the BPF LSM is to safely enable these
use cases without relying on LSM internals and hacks.

- KP

>
> I'm not objecting the performance benefits by replacing with static calls.
> I'm not happy that the LSM community ignores the Torvald's comment at https://lkml.org/lkml/2007/10/1/192
> and does not listen to minority's voices.
>

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Tetsuo Handa]

On 2023/09/22 23:45, KP Singh wrote:
>> I'm using LKM-based LSM with any version between 2.6.0 and 6.6-rc2, without patching
>> __ro_after_init out. We can load LKM-based LSMs, without patching the original kernel.
> 
> Then __ro_after_init is broken in your tree and you are missing some patches.

This fact applies to vanilla upstream kernel tree; __ro_after_init is not broken and
some patches are not missing. See https://akari.osdn.jp/1.0/chapter-3.html.en for details.



>>>
>>> The performance benefits here outweigh the need for a completely
>>> unsupported use case.
>>
>> LKM-based LSMs are not officially supported since 2.6.24. But people need LKM-based LSMs.
>> It is very sad that the LSM community is trying to lock out out of tree LSMs
>> ( https://lkml.kernel.org/r/ec37cd2f-24ee-3273-c253-58d480569117@I-love.SAKURA.ne.jp ).
>> The LSM interface is a common property for *all* Linux users.
> 
> Again, I don't understand how this locks out out-of-tree LSMs. One can
> go and patch static calls the same way one hacked around by directly
> adding stuff to the security_hook_heads. I am not going to suggest any
> hacks here but there are pretty obvious solutions out there.;

The change that locks out out-of-tree LSMs (regardless of whether that LSM is LKM-based LSM
or not) is a series including "[PATCH v15 01/11] LSM: Identify modules by more than name".

I was not pushing LKM-based LSM because the LSM community wanted to make it possible to
enable arbitrary combinations (e.g. enabling selinux and smack at the same time) before
making it possible to use LKM-based LSMs.

According to https://marc.info/?l=linux-security-module&m=123232076329805 (Jan 2009),
Casey said that "SELinux and Smack should never be stacked in the same kernel.".
I'm personally wondering how many users will enable selinux and smack at the same time.
But in that post, Casey also said "You could revive the notion of loadable modules
while you're at it." while implementing LSM Multiplexer LSM.

According to https://marc.info/?l=linux-security-module&m=133055410107878 (Feb 2012),
Casey said that support for multiple concurrent LSMs should be able to handle
loadable/unloadable LSMs.
The reason for removing unload support was that no in-tree users needed it, and
out of tree use-cases are generally not supported in mainline. That is, when the
LSM interface became static, the LSM community was not seeing the reality.
I don't think that rmmod support for LKM-based LSMs is needed, but I believe that
insmod support for LKM-based LSMs is needed.

According to https://lkml.kernel.org/r/50ABE354.1040407@schaufler-ca.com (Nov 2012),
Casey said that reintroducing LSMs as loadable modules is a work for another day
and a separate battle to fight.

These postings (just picked up from LSM mailing list archives matching keyword "loadable"
and sent from Casey) indicate that the LSM community was not making changes that forever
makes LKM-based LSMs impossible.

Finally, pasting Casey's message (Feb 2016) here (because the archive did not find this post):

  From: Casey Schaufler <casey@schaufler-ca.com>
  Subject: Re: LSM as a kernel module
  Date: Mon, 22 Feb 2016 10:17:26 -0800
  Message-ID: <56CB50B6.6060702@schaufler-ca.com>
  To: Roman Kubiak <r.kubiak@samsung.com>, linux-security-module@vger.kernel.org

  On 2/22/2016 5:37 AM, Roman Kubiak wrote:
  > I just wanted to make sure that it's not possible and is not planned in the future
  > to have LSM modules loaded as .ko kernel modules. Is that true for now and the far/near future ?
  >
  > best regards
  
  Tetsuo Handa is holding out hope for loadable security modules*.
  The work I've been doing on module stacking does not include
  support for loadable modules, but I've committed to not making
  it impossible. There has never really been a major issue with
  loading a security module, although there are a host of minor
  ones. The big problem is unloading the module and cleaning up
  properly.
  
  Near term I believe that you can count on not having to worry
  about dynamically loadable security modules. At some point in
  the future we may have an important use case, but I don't see
  that until before some time in the 20s.
  
  So now I'm curious. What are you up to that would be spoiled
  by loadable security modules?
  
  
  ---
  * The original name for the infrastructure was indeed
    "Loadable Security Modules". The memory management and
    security policy implications resulted in steadily
    diminishing support for any sort of dynamic configuration.
    It wasn't long before "Loadable" became "Linux".

But while I was waiting for "make it possible to enable arbitrary combinations" change,
the LSM community started making changes (such as defining the maximum number of "slots"
or "static calls" based on all LSMs are built into vmlinux) that violate Casey's promise.

As a reminder to tell that I still want to make LKM-based LSM officially supported again,
I'm responding to changes (like this patch) that are based on "any LSM must be built into
vmlinux". Please be careful not to make changes that forever make LKM-based LSMs impossible.



> 
> My recommendation would be to use BPF LSM for any custom MAC policy
> logic. That's the whole goal of the BPF LSM is to safely enable these
> use cases without relying on LSM internals and hacks.

I'm fine if you can reimplement TOMOYO (or AKARI or CaitSith) using BPF LSM.
Since BPF has many limitations, not every custom MAC policy can be implemented using BPF.

The need to insmod LKM-based LSMs will remain because the LSM community will not accept
whatever LSMs (that are publicly available) and the Linux distributors will not build
whatever LSMs (that are publicly available) into their vmlinux.

But "LSM: Identify modules by more than name" is the worst change because that change
locks out any publicly available out of tree LSMs, far away from allowing LKM-based LSMs.

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

On Sat, Sep 23, 2023 at 8:57 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/09/22 23:45, KP Singh wrote:
> >> I'm using LKM-based LSM with any version between 2.6.0 and 6.6-rc2, without patching
> >> __ro_after_init out. We can load LKM-based LSMs, without patching the original kernel.
> >
> > Then __ro_after_init is broken in your tree and you are missing some patches.
>
> This fact applies to vanilla upstream kernel tree; __ro_after_init is not broken and
> some patches are not missing. See https://akari.osdn.jp/1.0/chapter-3.html.en for details.
>

You are trying to use an unexported symbol from the module with lots
of hackery to write to be supported and bring it up in a discussion?
Good luck!

Regardless, if what you are doing really works after
https://lore.kernel.org/all/20200107133154.588958-1-omosnace@redhat.com,
then we need to fix this as the security_hook_heads should be
immutable after boot. I tried a build where the symbols are exported
and sure enough the module is unable to write to it. So, either your
kernel has the old CONFIG_SECURITY_HOOKS_WRITABLE, or it should
ideally fail with something like:

[   23.990387] kernel tried to execute NX-protected page - exploit
attempt? (uid: 0)
[   23.996796] BUG: unable to handle page fault for address: ffffffff83adf270
[   23.997433] #PF: supervisor instruction fetch in kernel mode
[   23.997936] #PF: error_code(0x0011) - permissions violation
[   23.998416] PGD 3247067 P4D 3247067 PUD 3248063 PMD 100b9e063 PTE
8000000003adf163
[   23.999069] Oops: 0011 [#1] PREEMPT SMP NOPTI
[   23.999445] CPU: 0 PID: 302 Comm: insmod Tainted: G           O
  6.6.0-rc2-next-20230921-dirty #13
[   24.000230] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[   24.001024] RIP: 0010:security_add_hooks+0x0/0xa0

If this is not happening, then it's a bug and you chose to report it.

>
>
> >>>
> >>> The performance benefits here outweigh the need for a completely
> >>> unsupported use case.
> >>
> >> LKM-based LSMs are not officially supported since 2.6.24. But people need LKM-based LSMs.
> >> It is very sad that the LSM community is trying to lock out out of tree LSMs
> >> ( https://lkml.kernel.org/r/ec37cd2f-24ee-3273-c253-58d480569117@I-love.SAKURA.ne.jp ).
> >> The LSM interface is a common property for *all* Linux users.
> >
> > Again, I don't understand how this locks out out-of-tree LSMs. One can
> > go and patch static calls the same way one hacked around by directly
> > adding stuff to the security_hook_heads. I am not going to suggest any
> > hacks here but there are pretty obvious solutions out there.;
>
> The change that locks out out-of-tree LSMs (regardless of whether that LSM is LKM-based LSM
> or not) is a series including "[PATCH v15 01/11] LSM: Identify modules by more than name".

This does not belong here, please stop cross posting stuff.

>
> I was not pushing LKM-based LSM because the LSM community wanted to make it possible to
> enable arbitrary combinations (e.g. enabling selinux and smack at the same time) before
> making it possible to use LKM-based LSMs.
>
> According to https://marc.info/?l=linux-security-module&m=123232076329805 (Jan 2009),
> Casey said that "SELinux and Smack should never be stacked in the same kernel.".
> I'm personally wondering how many users will enable selinux and smack at the same time.
> But in that post, Casey also said "You could revive the notion of loadable modules
> while you're at it." while implementing LSM Multiplexer LSM.
>
> According to https://marc.info/?l=linux-security-module&m=133055410107878 (Feb 2012),
> Casey said that support for multiple concurrent LSMs should be able to handle
> loadable/unloadable LSMs.
> The reason for removing unload support was that no in-tree users needed it, and
> out of tree use-cases are generally not supported in mainline. That is, when the
> LSM interface became static, the LSM community was not seeing the reality.
> I don't think that rmmod support for LKM-based LSMs is needed, but I believe that
> insmod support for LKM-based LSMs is needed.
>
> According to https://lkml.kernel.org/r/50ABE354.1040407@schaufler-ca.com (Nov 2012),
> Casey said that reintroducing LSMs as loadable modules is a work for another day
> and a separate battle to fight.
>
> These postings (just picked up from LSM mailing list archives matching keyword "loadable"
> and sent from Casey) indicate that the LSM community was not making changes that forever
> makes LKM-based LSMs impossible.
>
> Finally, pasting Casey's message (Feb 2016) here (because the archive did not find this post):
>
>   From: Casey Schaufler <casey@schaufler-ca.com>
>   Subject: Re: LSM as a kernel module
>   Date: Mon, 22 Feb 2016 10:17:26 -0800
>   Message-ID: <56CB50B6.6060702@schaufler-ca.com>
>   To: Roman Kubiak <r.kubiak@samsung.com>, linux-security-module@vger.kernel.org
>
>   On 2/22/2016 5:37 AM, Roman Kubiak wrote:
>   > I just wanted to make sure that it's not possible and is not planned in the future
>   > to have LSM modules loaded as .ko kernel modules. Is that true for now and the far/near future ?
>   >
>   > best regards
>
>   Tetsuo Handa is holding out hope for loadable security modules*.
>   The work I've been doing on module stacking does not include
>   support for loadable modules, but I've committed to not making
>   it impossible. There has never really been a major issue with
>   loading a security module, although there are a host of minor
>   ones. The big problem is unloading the module and cleaning up
>   properly.
>
>   Near term I believe that you can count on not having to worry
>   about dynamically loadable security modules. At some point in
>   the future we may have an important use case, but I don't see
>   that until before some time in the 20s.
>
>   So now I'm curious. What are you up to that would be spoiled
>   by loadable security modules?
>
>
>   ---
>   * The original name for the infrastructure was indeed
>     "Loadable Security Modules". The memory management and
>     security policy implications resulted in steadily
>     diminishing support for any sort of dynamic configuration.
>     It wasn't long before "Loadable" became "Linux".
>
> But while I was waiting for "make it possible to enable arbitrary combinations" change,
> the LSM community started making changes (such as defining the maximum number of "slots"
> or "static calls" based on all LSMs are built into vmlinux) that violate Casey's promise.
>
> As a reminder to tell that I still want to make LKM-based LSM officially supported again,
> I'm responding to changes (like this patch) that are based on "any LSM must be built into
> vmlinux". Please be careful not to make changes that forever make LKM-based LSMs impossible.
>
>
>
> >
> > My recommendation would be to use BPF LSM for any custom MAC policy
> > logic. That's the whole goal of the BPF LSM is to safely enable these
> > use cases without relying on LSM internals and hacks.
>
> I'm fine if you can reimplement TOMOYO (or AKARI or CaitSith) using BPF LSM.
> Since BPF has many limitations, not every custom MAC policy can be implemented using BPF.

Please stop making generic statements, either be explicit about your
understanding of the limitations or don't claim them without evidence.

- KP

>
> The need to insmod LKM-based LSMs will remain because the LSM community will not accept
> whatever LSMs (that are publicly available) and the Linux distributors will not build
> whatever LSMs (that are publicly available) into their vmlinux.
>
> But "LSM: Identify modules by more than name" is the worst change because that change
> locks out any publicly available out of tree LSMs, far away from allowing LKM-based LSMs.
>

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Tetsuo Handa]

On 2023/09/24 1:06, KP Singh wrote:
>> I was not pushing LKM-based LSM because the LSM community wanted to make it possible to
>> enable arbitrary combinations (e.g. enabling selinux and smack at the same time) before
>> making it possible to use LKM-based LSMs.
(...snipped...)
>> As a reminder to tell that I still want to make LKM-based LSM officially supported again,
>> I'm responding to changes (like this patch) that are based on "any LSM must be built into
>> vmlinux". Please be careful not to make changes that forever make LKM-based LSMs impossible.

You did not recognize the core chunk of this post. :-(

It is Casey's commitment that the LSM infrastructure will not forbid LKM-based LSMs.
We will start allowing LKM-based LSMs. But it is not clear how we can make it possible to
allow LKM-based LSMs.

Suppose you replace the linked list (which does not need to limit number of LSMs activated)
with static calls (which limits number of LSMs activated, due to use of compile-time determined
MAX_LSM_COUNT value at

  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;

. If NAME[MAX_LSM_COUNT] were allocated like

  NAME = kcalloc(sizeof(struct lsm_static_call), number_of_max_lsms_to_activate, GFP_KERNEL | __GFP_NOFAIL);

(where number_of_max_lsms_to_activate is controlled using kernel command line parameter)
rater than

  struct lsm_static_call NAME[MAX_LSM_COUNT];

, it is easy to allow LKM-based LSMs.

But if NAME[MAX_LSM_COUNT] is allocated in a way which cannot be expanded using kernel
command line parameter (this is what "[PATCH v3 2/5] security: Count the LSMs enabled
at compile time" does), how can the LKM-based LSMs be registered? Introduce a LSM module
which revives the linked list and registration function (which this patch tried to remove) ?
If yes, do we want to use

  #define LSM_HOOK(RET, DEFAULT, NAME, ...) \

for built-in LSMs and a different macro for LKM-based LSMs?

Do we want/agree to manage two different set of macros/functions only for handling
both built-in LSMs and loadable LSMs?

That's a lot of complication, compared to temporarily making the security_hook_heads writable.



> You are trying to use an unexported symbol from the module with lots
> of hackery to write to be supported and bring it up in a discussion?
> Good luck!

Currently LKM-based LSMs is not officially supported. But LKM-based LSMs will become
officially supported in the future. Therefore, I respond to any attempt which tries
to make LKM-based LSMs impossible.

> 
> Regardless, if what you are doing really works after
> https://lore.kernel.org/all/20200107133154.588958-1-omosnace@redhat.com,
> then we need to fix this as the security_hook_heads should be
> immutable after boot.

You should learn how the __ro_after_init works. I will throw NACK if someone tries
to add an exception to __ro_after_init handling before we make it possible to allow
LKM-based LSMs.

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

On Mon, Sep 25, 2023 at 1:03 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/09/24 1:06, KP Singh wrote:
> >> I was not pushing LKM-based LSM because the LSM community wanted to make it possible to
> >> enable arbitrary combinations (e.g. enabling selinux and smack at the same time) before
> >> making it possible to use LKM-based LSMs.
> (...snipped...)
> >> As a reminder to tell that I still want to make LKM-based LSM officially supported again,
> >> I'm responding to changes (like this patch) that are based on "any LSM must be built into
> >> vmlinux". Please be careful not to make changes that forever make LKM-based LSMs impossible.
>
> You did not recognize the core chunk of this post. :-(
>
> It is Casey's commitment that the LSM infrastructure will not forbid LKM-based LSMs.
> We will start allowing LKM-based LSMs. But it is not clear how we can make it possible to
> allow LKM-based LSMs.

I think this needs to be discussed if and when we allow LKM based LSMs.

>
> Suppose you replace the linked list (which does not need to limit number of LSMs activated)
> with static calls (which limits number of LSMs activated, due to use of compile-time determined
> MAX_LSM_COUNT value at
>
>   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;
>
> . If NAME[MAX_LSM_COUNT] were allocated like
>
>   NAME = kcalloc(sizeof(struct lsm_static_call), number_of_max_lsms_to_activate, GFP_KERNEL | __GFP_NOFAIL);
>
> (where number_of_max_lsms_to_activate is controlled using kernel command line parameter)
> rater than
>
>   struct lsm_static_call NAME[MAX_LSM_COUNT];
>
> , it is easy to allow LKM-based LSMs.
>

One needs to know MAX_LSM_COUNT at compile time (not via kernel
command line), I really suggest you try out your suggestions before
posting them. I had explained this to you earlier, you still chose to
ignore and keep suggesting stuff that does not work.

https://lore.kernel.org/bpf/CACYkzJ7Dn=W1Kd5M_bXOzoomzdjMXBoEZZo5k=cgQ4R6f5G+vw@mail.gmail.com/

It is used in the preprocessor to generate the static calls, it cannot
come from the command line.

> But if NAME[MAX_LSM_COUNT] is allocated in a way which cannot be expanded using kernel
> command line parameter (this is what "[PATCH v3 2/5] security: Count the LSMs enabled
> at compile time" does), how can the LKM-based LSMs be registered? Introduce a LSM module
> which revives the linked list and registration function (which this patch tried to remove) ?
> If yes, do we want to use
>
>   #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
>
> for built-in LSMs and a different macro for LKM-based LSMs?
>
> Do we want/agree to manage two different set of macros/functions only for handling
> both built-in LSMs and loadable LSMs?

We will see when this happens. I don't think it's a difficult problem
and there are many ways to implement this:

* Add a new slot(s) for modular LSMs (One can add up to N fast modular LSMs)
* Fallback to a linked list for modular LSMs, that's not a complexity.
There are serious performance gains and I think it's a fair trade-off.
This isn't even complex.

Now, this patch and the patch that makes security_hook_heads
__ro_after_init by removing CONFIG_SECURITY_HOOKS_WRITABLE breaks your
hack. But that hack (https://akari.osdn.jp/1.0/chapter-3.html.en) is
unsupported.

>
> That's a lot of complication, compared to temporarily making the security_hook_heads writable.

No, that's not complicated.  All I can say is, when the time comes,
and if the community agrees on LMK based modules, this patch won't
make it any difficult or easy. There are many implementations, even
this patch, that can provide LKM based LSMs API (but hacks will be
hard, sure!)


- KP

>
>
>
> > You are trying to use an unexported symbol from the module with lots
> > of hackery to write to be supported and bring it up in a discussion?
> > Good luck!
>
> Currently LKM-based LSMs is not officially supported. But LKM-based LSMs will become
> officially supported in the future. Therefore, I respond to any attempt which tries
> to make LKM-based LSMs impossible.
>
> >
> > Regardless, if what you are doing really works after
> > https://lore.kernel.org/all/20200107133154.588958-1-omosnace@redhat.com,
> > then we need to fix this as the security_hook_heads should be
> > immutable after boot.
>
> You should learn how the __ro_after_init works. I will throw NACK if someone tries
> to add an exception to __ro_after_init handling before we make it possible to allow
> LKM-based LSMs.
>

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Tetsuo Handa]

On 2023/09/25 20:22, KP Singh wrote:
>> It is Casey's commitment that the LSM infrastructure will not forbid LKM-based LSMs.
>> We will start allowing LKM-based LSMs. But it is not clear how we can make it possible to
>> allow LKM-based LSMs.
> 
> I think this needs to be discussed if and when we allow LKM based LSMs.

It is *now* (i.e. before your proposal is accepted) that we need to discuss.

> One needs to know MAX_LSM_COUNT at compile time (not via kernel
> command line), I really suggest you try out your suggestions before
> posting them. I had explained this to you earlier, you still chose to
> ignore and keep suggesting stuff that does not work.

Your proposal needs to know MAX_LSM_COUNT at compile time, that's why
we need to discuss now.

> We will see when this happens. I don't think it's a difficult problem
> and there are many ways to implement this:
> 
> * Add a new slot(s) for modular LSMs (One can add up to N fast modular LSMs)
> * Fallback to a linked list for modular LSMs, that's not a complexity.
> There are serious performance gains and I think it's a fair trade-off.
> This isn't even complex.

That won't help at all. You became so blind because what you want to use (i.e.
SELinux and BPF) are already supported by Linux distributors. The reason I'm
insisting on supporting LKM-based LSMs is that Linux distributors cannot afford
supporting minor LSMs.

Dave Chinner said

  Downstream distros support all sorts of out of tree filesystems loaded
  via kernel modules

at https://lkml.kernel.org/r/ZQo94mCzV7hOrVkh@dread.disaster.area , and e.g.
antivirus software vendors use out of tree filesystems loaded via kernel
modules (because neither the upstream kernel community nor the Linux distributors
can afford supporting out of tree filesystems used by antivirus software vendors).

If Linux distributors decide "we don't allow loading out of tree filesystems
via kernel modules because we can't support", that's the end of the world for
such filesystems.

What I'm saying is nothing but s/filesystem/LSM/g .
If Linux distributors decide "we don't allow loading out of tree LSMs
via kernel modules because we can't support", that's the end of the world for
LKM-based LSMs.

The mechanism which makes LKM-based LSMs possible must not be disabled by
the person/organization who builds the vmlinux.

You might still say that "You can build your vmlinux and distribute it", but
that is also impossible in practice. Some device drivers are meant to be loaded
for Linux distribution's prebuilt kernels. Also, debuginfo package is needed for
analyzing vmcore. Building vmlinux and distributing it is not practical without
becoming a well-known Linux distributors enough to get out-of-tree device drivers
being prebuilt (such as Red Hat).

Again, you are so blind.

> Now, this patch and the patch that makes security_hook_heads
> __ro_after_init by removing CONFIG_SECURITY_HOOKS_WRITABLE breaks your
> hack.

Like I demonstrated at https://lkml.kernel.org/r/cc8e16bb-5083-01da-4a77-d251a76dc8ff@I-love.SAKURA.ne.jp ,
removing CONFIG_SECURITY_HOOKS_WRITABLE does not break my hack.

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

On Sun, Oct 1, 2023 at 12:51 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/09/25 20:22, KP Singh wrote:
> >> It is Casey's commitment that the LSM infrastructure will not forbid LKM-based LSMs.
> >> We will start allowing LKM-based LSMs. But it is not clear how we can make it possible to
> >> allow LKM-based LSMs.
> >
> > I think this needs to be discussed if and when we allow LKM based LSMs.
>
> It is *now* (i.e. before your proposal is accepted) that we need to discuss.
>
> > One needs to know MAX_LSM_COUNT at compile time (not via kernel
> > command line), I really suggest you try out your suggestions before
> > posting them. I had explained this to you earlier, you still chose to
> > ignore and keep suggesting stuff that does not work.
>
> Your proposal needs to know MAX_LSM_COUNT at compile time, that's why
> we need to discuss now.

People already mention that you seem to deliberately ignore advice
given to you and continue with your own narrative. Here's my last
attempt to explain things to you:

You are conflating two use cases, built-in out-of-tree LSMS and
modular LSMs. However, the proposed changes block neither of the use
cases.

* For modules that are out-of-tree but compiled into the kernel, they
can just modify the MAX_LSM_COUNT
* For dynamically loadable LSMs, you anyways want a separate
security_hook_heads. The __ro_after_init should not be relaxed on the
existing security_hook_heads to prevent any memory corruption from
overriding LSM callbacks, this lowers the existing security posture.
And then, in the call_int_hook and security_for_each_hook you can
iterate over both the static call slots.

^^ I said the above multiple times but you ignored all of it!

- KP

>
> > We will see when this happens. I don't think it's a difficult problem
> > and there are many ways to implement this:
> >
> > * Add a new slot(s) for modular LSMs (One can add up to N fast modular LSMs)
> > * Fallback to a linked list for modular LSMs, that's not a complexity.
> > There are serious performance gains and I think it's a fair trade-off.
> > This isn't even complex.
>
> That won't help at all. You became so blind because what you want to use (i.e.
> SELinux and BPF) are already supported by Linux distributors. The reason I'm
> insisting on supporting LKM-based LSMs is that Linux distributors cannot afford
> supporting minor LSMs.
>
> Dave Chinner said
>
>   Downstream distros support all sorts of out of tree filesystems loaded
>   via kernel modules
>
> at https://lkml.kernel.org/r/ZQo94mCzV7hOrVkh@dread.disaster.area , and e.g.
> antivirus software vendors use out of tree filesystems loaded via kernel
> modules (because neither the upstream kernel community nor the Linux distributors
> can afford supporting out of tree filesystems used by antivirus software vendors).
>
> If Linux distributors decide "we don't allow loading out of tree filesystems
> via kernel modules because we can't support", that's the end of the world for
> such filesystems.
>
> What I'm saying is nothing but s/filesystem/LSM/g .
> If Linux distributors decide "we don't allow loading out of tree LSMs
> via kernel modules because we can't support", that's the end of the world for
> LKM-based LSMs.
>
> The mechanism which makes LKM-based LSMs possible must not be disabled by
> the person/organization who builds the vmlinux.
>
> You might still say that "You can build your vmlinux and distribute it", but
> that is also impossible in practice. Some device drivers are meant to be loaded
> for Linux distribution's prebuilt kernels. Also, debuginfo package is needed for
> analyzing vmcore. Building vmlinux and distributing it is not practical without
> becoming a well-known Linux distributors enough to get out-of-tree device drivers
> being prebuilt (such as Red Hat).
>
> Again, you are so blind.
>
> > Now, this patch and the patch that makes security_hook_heads
> > __ro_after_init by removing CONFIG_SECURITY_HOOKS_WRITABLE breaks your
> > hack.
>
> Like I demonstrated at https://lkml.kernel.org/r/cc8e16bb-5083-01da-4a77-d251a76dc8ff@I-love.SAKURA.ne.jp ,
> removing CONFIG_SECURITY_HOOKS_WRITABLE does not break my hack.
>

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Casey Schaufler]

On 10/1/2023 3:51 AM, Tetsuo Handa wrote:
> On 2023/09/25 20:22, KP Singh wrote:
>>> It is Casey's commitment that the LSM infrastructure will not forbid LKM-based LSMs.
>>> We will start allowing LKM-based LSMs. But it is not clear how we can make it possible to
>>> allow LKM-based LSMs.
>> I think this needs to be discussed if and when we allow LKM based LSMs.
> It is *now* (i.e. before your proposal is accepted) that we need to discuss.
>
>> One needs to know MAX_LSM_COUNT at compile time (not via kernel
>> command line), I really suggest you try out your suggestions before
>> posting them. I had explained this to you earlier, you still chose to
>> ignore and keep suggesting stuff that does not work.
> Your proposal needs to know MAX_LSM_COUNT at compile time, that's why
> we need to discuss now.
>
>> We will see when this happens. I don't think it's a difficult problem
>> and there are many ways to implement this:
>>
>> * Add a new slot(s) for modular LSMs (One can add up to N fast modular LSMs)
>> * Fallback to a linked list for modular LSMs, that's not a complexity.
>> There are serious performance gains and I think it's a fair trade-off.
>> This isn't even complex.
> That won't help at all.

This is exactly the solution I have been contemplating since this
discussion began. It will address the bulk of the issue. I'm almost
mad/crazy enough to produce the patch to demonstrate it. Almost.
There are still a bunch of details (e.g. shared blobs) that it doesn't
address. On the other hand, your memory management magic doesn't
address those issues either.

>  You became so blind because what you want to use (i.e.
> SELinux and BPF) are already supported by Linux distributors. The reason I'm
> insisting on supporting LKM-based LSMs is that Linux distributors cannot afford
> supporting minor LSMs.
>
> Dave Chinner said
>
>   Downstream distros support all sorts of out of tree filesystems loaded
>   via kernel modules
>
> at https://lkml.kernel.org/r/ZQo94mCzV7hOrVkh@dread.disaster.area , and e.g.
> antivirus software vendors use out of tree filesystems loaded via kernel
> modules (because neither the upstream kernel community nor the Linux distributors
> can afford supporting out of tree filesystems used by antivirus software vendors).
>
> If Linux distributors decide "we don't allow loading out of tree filesystems
> via kernel modules because we can't support", that's the end of the world for
> such filesystems.
>
> What I'm saying is nothing but s/filesystem/LSM/g .
> If Linux distributors decide "we don't allow loading out of tree LSMs
> via kernel modules because we can't support", that's the end of the world for
> LKM-based LSMs.
>
> The mechanism which makes LKM-based LSMs possible must not be disabled by
> the person/organization who builds the vmlinux.
>
> You might still say that "You can build your vmlinux and distribute it", but
> that is also impossible in practice. Some device drivers are meant to be loaded
> for Linux distribution's prebuilt kernels. Also, debuginfo package is needed for
> analyzing vmcore. Building vmlinux and distributing it is not practical without
> becoming a well-known Linux distributors enough to get out-of-tree device drivers
> being prebuilt (such as Red Hat).
>
> Again, you are so blind.
>
>> Now, this patch and the patch that makes security_hook_heads
>> __ro_after_init by removing CONFIG_SECURITY_HOOKS_WRITABLE breaks your
>> hack.
> Like I demonstrated at https://lkml.kernel.org/r/cc8e16bb-5083-01da-4a77-d251a76dc8ff@I-love.SAKURA.ne.jp ,
> removing CONFIG_SECURITY_HOOKS_WRITABLE does not break my hack.
>

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Tetsuo Handa]

On 2023/10/02 0:00, Casey Schaufler wrote:
> On 10/1/2023 3:51 AM, Tetsuo Handa wrote:
>> On 2023/09/25 20:22, KP Singh wrote:
>>>> It is Casey's commitment that the LSM infrastructure will not forbid LKM-based LSMs.
>>>> We will start allowing LKM-based LSMs. But it is not clear how we can make it possible to
>>>> allow LKM-based LSMs.
>>> I think this needs to be discussed if and when we allow LKM based LSMs.
>> It is *now* (i.e. before your proposal is accepted) that we need to discuss.
>>
>>> One needs to know MAX_LSM_COUNT at compile time (not via kernel
>>> command line), I really suggest you try out your suggestions before
>>> posting them. I had explained this to you earlier, you still chose to
>>> ignore and keep suggesting stuff that does not work.
>> Your proposal needs to know MAX_LSM_COUNT at compile time, that's why
>> we need to discuss now.
>>
>>> We will see when this happens. I don't think it's a difficult problem
>>> and there are many ways to implement this:
>>>
>>> * Add a new slot(s) for modular LSMs (One can add up to N fast modular LSMs)
>>> * Fallback to a linked list for modular LSMs, that's not a complexity.
>>> There are serious performance gains and I think it's a fair trade-off.
>>> This isn't even complex.
>> That won't help at all.
> 
> This is exactly the solution I have been contemplating since this
> discussion began. It will address the bulk of the issue. I'm almost
> mad/crazy enough to produce the patch to demonstrate it. Almost.

Yes, please show us one. I'm fine if the mechanism which allows LKM-based LSMs
cannot be disabled via the kernel configuration options.

I really want a commitment that none of the LSM community objects revival of
LKM-based LSMs. I'm worrying that some of the LSM community objects revival of
LKM-based LSMs because adding extra slots and/or linked list is e.g. an overhead,
increases attack surface etc.

Let's consider the Microsoft Windows operating system. Many security vendors are
offering security software which can run without recompiling the Windows OS.

But what about Linux? Security vendors cannot trivially add a security mechanism
because LKM-based LSMs are not supported since 2.6.24. As a result, some chose
hijacking LSM hooks, and others chose overwriting system call tables.

The Linux kernel is there for providing what the user needs. What about the LSM
infrastructure? The LSM infrastructure is too much evolving towards in-tree and
built-in security mechanisms.

The consequence of such evolving will be "Limited Security Modes" where users cannot
use what they need. New ideas cannot be easily tried if rebuild of vmlinux is
inevitable, which will also prevent a breath of fresh ideas from reaching the LSM
community.

Never "discussed *if* we allow LKM based LSMs", for the LSM community cannot
afford accepting whatever LSMs and the Linux distributors cannot afford enabling
whatever LSMs.

I'm not speaking for the security vendors. I'm speaking from the point of view of
minority/out-of-tree users.

> There are still a bunch of details (e.g. shared blobs) that it doesn't
> address. On the other hand, your memory management magic doesn't
> address those issues either.

Security is always trial-and-error. Just give all Linux users chances to continue
trial-and-error. You don't need to forbid LKM-based LSMs just because blob management
is not addressed. Please open the LSM infrastructure to anyone.

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

On Mon, Oct 2, 2023 at 12:56 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/10/02 0:00, Casey Schaufler wrote:
> > On 10/1/2023 3:51 AM, Tetsuo Handa wrote:
> >> On 2023/09/25 20:22, KP Singh wrote:
> >>>> It is Casey's commitment that the LSM infrastructure will not forbid LKM-based LSMs.
> >>>> We will start allowing LKM-based LSMs. But it is not clear how we can make it possible to
> >>>> allow LKM-based LSMs.
> >>> I think this needs to be discussed if and when we allow LKM based LSMs.
> >> It is *now* (i.e. before your proposal is accepted) that we need to discuss.
> >>
> >>> One needs to know MAX_LSM_COUNT at compile time (not via kernel
> >>> command line), I really suggest you try out your suggestions before
> >>> posting them. I had explained this to you earlier, you still chose to
> >>> ignore and keep suggesting stuff that does not work.
> >> Your proposal needs to know MAX_LSM_COUNT at compile time, that's why
> >> we need to discuss now.
> >>
> >>> We will see when this happens. I don't think it's a difficult problem
> >>> and there are many ways to implement this:
> >>>
> >>> * Add a new slot(s) for modular LSMs (One can add up to N fast modular LSMs)
> >>> * Fallback to a linked list for modular LSMs, that's not a complexity.
> >>> There are serious performance gains and I think it's a fair trade-off.
> >>> This isn't even complex.
> >> That won't help at all.
> >
> > This is exactly the solution I have been contemplating since this
> > discussion began. It will address the bulk of the issue. I'm almost
> > mad/crazy enough to produce the patch to demonstrate it. Almost.
>
> Yes, please show us one. I'm fine if the mechanism which allows LKM-based LSMs
> cannot be disabled via the kernel configuration options.
>
> I really want a commitment that none of the LSM community objects revival of
> LKM-based LSMs. I'm worrying that some of the LSM community objects revival of
> LKM-based LSMs because adding extra slots and/or linked list is e.g. an overhead,
> increases attack surface etc.
>
> Let's consider the Microsoft Windows operating system. Many security vendors are
> offering security software which can run without recompiling the Windows OS.
>
> But what about Linux? Security vendors cannot trivially add a security mechanism
> because LKM-based LSMs are not supported since 2.6.24. As a result, some chose
> hijacking LSM hooks, and others chose overwriting system call tables.
>
> The Linux kernel is there for providing what the user needs. What about the LSM
> infrastructure? The LSM infrastructure is too much evolving towards in-tree and
> built-in security mechanisms.
>
> The consequence of such evolving will be "Limited Security Modes" where users cannot
> use what they need. New ideas cannot be easily tried if rebuild of vmlinux is
> inevitable, which will also prevent a breath of fresh ideas from reaching the LSM
> community.
>
> Never "discussed *if* we allow LKM based LSMs", for the LSM community cannot
> afford accepting whatever LSMs and the Linux distributors cannot afford enabling
> whatever LSMs.
>
> I'm not speaking for the security vendors. I'm speaking from the point of view of
> minority/out-of-tree users.
>
> > There are still a bunch of details (e.g. shared blobs) that it doesn't
> > address. On the other hand, your memory management magic doesn't
> > address those issues either.
>
> Security is always trial-and-error. Just give all Linux users chances to continue
> trial-and-error. You don't need to forbid LKM-based LSMs just because blob management
> is not addressed. Please open the LSM infrastructure to anyone.

It already is, the community is already using BPF LSM.

e.g. https://github.com/linux-lock/bpflock

>

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Tetsuo Handa]

On 2023/10/02 19:56, Tetsuo Handa wrote:
>> This is exactly the solution I have been contemplating since this
>> discussion began. It will address the bulk of the issue. I'm almost
>> mad/crazy enough to produce the patch to demonstrate it. Almost.
> 
> Yes, please show us one.

If "[PATCH v15 01/11] LSM: Identify modules by more than name" does not allow LKM-based
LSMs (which are likely out-of-tree) to have stable LSM ID values, lsm_list_modules()
provided by "[PATCH v15 05/11] LSM: Create lsm_list_modules system call" cannot report
stable string names. And "[PATCH v15 11/11] LSM: selftests for Linux Security Module
syscalls" cannot work on LKM-based LSMs.

Then, how are LKM-based LSMs activated? LKM-based LSMs can use LSM hooks but cannot use
(or show up in) lsm_get_self_attr()/lsm_set_self_attr()/lsm_list_modules() syscalls?
That looks quite strange, for the title of "[PATCH v15 01/11]" is not "LSM: Identify only
built-in and in-tree modules by more than name".

If you think about allowing LKM-based LSMs a bit, you will find that how can the current policy
be compatible. We cannot introduce lsm_get_self_attr()/lsm_set_self_attr()/lsm_list_modules()
syscalls without admitting stable LSM ID values being assigned to any publicly available LSMs.

Simple notification to the LSM community has to be the only requirement for assigning
stable LSM ID values. You should not distinguish in-tree and not-in-tree LSMs regarding
"[PATCH v15 00/11] LSM: Three basic syscalls". Otherwise, the attempt to introduce these
syscalls is a regression that will harm LKM-based LSMs.

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Casey Schaufler]

On 9/25/2023 4:03 AM, Tetsuo Handa wrote:
> On 2023/09/24 1:06, KP Singh wrote:
>>> I was not pushing LKM-based LSM because the LSM community wanted to make it possible to
>>> enable arbitrary combinations (e.g. enabling selinux and smack at the same time) before
>>> making it possible to use LKM-based LSMs.
> (...snipped...)
>>> As a reminder to tell that I still want to make LKM-based LSM officially supported again,
>>> I'm responding to changes (like this patch) that are based on "any LSM must be built into
>>> vmlinux". Please be careful not to make changes that forever make LKM-based LSMs impossible.
> You did not recognize the core chunk of this post. :-(
>
> It is Casey's commitment that the LSM infrastructure will not forbid LKM-based LSMs.

... And this code doesn't. I you want LKM based LSM support I suggest you
provide patches. If there is anything in the LSM infrastructure that you can't
work around I'll help work out how to do it. But I am not going to do it for
you, and I don't think anyone else is inclined to, either.

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Casey Schaufler]

On 9/22/2023 11:56 PM, Tetsuo Handa wrote:
> ...
> The change that locks out out-of-tree LSMs (regardless of whether that LSM is LKM-based LSM
> or not) is a series including "[PATCH v15 01/11] LSM: Identify modules by more than name".

Please supply patches for your implementation of loadable security modules.
This will provide a base from which we can work out what needs changed to
accommodate your needs. I have more than 20 years worth of projects that I
hope to get to before I would even start to work on loadable security modules.

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[Paul Moore]

On Fri, Sep 22, 2023 at 7:25 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2023/09/21 22:58, KP Singh wrote:
> > Yeah, LSMs are not meant to be used from a kernel module. The data
> > structure is actually __ro_after_init. So, I am not even sure how you
> > are using it in kernel modules (unless you are patching this out).
> > And, if you are really patching stuff to get your out of tree LSMs to
> > work, then you might as well add your "custom" LSM config here or just
> > override this count.
>
> I'm using LKM-based LSM with any version between 2.6.0 and 6.6-rc2, without patching
> __ro_after_init out. We can load LKM-based LSMs, without patching the original kernel.
> And it seems to me that several proprietary security products for Linux are using
> this trick, for LSMs for such products cannot be built into distributor's kernels...

...

> > The performance benefits here outweigh the need for a completely
> > unsupported use case.
>
> LKM-based LSMs are not officially supported since 2.6.24. But people need LKM-based LSMs.
> It is very sad that the LSM community is trying to lock out out of tree LSMs
> ( https://lkml.kernel.org/r/ec37cd2f-24ee-3273-c253-58d480569117@I-love.SAKURA.ne.jp ).
> The LSM interface is a common property for *all* Linux users.
>
> I'm not objecting the performance benefits by replacing with static calls.
> I'm not happy that the LSM community ignores the Torvald's comment at https://lkml.org/lkml/2007/10/1/192
> and does not listen to minority's voices.

Despite a previous comment that I was done engaging with Tetsuo on
this topic, I feel it is worth commenting here as there are a number
of people on the To/CC line that have likely not been following the
related discussion threads on the LSM list.

First and foremost I want to reiterate that the LSM community's first
priority are those LSMs which have been accepted and merged into the
upstream Linux kernel.  While I have no intention, or desire, to harm
out-of-tree LSMs, I stand firm that we should not compromise designs
for in-tree LSMs/functionality solely to benefit out-of-tree LSMs.  I
believe this is consistent, or at least compatible, with the general
Linux kernel community's stance on in-tree vs out-of-tree code.

The (relatively) newly proposed LSM syscalls have proven to be a
contentious topic between Tetsuo and the LSM community as a whole; I
won't rehash the arguments here, as they are all available on
lore.kernel.org (simply look for any threads that Tetsuo has been
involved in over the past several months) but we have discussed this
issue at great length and Tetsuo remains the only opposing opinion.
It was my hope that Tetsuo would respect the opinion of the upstream
LSM community, even if he didn't agree with the details.  After all,
this is how we move forward in cases where differing opinions cannot
all be accommodated in the code.

Unfortunately Tetsuo's continued and stubborn refusal to accept the
majority opinion has started to spill into other discussion threads,
disrupting the discussion there and twisting some of the core issues
to better fit his arguments.  Not only is this frustrating, it is
becoming rather disruptive.  My suggestion is to simply follow some
classic Internet advice and "don't feed the trolls".

As we discussed off-list (and in-person!) this week, I am generally
supportive of work that improves performance, but correctness will
always be my priority with maintainability a close second.  We have a
few more pressing issues at the LSM layer which are demanding my time
at the moment, but I do promise to come back to this issue/patchset as
these other high priority issues are resolved.

Thanks for your patience and understanding KP :)

-- 
paul-moore.com

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

On Fri, Sep 22, 2023 at 4:57 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Fri, Sep 22, 2023 at 7:25 AM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
> > On 2023/09/21 22:58, KP Singh wrote:
> > > Yeah, LSMs are not meant to be used from a kernel module. The data
> > > structure is actually __ro_after_init. So, I am not even sure how you
> > > are using it in kernel modules (unless you are patching this out).
> > > And, if you are really patching stuff to get your out of tree LSMs to
> > > work, then you might as well add your "custom" LSM config here or just
> > > override this count.
> >
> > I'm using LKM-based LSM with any version between 2.6.0 and 6.6-rc2, without patching
> > __ro_after_init out. We can load LKM-based LSMs, without patching the original kernel.
> > And it seems to me that several proprietary security products for Linux are using
> > this trick, for LSMs for such products cannot be built into distributor's kernels...
>
> ...
>
> > > The performance benefits here outweigh the need for a completely
> > > unsupported use case.
> >
> > LKM-based LSMs are not officially supported since 2.6.24. But people need LKM-based LSMs.
> > It is very sad that the LSM community is trying to lock out out of tree LSMs
> > ( https://lkml.kernel.org/r/ec37cd2f-24ee-3273-c253-58d480569117@I-love.SAKURA.ne.jp ).
> > The LSM interface is a common property for *all* Linux users.
> >
> > I'm not objecting the performance benefits by replacing with static calls.
> > I'm not happy that the LSM community ignores the Torvald's comment at https://lkml.org/lkml/2007/10/1/192
> > and does not listen to minority's voices.
>
> Despite a previous comment that I was done engaging with Tetsuo on
> this topic, I feel it is worth commenting here as there are a number
> of people on the To/CC line that have likely not been following the
> related discussion threads on the LSM list.
>
> First and foremost I want to reiterate that the LSM community's first
> priority are those LSMs which have been accepted and merged into the
> upstream Linux kernel.  While I have no intention, or desire, to harm
> out-of-tree LSMs, I stand firm that we should not compromise designs
> for in-tree LSMs/functionality solely to benefit out-of-tree LSMs.  I
> believe this is consistent, or at least compatible, with the general
> Linux kernel community's stance on in-tree vs out-of-tree code.
>
> The (relatively) newly proposed LSM syscalls have proven to be a
> contentious topic between Tetsuo and the LSM community as a whole; I
> won't rehash the arguments here, as they are all available on
> lore.kernel.org (simply look for any threads that Tetsuo has been
> involved in over the past several months) but we have discussed this
> issue at great length and Tetsuo remains the only opposing opinion.
> It was my hope that Tetsuo would respect the opinion of the upstream
> LSM community, even if he didn't agree with the details.  After all,
> this is how we move forward in cases where differing opinions cannot
> all be accommodated in the code.
>
> Unfortunately Tetsuo's continued and stubborn refusal to accept the
> majority opinion has started to spill into other discussion threads,
> disrupting the discussion there and twisting some of the core issues
> to better fit his arguments.  Not only is this frustrating, it is
> becoming rather disruptive.  My suggestion is to simply follow some
> classic Internet advice and "don't feed the trolls".
>
> As we discussed off-list (and in-person!) this week, I am generally
> supportive of work that improves performance, but correctness will
> always be my priority with maintainability a close second.  We have a
> few more pressing issues at the LSM layer which are demanding my time
> at the moment, but I do promise to come back to this issue/patchset as
> these other high priority issues are resolved.
>
> Thanks for your patience and understanding KP :)

Thank you for the context Paul, this explains a lot!


>
> --
> paul-moore.com

Re: [PATCH v3 2/5] security: Count the LSMs enabled at compile time

[KP Singh]

On Thu, Sep 21, 2023 at 3:21 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/09/19 6:24, KP Singh wrote:
> > 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.
>
> As a LKM-based LSM user, indirect function calls using a linked list have
> an advantage which this series kills. There always is a situation where a
> LSM cannot be built into vmlinux (and hence has to be loaded as a LKM-based
> LSM) due to distributor's support policy. Therefore, honestly speaking,
> I don't want LSM infrastructure to define the maximum number of "slots" or
> "static calls"...
>
> >
> > Without this one would need to generate static calls for (number of
> > possible LSMs * number of LSM hooks) which ends up being quite wasteful
> > especially when some LSMs are not compiled into the kernel.
>
> I can't interpret "number of possible LSMs * number of LSM hooks" part.
> Is this tokenized as "number of possible LSMs" (an integer) * (multipled by)
> "number of LSM hooks" (an integer) ? But the next patch includes
>

The tokenization is in the name of the static call slots. you cannot
have __SCT__lsm_static_call_bprm_check_security_1+1+1 it's not a valid
name. You may want to build security/security.i to see what's going on
(and actually try disabling some of the DEFINE_STATIC_CALL macros to
reduce further expansion of macros.

>   struct lsm_static_calls_table {
>   #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
>                 struct lsm_static_call NAME[MAX_LSM_COUNT];

Each LSM that is compiled in the kernel can theoretically register a
callback, so we add MAX_LSM_COUNT slots. Now the word "possible"
because one may compile the LSM but not choose to enable it with the
lsm= parameter.


>         #include <linux/lsm_hook_defs.h>
>         #undef LSM_HOOK
>   } __randomize_layout;
>
> which seems to me that lsm_static_calls_table will get "number of possible
> LSMs" static calls for each LSM hook defined in linux/lsm_hook_defs.h .
> How did this patch help reducing static calls? What does "possible LSMs" mean?

If the kernel is compiled only with CONFIG_BPF_LSM, CONFIG_SELINUX and
CONFIG_SECURITY (for capabilities) and not any other LSM, then one
does not need to generate 12 slots for all each LSM hook when there
are only 3 LSMs compiled in (capabilities being implicitly behind
CONFIG_SECURITY).

> Should "number of possible LSMs" be replaced with "number of built-in LSMs" ?

Sure. I think "compiled LSMs" is a better word here.


>
> > Suggested-by: Andrii Nakryiko <andrii@kernel.org
>
> Trailing ">" is missing.

Fixed.

>
> > +/*
> > + * Macros to count the number of LSMs enabled in the kernel at compile time.
> > + */
> > +#define MAX_LSM_COUNT                        \
> > +     ___COUNT_COMMAS(                \
> > +             CAPABILITIES_ENABLED    \
> > +             SELINUX_ENABLED         \
> > +             SMACK_ENABLED           \
> > +             APPARMOR_ENABLED        \
> > +             TOMOYO_ENABLED          \
> > +             YAMA_ENABLED            \
> > +             LOADPIN_ENABLED         \
> > +             LOCKDOWN_ENABLED        \
> > +             BPF_LSM_ENABLED         \
> > +             LANDLOCK_ENABLED)
>
> Since IS_ENABLED(CONFIG_FOO) is evaluated to either 1 or 0, why can't you directly
> do like IS_ENABLED(CONFIG_FOO) + IS_ENABLED(CONFIG_BAR) + IS_ENABLED(CONFIG_BUZ) ?
> If you can't do direct "+", can't you still do indirect "+" like something below?
>
> #if IS_ENABLED(CONFIG_FOO)
> #define FOO_ENABLED 1
> #else
> #define FOO_ENABLED 0
> #endif
>

[PATCH v3 3/5] security: Replace indirect LSM hook calls with static calls

[KP Singh]

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.

[1] https://lore.kernel.org/linux-security-module/20220609234601.2026362-1-kpsingh@kernel.org/

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/lsm_hooks.h |  70 +++++++++++--
 security/security.c       | 208 +++++++++++++++++++++++++-------------
 2 files changed, 199 insertions(+), 79 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index dcb5e5b5eb13..eb9afe93496f 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -29,26 +29,77 @@
 #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;
+};
+
+/*
+ * Table of the static calls for each LSM hook.
+ * Once the LSMs are initialized, their callbacks will be copied to these
+ * tables such that the calls are filled backwards (from last to first).
+ * This way, we can jump directly to the first used static call, and execute
+ * all of them after. This essentially makes the entry point
+ * dynamic to adapt the number of static calls to the number of callbacks.
+ */
+struct lsm_static_calls_table {
+	#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
+		struct lsm_static_call NAME[MAX_LSM_COUNT];
+	#include <linux/lsm_hook_defs.h>
 	#undef LSM_HOOK
 } __randomize_layout;
 
 /*
  * Security module hook list structure.
  * For use with generic list macros for common operations.
+ *
+ * struct security_hook_list - Contents of a cacheable, mappable object.
+ * @scalls: The beginning of the array of static calls assigned to this hook.
+ * @hook: The callback for the hook.
+ * @lsm: The name of the lsm that owns this hook.
  */
 struct security_hook_list {
-	struct hlist_node		list;
-	struct hlist_head		*head;
+	struct lsm_static_call	*scalls;
 	union security_list_options	hook;
 	const char			*lsm;
 } __randomize_layout;
@@ -97,10 +148,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,
@@ -138,5 +191,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 7b0052e96806..c2c2cf6b711f 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)
@@ -73,7 +75,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;
@@ -92,6 +93,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 {							\
@@ -152,7 +198,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. */
@@ -325,6 +371,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);
 
@@ -404,11 +469,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;
@@ -524,7 +584,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
 
 	for (i = 0; i < count; i++) {
 		hooks[i].lsm = lsm;
-		hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
+		lsm_static_call_init(&hooks[i]);
 	}
 
 	/*
@@ -762,29 +822,41 @@ static int lsm_superblock_alloc(struct super_block *sb)
  * 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 */
 
 /**
@@ -1020,7 +1092,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;
 
@@ -1031,8 +1103,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;
@@ -1184,13 +1256,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)
@@ -1553,19 +1624,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);
@@ -1625,7 +1696,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;
 
@@ -1643,9 +1714,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;
@@ -2405,7 +2475,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)))
@@ -2413,9 +2483,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;
 	}
@@ -2440,7 +2509,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)))
@@ -2448,9 +2517,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;
 	}
@@ -2524,7 +2592,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;
 
 	/*
@@ -2532,9 +2600,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;
 	}
@@ -3414,10 +3481,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)
@@ -3814,12 +3881,12 @@ EXPORT_SYMBOL(security_d_instantiate);
 int security_getprocattr(struct task_struct *p, const char *lsm,
 			 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 (lsm != NULL && strcmp(lsm, hp->lsm))
+	lsm_for_each_hook(scall, getprocattr) {
+		if (lsm != NULL && strcmp(lsm, scall->hl->lsm))
 			continue;
-		return hp->hook.getprocattr(p, name, value);
+		return scall->hl->hook.getprocattr(p, name, value);
 	}
 	return LSM_RET_DEFAULT(getprocattr);
 }
@@ -3839,12 +3906,12 @@ int security_getprocattr(struct task_struct *p, const char *lsm,
 int security_setprocattr(const char *lsm, 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 (lsm != NULL && strcmp(lsm, hp->lsm))
+	lsm_for_each_hook(scall, setprocattr) {
+		if (lsm != NULL && strcmp(lsm, scall->hl->lsm))
 			continue;
-		return hp->hook.setprocattr(name, value, size);
+		return scall->hl->hook.setprocattr(name, value, size);
 	}
 	return LSM_RET_DEFAULT(setprocattr);
 }
@@ -3896,15 +3963,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;
 	}
@@ -4947,7 +5014,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);
 
 	/*
@@ -4959,9 +5026,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.42.0.459.ge4e396fd5e-goog

Re: [PATCH v3 3/5] security: Replace indirect LSM hook calls with static calls

[Kees Cook]

On Mon, Sep 18, 2023 at 11:24:57PM +0200, KP Singh 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:

I feel like the performance details in the cover letter should be
repeated in this patch, since it's the one doing the heavy lifting.

> [...]
> 
> Signed-off-by: KP Singh <kpsingh@kernel.org>

Regardless, this is a nice improvement on execution time and one of the
more complex cases for static calls.

> -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;
> +};

Can this be marked __randomize_layout too?

Everything else looks good to me. I actually find the result more
readable that before. But then I do love a good macro. :)

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v3 3/5] security: Replace indirect LSM hook calls with static calls

[KP Singh]

On Wed, Sep 20, 2023 at 5:54 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Sep 18, 2023 at 11:24:57PM +0200, KP Singh 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:
>
> I feel like the performance details in the cover letter should be
> repeated in this patch, since it's the one doing the heavy lifting.

Good point, added the results to the patch as well.

>
> > [...]
> >
> > Signed-off-by: KP Singh <kpsingh@kernel.org>
>
> Regardless, this is a nice improvement on execution time and one of the
> more complex cases for static calls.
>
> > -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;
> > +};
>
> Can this be marked __randomize_layout too?

Yes, done.

>
> Everything else looks good to me. I actually find the result more
> readable that before. But then I do love a good macro. :)

Yay!

>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> --
> Kees Cook

Re: [PATCH v3 3/5] security: Replace indirect LSM hook calls with static calls

[Casey Schaufler]

On 9/18/2023 2:24 PM, KP Singh 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>
>
> 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.
>
> [1] https://lore.kernel.org/linux-security-module/20220609234601.2026362-1-kpsingh@kernel.org/
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>

Good job on reducing the impact in security.c.

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>

> ---
>  include/linux/lsm_hooks.h |  70 +++++++++++--
>  security/security.c       | 208 +++++++++++++++++++++++++-------------
>  2 files changed, 199 insertions(+), 79 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index dcb5e5b5eb13..eb9afe93496f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -29,26 +29,77 @@
>  #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;
> +};
> +
> +/*
> + * Table of the static calls for each LSM hook.
> + * Once the LSMs are initialized, their callbacks will be copied to these
> + * tables such that the calls are filled backwards (from last to first).
> + * This way, we can jump directly to the first used static call, and execute
> + * all of them after. This essentially makes the entry point
> + * dynamic to adapt the number of static calls to the number of callbacks.
> + */
> +struct lsm_static_calls_table {
> +	#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> +		struct lsm_static_call NAME[MAX_LSM_COUNT];
> +	#include <linux/lsm_hook_defs.h>
>  	#undef LSM_HOOK
>  } __randomize_layout;
>  
>  /*
>   * Security module hook list structure.
>   * For use with generic list macros for common operations.
> + *
> + * struct security_hook_list - Contents of a cacheable, mappable object.
> + * @scalls: The beginning of the array of static calls assigned to this hook.
> + * @hook: The callback for the hook.
> + * @lsm: The name of the lsm that owns this hook.
>   */
>  struct security_hook_list {
> -	struct hlist_node		list;
> -	struct hlist_head		*head;
> +	struct lsm_static_call	*scalls;
>  	union security_list_options	hook;
>  	const char			*lsm;
>  } __randomize_layout;
> @@ -97,10 +148,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,
> @@ -138,5 +191,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 7b0052e96806..c2c2cf6b711f 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)
> @@ -73,7 +75,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;
> @@ -92,6 +93,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 {							\
> @@ -152,7 +198,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. */
> @@ -325,6 +371,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);
>  
> @@ -404,11 +469,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;
> @@ -524,7 +584,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  
>  	for (i = 0; i < count; i++) {
>  		hooks[i].lsm = lsm;
> -		hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
> +		lsm_static_call_init(&hooks[i]);
>  	}
>  
>  	/*
> @@ -762,29 +822,41 @@ static int lsm_superblock_alloc(struct super_block *sb)
>   * 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 */
>  
>  /**
> @@ -1020,7 +1092,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;
>  
> @@ -1031,8 +1103,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;
> @@ -1184,13 +1256,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)
> @@ -1553,19 +1624,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);
> @@ -1625,7 +1696,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;
>  
> @@ -1643,9 +1714,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;
> @@ -2405,7 +2475,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)))
> @@ -2413,9 +2483,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;
>  	}
> @@ -2440,7 +2509,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)))
> @@ -2448,9 +2517,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;
>  	}
> @@ -2524,7 +2592,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;
>  
>  	/*
> @@ -2532,9 +2600,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;
>  	}
> @@ -3414,10 +3481,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)
> @@ -3814,12 +3881,12 @@ EXPORT_SYMBOL(security_d_instantiate);
>  int security_getprocattr(struct task_struct *p, const char *lsm,
>  			 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 (lsm != NULL && strcmp(lsm, hp->lsm))
> +	lsm_for_each_hook(scall, getprocattr) {
> +		if (lsm != NULL && strcmp(lsm, scall->hl->lsm))
>  			continue;
> -		return hp->hook.getprocattr(p, name, value);
> +		return scall->hl->hook.getprocattr(p, name, value);
>  	}
>  	return LSM_RET_DEFAULT(getprocattr);
>  }
> @@ -3839,12 +3906,12 @@ int security_getprocattr(struct task_struct *p, const char *lsm,
>  int security_setprocattr(const char *lsm, 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 (lsm != NULL && strcmp(lsm, hp->lsm))
> +	lsm_for_each_hook(scall, setprocattr) {
> +		if (lsm != NULL && strcmp(lsm, scall->hl->lsm))
>  			continue;
> -		return hp->hook.setprocattr(name, value, size);
> +		return scall->hl->hook.setprocattr(name, value, size);
>  	}
>  	return LSM_RET_DEFAULT(setprocattr);
>  }
> @@ -3896,15 +3963,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;
>  	}
> @@ -4947,7 +5014,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);
>  
>  	/*
> @@ -4959,9 +5026,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;

Re: [PATCH v3 3/5] security: Replace indirect LSM hook calls with static calls

[KP Singh]

[...]

> > [1] https://lore.kernel.org/linux-security-module/20220609234601.2026362-1-kpsingh@kernel.org/
> >
> > Signed-off-by: KP Singh <kpsingh@kernel.org>
>
> Good job on reducing the impact in security.c.

Thanks!

> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


On Wed, Sep 20, 2023 at 8:11 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 9/18/2023 2:24 PM, KP Singh 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>
> >
> > 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.
> >
> > [1] https://lore.kernel.org/linux-security-module/20220609234601.2026362-1-kpsingh@kernel.org/
> >
> > Signed-off-by: KP Singh <kpsingh@kernel.org>
>
> Good job on reducing the impact in security.c.
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
>
> > ---
> >  include/linux/lsm_hooks.h |  70 +++++++++++--
> >  security/security.c       | 208 +++++++++++++++++++++++++-------------
> >  2 files changed, 199 insertions(+), 79 deletions(-)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index dcb5e5b5eb13..eb9afe93496f 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -29,26 +29,77 @@
> >  #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;
> > +};
> > +
> > +/*
> > + * Table of the static calls for each LSM hook.
> > + * Once the LSMs are initialized, their callbacks will be copied to these
> > + * tables such that the calls are filled backwards (from last to first).
> > + * This way, we can jump directly to the first used static call, and execute
> > + * all of them after. This essentially makes the entry point
> > + * dynamic to adapt the number of static calls to the number of callbacks.
> > + */
> > +struct lsm_static_calls_table {
> > +     #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> > +             struct lsm_static_call NAME[MAX_LSM_COUNT];
> > +     #include <linux/lsm_hook_defs.h>
> >       #undef LSM_HOOK
> >  } __randomize_layout;
> >
> >  /*
> >   * Security module hook list structure.
> >   * For use with generic list macros for common operations.
> > + *
> > + * struct security_hook_list - Contents of a cacheable, mappable object.
> > + * @scalls: The beginning of the array of static calls assigned to this hook.
> > + * @hook: The callback for the hook.
> > + * @lsm: The name of the lsm that owns this hook.
> >   */
> >  struct security_hook_list {
> > -     struct hlist_node               list;
> > -     struct hlist_head               *head;
> > +     struct lsm_static_call  *scalls;
> >       union security_list_options     hook;
> >       const char                      *lsm;
> >  } __randomize_layout;
> > @@ -97,10 +148,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,
> > @@ -138,5 +191,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 7b0052e96806..c2c2cf6b711f 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)
> > @@ -73,7 +75,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;
> > @@ -92,6 +93,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 {                                                    \
> > @@ -152,7 +198,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. */
> > @@ -325,6 +371,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);
> >
> > @@ -404,11 +469,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;
> > @@ -524,7 +584,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> >
> >       for (i = 0; i < count; i++) {
> >               hooks[i].lsm = lsm;
> > -             hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
> > +             lsm_static_call_init(&hooks[i]);
> >       }
> >
> >       /*
> > @@ -762,29 +822,41 @@ static int lsm_superblock_alloc(struct super_block *sb)
> >   * 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 */
> >
> >  /**
> > @@ -1020,7 +1092,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;
> >
> > @@ -1031,8 +1103,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;
> > @@ -1184,13 +1256,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)
> > @@ -1553,19 +1624,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);
> > @@ -1625,7 +1696,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;
> >
> > @@ -1643,9 +1714,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;
> > @@ -2405,7 +2475,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)))
> > @@ -2413,9 +2483,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;
> >       }
> > @@ -2440,7 +2509,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)))
> > @@ -2448,9 +2517,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;
> >       }
> > @@ -2524,7 +2592,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;
> >
> >       /*
> > @@ -2532,9 +2600,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;
> >       }
> > @@ -3414,10 +3481,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)
> > @@ -3814,12 +3881,12 @@ EXPORT_SYMBOL(security_d_instantiate);
> >  int security_getprocattr(struct task_struct *p, const char *lsm,
> >                        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 (lsm != NULL && strcmp(lsm, hp->lsm))
> > +     lsm_for_each_hook(scall, getprocattr) {
> > +             if (lsm != NULL && strcmp(lsm, scall->hl->lsm))
> >                       continue;
> > -             return hp->hook.getprocattr(p, name, value);
> > +             return scall->hl->hook.getprocattr(p, name, value);
> >       }
> >       return LSM_RET_DEFAULT(getprocattr);
> >  }
> > @@ -3839,12 +3906,12 @@ int security_getprocattr(struct task_struct *p, const char *lsm,
> >  int security_setprocattr(const char *lsm, 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 (lsm != NULL && strcmp(lsm, hp->lsm))
> > +     lsm_for_each_hook(scall, setprocattr) {
> > +             if (lsm != NULL && strcmp(lsm, scall->hl->lsm))
> >                       continue;
> > -             return hp->hook.setprocattr(name, value, size);
> > +             return scall->hl->hook.setprocattr(name, value, size);
> >       }
> >       return LSM_RET_DEFAULT(setprocattr);
> >  }
> > @@ -3896,15 +3963,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;
> >       }
> > @@ -4947,7 +5014,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);
> >
> >       /*
> > @@ -4959,9 +5026,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;

Re: [PATCH v3 3/5] security: Replace indirect LSM hook calls with static calls

[Song Liu]

On Mon, Sep 18, 2023 at 2:25 PM KP Singh <kpsingh@kernel.org> wrote:
>
[...]
>
> 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.
>
> [1] https://lore.kernel.org/linux-security-module/20220609234601.2026362-1-kpsingh@kernel.org/
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>

Acked-by: Song Liu <song@kernel.org>

> ---
[...]

[PATCH v3 4/5] bpf: Only enable BPF LSM hooks when an LSM program is attached

[KP Singh]

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

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/bpf.h       |  1 +
 include/linux/bpf_lsm.h   |  5 +++++
 include/linux/lsm_hooks.h | 13 ++++++++++++-
 kernel/bpf/trampoline.c   | 29 +++++++++++++++++++++++++++--
 security/bpf/hooks.c      | 25 ++++++++++++++++++++++++-
 security/security.c       |  3 ++-
 6 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b9e573159432..84c9eb6ae07a 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1159,6 +1159,7 @@ struct bpf_attach_target_info {
 	struct module *tgt_mod;
 	const char *tgt_name;
 	const struct btf_type *tgt_type;
+	bool is_lsm_target;
 };
 
 #define BPF_DISPATCHER_MAX 48 /* Fits in 2048B */
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 eb9afe93496f..0797e9f97cb3 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -97,11 +97,14 @@ struct lsm_static_calls_table {
  * @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 char			*lsm;
+	bool				default_state;
 } __randomize_layout;
 
 /*
@@ -151,7 +154,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_state = true			\
+	}
+
+#define LSM_HOOK_INIT_DISABLED(NAME, CALLBACK)		\
+	{						\
+		.scalls = static_calls_table.NAME,	\
+		.hook = { .NAME = CALLBACK },		\
+		.default_state = false			\
 	}
 
 extern char *lsm_names;
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index e97aeda3a86b..df9699bce372 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 = {
@@ -514,7 +515,7 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
 {
 	enum bpf_tramp_prog_type kind;
 	struct bpf_tramp_link *link_exiting;
-	int err = 0;
+	int err = 0, num_lsm_progs = 0;
 	int cnt = 0, i;
 
 	kind = bpf_attach_type_to_tramp(link->link.prog);
@@ -545,8 +546,14 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
 			continue;
 		/* prog already linked */
 		return -EBUSY;
+
+		if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
+			num_lsm_progs++;
 	}
 
+	if (!num_lsm_progs && link->link.prog->type == BPF_PROG_TYPE_LSM)
+		bpf_lsm_toggle_hook(tr->func.addr, true);
+
 	hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
 	tr->progs_cnt[kind]++;
 	err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
@@ -569,8 +576,10 @@ int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline
 
 static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr)
 {
+	struct bpf_tramp_link *link_exiting;
 	enum bpf_tramp_prog_type kind;
-	int err;
+	bool lsm_link_found = false;
+	int err, num_lsm_progs = 0;
 
 	kind = bpf_attach_type_to_tramp(link->link.prog);
 	if (kind == BPF_TRAMP_REPLACE) {
@@ -580,8 +589,24 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_
 		tr->extension_prog = NULL;
 		return err;
 	}
+
+	if (link->link.prog->type == BPF_PROG_TYPE_LSM) {
+		hlist_for_each_entry(link_exiting, &tr->progs_hlist[kind],
+				     tramp_hlist) {
+			if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
+				num_lsm_progs++;
+
+			if (link_exiting->link.prog == link->link.prog)
+				lsm_link_found = true;
+		}
+	}
+
 	hlist_del_init(&link->tramp_hlist);
 	tr->progs_cnt[kind]--;
+
+	if (lsm_link_found && num_lsm_progs == 1)
+		bpf_lsm_toggle_hook(tr->func.addr, false);
+
 	return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
 }
 
diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
index cfaf1d0e6a5f..1957244196d0 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -8,7 +8,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),
@@ -32,3 +32,26 @@ DEFINE_LSM(bpf) = {
 	.init = bpf_lsm_init,
 	.blobs = &bpf_lsm_blob_sizes
 };
+
+void bpf_lsm_toggle_hook(void *addr, bool value)
+{
+	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];
+		scalls = h->scalls;
+		if (h->hook.lsm_callback == addr)
+			continue;
+
+		for (j = 0; j < MAX_LSM_COUNT; j++) {
+			if (scalls[j].hl != h)
+				continue;
+			if (value)
+				static_branch_enable(scalls[j].active);
+			else
+				static_branch_disable(scalls[j].active);
+		}
+	}
+}
diff --git a/security/security.c b/security/security.c
index c2c2cf6b711f..d1ee72e563cc 100644
--- a/security/security.c
+++ b/security/security.c
@@ -382,7 +382,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_state)
+				static_branch_enable(scall->active);
 			return;
 		}
 		scall++;
-- 
2.42.0.459.ge4e396fd5e-goog

Re: [PATCH v3 4/5] bpf: Only enable BPF LSM hooks when an LSM program is attached

[Kees Cook]

On Mon, Sep 18, 2023 at 11:24:58PM +0200, KP Singh wrote:
> [...]
> +void bpf_lsm_toggle_hook(void *addr, bool value)
> +{
> +	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];
> +		scalls = h->scalls;
> +		if (h->hook.lsm_callback == addr)
> +			continue;
> +
> +		for (j = 0; j < MAX_LSM_COUNT; j++) {
> +			if (scalls[j].hl != h)
> +				continue;
> +			if (value)
> +				static_branch_enable(scalls[j].active);
> +			else
> +				static_branch_disable(scalls[j].active);
> +		}
> +	}
> +}

And this happily works with everything being read-only? I double-checked
these structures, and it seems like the answer is "yes". :)

So, to that end:

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v3 4/5] bpf: Only enable BPF LSM hooks when an LSM program is attached

[Casey Schaufler]

On 9/18/2023 2:24 PM, KP Singh 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
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>

> ---
>  include/linux/bpf.h       |  1 +
>  include/linux/bpf_lsm.h   |  5 +++++
>  include/linux/lsm_hooks.h | 13 ++++++++++++-
>  kernel/bpf/trampoline.c   | 29 +++++++++++++++++++++++++++--
>  security/bpf/hooks.c      | 25 ++++++++++++++++++++++++-
>  security/security.c       |  3 ++-
>  6 files changed, 71 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b9e573159432..84c9eb6ae07a 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1159,6 +1159,7 @@ struct bpf_attach_target_info {
>  	struct module *tgt_mod;
>  	const char *tgt_name;
>  	const struct btf_type *tgt_type;
> +	bool is_lsm_target;
>  };
>  
>  #define BPF_DISPATCHER_MAX 48 /* Fits in 2048B */
> 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 eb9afe93496f..0797e9f97cb3 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -97,11 +97,14 @@ struct lsm_static_calls_table {
>   * @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 char			*lsm;
> +	bool				default_state;
>  } __randomize_layout;
>  
>  /*
> @@ -151,7 +154,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_state = true			\
> +	}
> +
> +#define LSM_HOOK_INIT_DISABLED(NAME, CALLBACK)		\
> +	{						\
> +		.scalls = static_calls_table.NAME,	\
> +		.hook = { .NAME = CALLBACK },		\
> +		.default_state = false			\
>  	}
>  
>  extern char *lsm_names;
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index e97aeda3a86b..df9699bce372 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 = {
> @@ -514,7 +515,7 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
>  {
>  	enum bpf_tramp_prog_type kind;
>  	struct bpf_tramp_link *link_exiting;
> -	int err = 0;
> +	int err = 0, num_lsm_progs = 0;
>  	int cnt = 0, i;
>  
>  	kind = bpf_attach_type_to_tramp(link->link.prog);
> @@ -545,8 +546,14 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
>  			continue;
>  		/* prog already linked */
>  		return -EBUSY;
> +
> +		if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> +			num_lsm_progs++;
>  	}
>  
> +	if (!num_lsm_progs && link->link.prog->type == BPF_PROG_TYPE_LSM)
> +		bpf_lsm_toggle_hook(tr->func.addr, true);
> +
>  	hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
>  	tr->progs_cnt[kind]++;
>  	err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
> @@ -569,8 +576,10 @@ int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline
>  
>  static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr)
>  {
> +	struct bpf_tramp_link *link_exiting;
>  	enum bpf_tramp_prog_type kind;
> -	int err;
> +	bool lsm_link_found = false;
> +	int err, num_lsm_progs = 0;
>  
>  	kind = bpf_attach_type_to_tramp(link->link.prog);
>  	if (kind == BPF_TRAMP_REPLACE) {
> @@ -580,8 +589,24 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_
>  		tr->extension_prog = NULL;
>  		return err;
>  	}
> +
> +	if (link->link.prog->type == BPF_PROG_TYPE_LSM) {
> +		hlist_for_each_entry(link_exiting, &tr->progs_hlist[kind],
> +				     tramp_hlist) {
> +			if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> +				num_lsm_progs++;
> +
> +			if (link_exiting->link.prog == link->link.prog)
> +				lsm_link_found = true;
> +		}
> +	}
> +
>  	hlist_del_init(&link->tramp_hlist);
>  	tr->progs_cnt[kind]--;
> +
> +	if (lsm_link_found && num_lsm_progs == 1)
> +		bpf_lsm_toggle_hook(tr->func.addr, false);
> +
>  	return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
>  }
>  
> diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
> index cfaf1d0e6a5f..1957244196d0 100644
> --- a/security/bpf/hooks.c
> +++ b/security/bpf/hooks.c
> @@ -8,7 +8,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),
> @@ -32,3 +32,26 @@ DEFINE_LSM(bpf) = {
>  	.init = bpf_lsm_init,
>  	.blobs = &bpf_lsm_blob_sizes
>  };
> +
> +void bpf_lsm_toggle_hook(void *addr, bool value)
> +{
> +	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];
> +		scalls = h->scalls;
> +		if (h->hook.lsm_callback == addr)
> +			continue;
> +
> +		for (j = 0; j < MAX_LSM_COUNT; j++) {
> +			if (scalls[j].hl != h)
> +				continue;
> +			if (value)
> +				static_branch_enable(scalls[j].active);
> +			else
> +				static_branch_disable(scalls[j].active);
> +		}
> +	}
> +}
> diff --git a/security/security.c b/security/security.c
> index c2c2cf6b711f..d1ee72e563cc 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -382,7 +382,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_state)
> +				static_branch_enable(scall->active);
>  			return;
>  		}
>  		scall++;

Re: [PATCH v3 4/5] bpf: Only enable BPF LSM hooks when an LSM program is attached

[Song Liu]

On Mon, Sep 18, 2023 at 2:25 PM KP Singh <kpsingh@kernel.org> wrote:
>
[...]
>    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
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>

Acked-by: Song Liu <song@kernel.org>

[PATCH v3 5/5] security: Add CONFIG_SECURITY_HOOK_LIKELY

[KP Singh]

This config influences the nature of the static key that guards the
static call for LSM hooks.

When enabled, it indicates that an LSM static call slot is more likely
to be initialized. When disabled, it optimizes for the case when static
call slot is more likely to be not initialized.

When a major LSM like (SELinux, AppArmor, Smack etc) is active on a
system the system would benefit from enabling the config. However there
are other cases which would benefit from the config being disabled
(e.g. a system with a BPF LSM with no hooks enabled by default, or an
LSM like loadpin / yama). Ultimately, there is no one-size fits all
solution.

with CONFIG_SECURITY_HOOK_LIKELY enabled, the inactive /
uninitialized case is penalized with a direct jmp (still better than
an indirect jmp):

function security_file_ioctl:
   0xffffffff818f0c80 <+0>:	endbr64
   0xffffffff818f0c84 <+4>:	nopl   0x0(%rax,%rax,1)
   0xffffffff818f0c89 <+9>:	push   %rbp
   0xffffffff818f0c8a <+10>:	push   %r14
   0xffffffff818f0c8c <+12>:	push   %rbx
   0xffffffff818f0c8d <+13>:	mov    %rdx,%rbx
   0xffffffff818f0c90 <+16>:	mov    %esi,%ebp
   0xffffffff818f0c92 <+18>:	mov    %rdi,%r14
   0xffffffff818f0c95 <+21>:	jmp    0xffffffff818f0ca8 <security_file_ioctl+40>

   jump to skip the inactive BPF LSM hook.

   0xffffffff818f0c97 <+23>:	mov    %r14,%rdi
   0xffffffff818f0c9a <+26>:	mov    %ebp,%esi
   0xffffffff818f0c9c <+28>:	mov    %rbx,%rdx
   0xffffffff818f0c9f <+31>:	call   0xffffffff8141e3b0 <bpf_lsm_file_ioctl>
   0xffffffff818f0ca4 <+36>:	test   %eax,%eax
   0xffffffff818f0ca6 <+38>:	jne    0xffffffff818f0cbf <security_file_ioctl+63>
   0xffffffff818f0ca8 <+40>:	endbr64
   0xffffffff818f0cac <+44>:	jmp    0xffffffff818f0ccd <security_file_ioctl+77>

   jump to skip the empty slot.

   0xffffffff818f0cae <+46>:	mov    %r14,%rdi
   0xffffffff818f0cb1 <+49>:	mov    %ebp,%esi
   0xffffffff818f0cb3 <+51>:	mov    %rbx,%rdx
   0xffffffff818f0cb6 <+54>:	nopl   0x0(%rax,%rax,1)
  				^^^^^^^^^^^^^^^^^^^^^^^
				Empty slot

   0xffffffff818f0cbb <+59>:	test   %eax,%eax
   0xffffffff818f0cbd <+61>:	je     0xffffffff818f0ccd <security_file_ioctl+77>
   0xffffffff818f0cbf <+63>:	endbr64
   0xffffffff818f0cc3 <+67>:	pop    %rbx
   0xffffffff818f0cc4 <+68>:	pop    %r14
   0xffffffff818f0cc6 <+70>:	pop    %rbp
   0xffffffff818f0cc7 <+71>:	cs jmp 0xffffffff82c00000 <__x86_return_thunk>
   0xffffffff818f0ccd <+77>:	endbr64
   0xffffffff818f0cd1 <+81>:	xor    %eax,%eax
   0xffffffff818f0cd3 <+83>:	jmp    0xffffffff818f0cbf <security_file_ioctl+63>
   0xffffffff818f0cd5 <+85>:	mov    %r14,%rdi
   0xffffffff818f0cd8 <+88>:	mov    %ebp,%esi
   0xffffffff818f0cda <+90>:	mov    %rbx,%rdx
   0xffffffff818f0cdd <+93>:	pop    %rbx
   0xffffffff818f0cde <+94>:	pop    %r14
   0xffffffff818f0ce0 <+96>:	pop    %rbp
   0xffffffff818f0ce1 <+97>:	ret

When the config is disabled, the case optimizes the scenario above.

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>:	xchg   %ax,%ax
   0xffffffff818f0e47 <+23>:	xchg   %ax,%ax

   The static keys in their disabled state do not create jumps leading
   to faster code.

   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   0xffffffff8141e3b0 <bpf_lsm_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>:	nopl   0x0(%rax,%rax,1)
   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

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 security/Kconfig    | 11 +++++++++++
 security/security.c | 12 +++++++-----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/security/Kconfig b/security/Kconfig
index 52c9af08ad35..bd2a0dff991a 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -32,6 +32,17 @@ config SECURITY
 
 	  If you are unsure how to answer this question, answer N.
 
+config SECURITY_HOOK_LIKELY
+	bool "LSM hooks are likely to be initialized"
+	depends on SECURITY
+	default y
+	help
+	  This controls the behaviour of the static keys that guard LSM hooks.
+	  If LSM hooks are likely to be initialized by LSMs, then one gets
+	  better performance by enabling this option. However, if the system is
+	  using an LSM where hooks are much likely to be disabled, one gets
+	  better performance by disabling this config.
+
 config SECURITYFS
 	bool "Enable the securityfs filesystem"
 	help
diff --git a/security/security.c b/security/security.c
index d1ee72e563cc..7ab0e044f83d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -105,9 +105,9 @@ static __initdata struct lsm_info *exclusive;
  * 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 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, ...)				\
@@ -825,7 +825,8 @@ static int lsm_superblock_alloc(struct super_block *sb)
  */
 #define __CALL_STATIC_VOID(NUM, HOOK, ...)				     \
 do {									     \
-	if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {    \
+	if (static_branch_maybe(CONFIG_SECURITY_HOOK_LIKELY,		     \
+				&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {     \
 		static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__);	     \
 	}								     \
 } while (0);
@@ -837,7 +838,8 @@ do {									     \
 
 #define __CALL_STATIC_INT(NUM, R, HOOK, LABEL, ...)			     \
 do {									     \
-	if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {  \
+	if (static_branch_maybe(CONFIG_SECURITY_HOOK_LIKELY,		     \
+				&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {     \
 		R = static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__);    \
 		if (R != 0)						     \
 			goto LABEL;					     \
-- 
2.42.0.459.ge4e396fd5e-goog

Re: [PATCH v3 5/5] security: Add CONFIG_SECURITY_HOOK_LIKELY

[Kees Cook]

On Mon, Sep 18, 2023 at 11:24:59PM +0200, KP Singh wrote:
> This config influences the nature of the static key that guards the
> static call for LSM hooks.
> 
> When enabled, it indicates that an LSM static call slot is more likely
> to be initialized. When disabled, it optimizes for the case when static
> call slot is more likely to be not initialized.
> 
> When a major LSM like (SELinux, AppArmor, Smack etc) is active on a
> system the system would benefit from enabling the config. However there
> are other cases which would benefit from the config being disabled
> (e.g. a system with a BPF LSM with no hooks enabled by default, or an
> LSM like loadpin / yama). Ultimately, there is no one-size fits all
> solution.
> 
> with CONFIG_SECURITY_HOOK_LIKELY enabled, the inactive /
> uninitialized case is penalized with a direct jmp (still better than
> an indirect jmp):
> [...]
> index 52c9af08ad35..bd2a0dff991a 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -32,6 +32,17 @@ config SECURITY
>  
>  	  If you are unsure how to answer this question, answer N.
>  
> +config SECURITY_HOOK_LIKELY
> +	bool "LSM hooks are likely to be initialized"
> +	depends on SECURITY
> +	default y
> +	help
> +	  This controls the behaviour of the static keys that guard LSM hooks.
> +	  If LSM hooks are likely to be initialized by LSMs, then one gets
> +	  better performance by enabling this option. However, if the system is
> +	  using an LSM where hooks are much likely to be disabled, one gets
> +	  better performance by disabling this config.

Since you described the situations where it's a net benefit, this could
be captured in the Kconfig too. How about this, which tracks the "major"
LSMs as in the DEFAULT_SECURITY choice:

	depends on SECURITY && EXPERT
	default BPF_LSM || SECURITY_SELINUX || SECURITY_SMACK || SECURITY_TOMOYO || SECURITY_APPARMOR


-- 
Kees Cook

Re: [PATCH v3 5/5] security: Add CONFIG_SECURITY_HOOK_LIKELY

[KP Singh]

[...]

> > +config SECURITY_HOOK_LIKELY
> > +     bool "LSM hooks are likely to be initialized"
> > +     depends on SECURITY
> > +     default y
> > +     help
> > +       This controls the behaviour of the static keys that guard LSM hooks.
> > +       If LSM hooks are likely to be initialized by LSMs, then one gets
> > +       better performance by enabling this option. However, if the system is
> > +       using an LSM where hooks are much likely to be disabled, one gets
> > +       better performance by disabling this config.
>
> Since you described the situations where it's a net benefit, this could
> be captured in the Kconfig too. How about this, which tracks the "major"
> LSMs as in the DEFAULT_SECURITY choice:
>
>         depends on SECURITY && EXPERT
>         default BPF_LSM || SECURITY_SELINUX || SECURITY_SMACK || SECURITY_TOMOYO || SECURITY_APPARMOR\

I think for BPF_LSM the option would not be y. But yeah I like this suggestion.

>
>
> --
> Kees Cook

Re: [PATCH v3 5/5] security: Add CONFIG_SECURITY_HOOK_LIKELY

[Song Liu]

On Mon, Sep 18, 2023 at 2:25 PM KP Singh <kpsingh@kernel.org> wrote:
>
[...]
>    0xffffffff818f0e72 <+66>:    mov    %r14,%rdi
>    0xffffffff818f0e75 <+69>:    mov    %ebp,%esi
>    0xffffffff818f0e77 <+71>:    mov    %rbx,%rdx
>    0xffffffff818f0e7a <+74>:    nopl   0x0(%rax,%rax,1)
>    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
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>

Acked-by: Song Liu <song@kernel.org>

Thanks,
Song



> ---
>  security/Kconfig    | 11 +++++++++++
>  security/security.c | 12 +++++++-----
>  2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 52c9af08ad35..bd2a0dff991a 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -32,6 +32,17 @@ config SECURITY
>
>           If you are unsure how to answer this question, answer N.
>
> +config SECURITY_HOOK_LIKELY
> +       bool "LSM hooks are likely to be initialized"
> +       depends on SECURITY
> +       default y
> +       help
> +         This controls the behaviour of the static keys that guard LSM hooks.
> +         If LSM hooks are likely to be initialized by LSMs, then one gets
> +         better performance by enabling this option. However, if the system is
> +         using an LSM where hooks are much likely to be disabled, one gets
> +         better performance by disabling this config.
> +
>  config SECURITYFS
>         bool "Enable the securityfs filesystem"
>         help
> diff --git a/security/security.c b/security/security.c
> index d1ee72e563cc..7ab0e044f83d 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -105,9 +105,9 @@ static __initdata struct lsm_info *exclusive;
>   * 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 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, ...)                              \
> @@ -825,7 +825,8 @@ static int lsm_superblock_alloc(struct super_block *sb)
>   */
>  #define __CALL_STATIC_VOID(NUM, HOOK, ...)                                  \
>  do {                                                                        \
> -       if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {    \
> +       if (static_branch_maybe(CONFIG_SECURITY_HOOK_LIKELY,                 \
> +                               &SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {     \
>                 static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__);        \
>         }                                                                    \
>  } while (0);
> @@ -837,7 +838,8 @@ do {                                                                             \
>
>  #define __CALL_STATIC_INT(NUM, R, HOOK, LABEL, ...)                         \
>  do {                                                                        \
> -       if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {  \
> +       if (static_branch_maybe(CONFIG_SECURITY_HOOK_LIKELY,                 \
> +                               &SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {     \
>                 R = static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__);    \
>                 if (R != 0)                                                  \
>                         goto LABEL;                                          \
> --
> 2.42.0.459.ge4e396fd5e-goog
>