diff mbox series

[bpf-next,2/4] security: Generate a header with the count of enabled LSMs

Message ID 20230119231033.1307221-3-kpsingh@kernel.org (mailing list archive)
State Superseded
Headers show
Series Reduce overhead of LSMs with static calls | expand

Commit Message

KP Singh Jan. 19, 2023, 11:10 p.m. UTC
The header defines a MAX_LSM_COUNT constant which is used in a
subsequent patch to generate the static calls for each LSM hook which
are named using preprocessor token pasting. Since token pasting does not
work with arithmetic expressions, generate a simple lsm_count.h header
which represents the subset of LSMs that can be enabled on a given
kernel based on the config.

While one can generate static calls for all the possible LSMs that the
kernel has, this is actually wasteful as most kernels only enable a
handful of LSMs.

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 scripts/Makefile                 |  1 +
 scripts/security/.gitignore      |  1 +
 scripts/security/Makefile        |  4 +++
 scripts/security/gen_lsm_count.c | 57 ++++++++++++++++++++++++++++++++
 security/Makefile                | 11 ++++++
 5 files changed, 74 insertions(+)
 create mode 100644 scripts/security/.gitignore
 create mode 100644 scripts/security/Makefile
 create mode 100644 scripts/security/gen_lsm_count.c

Comments

Casey Schaufler Jan. 20, 2023, 1:32 a.m. UTC | #1
On 1/19/2023 3:10 PM, KP Singh wrote:
> The header defines a MAX_LSM_COUNT constant which is used in a
> subsequent patch to generate the static calls for each LSM hook which
> are named using preprocessor token pasting. Since token pasting does not
> work with arithmetic expressions, generate a simple lsm_count.h header
> which represents the subset of LSMs that can be enabled on a given
> kernel based on the config.
>
> While one can generate static calls for all the possible LSMs that the
> kernel has, this is actually wasteful as most kernels only enable a
> handful of LSMs.

Why "generate" anything? Why not include your GEN_MAX_LSM_COUNT macro
in security.h and be done with it? I've proposed doing just that in the
stacking patch set for some time. This seems to be much more complicated
than it needs to be.

> Signed-off-by: KP Singh <kpsingh@kernel.org>
> ---
>  scripts/Makefile                 |  1 +
>  scripts/security/.gitignore      |  1 +
>  scripts/security/Makefile        |  4 +++
>  scripts/security/gen_lsm_count.c | 57 ++++++++++++++++++++++++++++++++
>  security/Makefile                | 11 ++++++
>  5 files changed, 74 insertions(+)
>  create mode 100644 scripts/security/.gitignore
>  create mode 100644 scripts/security/Makefile
>  create mode 100644 scripts/security/gen_lsm_count.c
>
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 1575af84d557..9712249c0fb3 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -41,6 +41,7 @@ targets += module.lds
>  subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
>  subdir-$(CONFIG_MODVERSIONS) += genksyms
>  subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> +subdir-$(CONFIG_SECURITY) += security
>  
>  # Let clean descend into subdirs
>  subdir-	+= basic dtc gdb kconfig mod
> diff --git a/scripts/security/.gitignore b/scripts/security/.gitignore
> new file mode 100644
> index 000000000000..684af16735f1
> --- /dev/null
> +++ b/scripts/security/.gitignore
> @@ -0,0 +1 @@
> +gen_lsm_count
> diff --git a/scripts/security/Makefile b/scripts/security/Makefile
> new file mode 100644
> index 000000000000..05f7e4109052
> --- /dev/null
> +++ b/scripts/security/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +hostprogs-always-y += gen_lsm_count
> +HOST_EXTRACFLAGS += \
> +	-I$(srctree)/include/uapi -I$(srctree)/include
> diff --git a/scripts/security/gen_lsm_count.c b/scripts/security/gen_lsm_count.c
> new file mode 100644
> index 000000000000..a9a227724d84
> --- /dev/null
> +++ b/scripts/security/gen_lsm_count.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/* NOTE: we really do want to use the kernel headers here */
> +#define __EXPORTED_HEADERS__
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <ctype.h>
> +
> +#include <linux/kconfig.h>
> +
> +#define GEN_MAX_LSM_COUNT (				\
> +	/* Capabilities */				\
> +	IS_ENABLED(CONFIG_SECURITY) +			\
> +	IS_ENABLED(CONFIG_SECURITY_SELINUX) +		\
> +	IS_ENABLED(CONFIG_SECURITY_SMACK) +		\
> +	IS_ENABLED(CONFIG_SECURITY_TOMOYO) +		\
> +	IS_ENABLED(CONFIG_SECURITY_APPARMOR) +		\
> +	IS_ENABLED(CONFIG_SECURITY_YAMA) +		\
> +	IS_ENABLED(CONFIG_SECURITY_LOADPIN) +		\
> +	IS_ENABLED(CONFIG_SECURITY_SAFESETID) +		\
> +	IS_ENABLED(CONFIG_SECURITY_LOCKDOWN_LSM) + 	\
> +	IS_ENABLED(CONFIG_BPF_LSM) + \
> +	IS_ENABLED(CONFIG_SECURITY_LANDLOCK))
> +
> +const char *progname;
> +
> +static void usage(void)
> +{
> +	printf("usage: %s lsm_count.h\n", progname);
> +	exit(1);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	FILE *fout;
> +
> +	progname = argv[0];
> +
> +	if (argc < 2)
> +		usage();
> +
> +	fout = fopen(argv[1], "w");
> +	if (!fout) {
> +		fprintf(stderr, "Could not open %s for writing:  %s\n",
> +			argv[1], strerror(errno));
> +		exit(2);
> +	}
> +
> +	fprintf(fout, "#ifndef _LSM_COUNT_H_\n#define _LSM_COUNT_H_\n\n");
> +	fprintf(fout, "\n#define MAX_LSM_COUNT %d\n", GEN_MAX_LSM_COUNT);
> +	fprintf(fout, "#endif /* _LSM_COUNT_H_ */\n");
> +	exit(0);
> +}
> diff --git a/security/Makefile b/security/Makefile
> index 18121f8f85cd..7a47174831f4 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -3,6 +3,7 @@
>  # Makefile for the kernel security code
>  #
>  
> +gen := include/generated
>  obj-$(CONFIG_KEYS)			+= keys/
>  
>  # always enable default capabilities
> @@ -27,3 +28,13 @@ obj-$(CONFIG_SECURITY_LANDLOCK)		+= landlock/
>  
>  # Object integrity file lists
>  obj-$(CONFIG_INTEGRITY)			+= integrity/
> +
> +$(addprefix $(obj)/,$(obj-y)): $(gen)/lsm_count.h
> +
> +quiet_cmd_lsm_count = GEN     ${gen}/lsm_count.h
> +      cmd_lsm_count = scripts/security/gen_lsm_count ${gen}/lsm_count.h
> +
> +targets += lsm_count.h
> +
> +${gen}/lsm_count.h: FORCE
> +	$(call if_changed,lsm_count)
KP Singh Jan. 20, 2023, 2:15 a.m. UTC | #2
On Fri, Jan 20, 2023 at 2:32 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 1/19/2023 3:10 PM, KP Singh wrote:
> > The header defines a MAX_LSM_COUNT constant which is used in a
> > subsequent patch to generate the static calls for each LSM hook which
> > are named using preprocessor token pasting. Since token pasting does not
> > work with arithmetic expressions, generate a simple lsm_count.h header
> > which represents the subset of LSMs that can be enabled on a given
> > kernel based on the config.
> >
> > While one can generate static calls for all the possible LSMs that the
> > kernel has, this is actually wasteful as most kernels only enable a
> > handful of LSMs.
>
> Why "generate" anything? Why not include your GEN_MAX_LSM_COUNT macro
> in security.h and be done with it? I've proposed doing just that in the
> stacking patch set for some time. This seems to be much more complicated
> than it needs to be.

The answer is in the commit description, the count is used in token
pasting and you cannot have arithmetic in when you generate tokens in
preprocessor macros.

you cannot generate bprm_check_security_call_1 + 1 + 1 this does not
get resolved by preprocessor.
Kui-Feng Lee Jan. 20, 2023, 6:35 p.m. UTC | #3
The following idea should work with the use case here.

#define COUNT_8(x, y...) 8
#define COUNT_7(x, y...) 7
#define COUNT_6(x, y...) 6
#define COUNT_5(x, y...) 5
#define COUNT_4(x, y...) 4
#define COUNT_3(x, y...) 3
#define COUNT_2(x, y...) 2
#define COUNT_1(x, y...) 1
#define COUNT_0(x, y...) 0
#define COUNT1_8(x, y...) COUNT ## x ## _9(y)
#define COUNT1_7(x, y...) COUNT ## x ## _8(y)
#define COUNT1_6(x, y...) COUNT ## x ## _7(y)
#define COUNT1_5(x, y...) COUNT ## x ## _6(y)
#define COUNT1_4(x, y...) COUNT ## x ## _5(y)
#define COUNT1_3(x, y...) COUNT ## x ## _4(y)
#define COUNT1_2(x, y...) COUNT ## x ## _3(y)
#define COUNT1_1(x, y...) COUNT ## x ## _2(y)
#define COUNT1_0(x, y...) COUNT ## x ## _1(y)
#define COUNT(x, y...) COUNT ## x ## _0(y)

#define COUNT_EXPAND(x...) COUNT(x)


#if IS_ENABLED(CONFIG_SECURITY_SELINUX)
#define SELINUX_ENABLE 1,
#else
#define SELINUX_ENABLE
#endif
#if IS_ENABLED(CONFIG_SECURITY_XXXX)
#define XXX_ENABLE 1,
#else
#define XXX_ENABLE
#endif
....

#define MAX_LSM_COUNT COUNT_EXPAND(SELINUX_ENABLE XXX_ENABLE ......)

On 1/19/23 18:15, KP Singh wrote:
> On Fri, Jan 20, 2023 at 2:32 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 1/19/2023 3:10 PM, KP Singh wrote:
>>> The header defines a MAX_LSM_COUNT constant which is used in a
>>> subsequent patch to generate the static calls for each LSM hook which
>>> are named using preprocessor token pasting. Since token pasting does not
>>> work with arithmetic expressions, generate a simple lsm_count.h header
>>> which represents the subset of LSMs that can be enabled on a given
>>> kernel based on the config.
>>>
>>> While one can generate static calls for all the possible LSMs that the
>>> kernel has, this is actually wasteful as most kernels only enable a
>>> handful of LSMs.
>> Why "generate" anything? Why not include your GEN_MAX_LSM_COUNT macro
>> in security.h and be done with it? I've proposed doing just that in the
>> stacking patch set for some time. This seems to be much more complicated
>> than it needs to be.
> The answer is in the commit description, the count is used in token
> pasting and you cannot have arithmetic in when you generate tokens in
> preprocessor macros.
>
> you cannot generate bprm_check_security_call_1 + 1 + 1 this does not
> get resolved by preprocessor.
Kees Cook Jan. 20, 2023, 7:40 p.m. UTC | #4
On Fri, Jan 20, 2023 at 10:35:02AM -0800, Kui-Feng Lee wrote:
> The following idea should work with the use case here.
> 
> #define COUNT_8(x, y...) 8
> #define COUNT_7(x, y...) 7
> #define COUNT_6(x, y...) 6
> #define COUNT_5(x, y...) 5
> #define COUNT_4(x, y...) 4
> #define COUNT_3(x, y...) 3
> #define COUNT_2(x, y...) 2
> #define COUNT_1(x, y...) 1
> #define COUNT_0(x, y...) 0
> #define COUNT1_8(x, y...) COUNT ## x ## _9(y)
> #define COUNT1_7(x, y...) COUNT ## x ## _8(y)
> #define COUNT1_6(x, y...) COUNT ## x ## _7(y)
> #define COUNT1_5(x, y...) COUNT ## x ## _6(y)
> #define COUNT1_4(x, y...) COUNT ## x ## _5(y)
> #define COUNT1_3(x, y...) COUNT ## x ## _4(y)
> #define COUNT1_2(x, y...) COUNT ## x ## _3(y)
> #define COUNT1_1(x, y...) COUNT ## x ## _2(y)
> #define COUNT1_0(x, y...) COUNT ## x ## _1(y)
> #define COUNT(x, y...) COUNT ## x ## _0(y)
> 
> #define COUNT_EXPAND(x...) COUNT(x)
> 
> 
> #if IS_ENABLED(CONFIG_SECURITY_SELINUX)
> #define SELINUX_ENABLE 1,
> #else
> #define SELINUX_ENABLE
> #endif
> #if IS_ENABLED(CONFIG_SECURITY_XXXX)
> #define XXX_ENABLE 1,
> #else
> #define XXX_ENABLE
> #endif
> ....
> 
> #define MAX_LSM_COUNT COUNT_EXPAND(SELINUX_ENABLE XXX_ENABLE ......)

Oh, I love it! :) Yup, that should do it nicely.
diff mbox series

Patch

diff --git a/scripts/Makefile b/scripts/Makefile
index 1575af84d557..9712249c0fb3 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -41,6 +41,7 @@  targets += module.lds
 subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
 subdir-$(CONFIG_MODVERSIONS) += genksyms
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
+subdir-$(CONFIG_SECURITY) += security
 
 # Let clean descend into subdirs
 subdir-	+= basic dtc gdb kconfig mod
diff --git a/scripts/security/.gitignore b/scripts/security/.gitignore
new file mode 100644
index 000000000000..684af16735f1
--- /dev/null
+++ b/scripts/security/.gitignore
@@ -0,0 +1 @@ 
+gen_lsm_count
diff --git a/scripts/security/Makefile b/scripts/security/Makefile
new file mode 100644
index 000000000000..05f7e4109052
--- /dev/null
+++ b/scripts/security/Makefile
@@ -0,0 +1,4 @@ 
+# SPDX-License-Identifier: GPL-2.0
+hostprogs-always-y += gen_lsm_count
+HOST_EXTRACFLAGS += \
+	-I$(srctree)/include/uapi -I$(srctree)/include
diff --git a/scripts/security/gen_lsm_count.c b/scripts/security/gen_lsm_count.c
new file mode 100644
index 000000000000..a9a227724d84
--- /dev/null
+++ b/scripts/security/gen_lsm_count.c
@@ -0,0 +1,57 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+/* NOTE: we really do want to use the kernel headers here */
+#define __EXPORTED_HEADERS__
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+#include <ctype.h>
+
+#include <linux/kconfig.h>
+
+#define GEN_MAX_LSM_COUNT (				\
+	/* Capabilities */				\
+	IS_ENABLED(CONFIG_SECURITY) +			\
+	IS_ENABLED(CONFIG_SECURITY_SELINUX) +		\
+	IS_ENABLED(CONFIG_SECURITY_SMACK) +		\
+	IS_ENABLED(CONFIG_SECURITY_TOMOYO) +		\
+	IS_ENABLED(CONFIG_SECURITY_APPARMOR) +		\
+	IS_ENABLED(CONFIG_SECURITY_YAMA) +		\
+	IS_ENABLED(CONFIG_SECURITY_LOADPIN) +		\
+	IS_ENABLED(CONFIG_SECURITY_SAFESETID) +		\
+	IS_ENABLED(CONFIG_SECURITY_LOCKDOWN_LSM) + 	\
+	IS_ENABLED(CONFIG_BPF_LSM) + \
+	IS_ENABLED(CONFIG_SECURITY_LANDLOCK))
+
+const char *progname;
+
+static void usage(void)
+{
+	printf("usage: %s lsm_count.h\n", progname);
+	exit(1);
+}
+
+int main(int argc, char *argv[])
+{
+	FILE *fout;
+
+	progname = argv[0];
+
+	if (argc < 2)
+		usage();
+
+	fout = fopen(argv[1], "w");
+	if (!fout) {
+		fprintf(stderr, "Could not open %s for writing:  %s\n",
+			argv[1], strerror(errno));
+		exit(2);
+	}
+
+	fprintf(fout, "#ifndef _LSM_COUNT_H_\n#define _LSM_COUNT_H_\n\n");
+	fprintf(fout, "\n#define MAX_LSM_COUNT %d\n", GEN_MAX_LSM_COUNT);
+	fprintf(fout, "#endif /* _LSM_COUNT_H_ */\n");
+	exit(0);
+}
diff --git a/security/Makefile b/security/Makefile
index 18121f8f85cd..7a47174831f4 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -3,6 +3,7 @@ 
 # Makefile for the kernel security code
 #
 
+gen := include/generated
 obj-$(CONFIG_KEYS)			+= keys/
 
 # always enable default capabilities
@@ -27,3 +28,13 @@  obj-$(CONFIG_SECURITY_LANDLOCK)		+= landlock/
 
 # Object integrity file lists
 obj-$(CONFIG_INTEGRITY)			+= integrity/
+
+$(addprefix $(obj)/,$(obj-y)): $(gen)/lsm_count.h
+
+quiet_cmd_lsm_count = GEN     ${gen}/lsm_count.h
+      cmd_lsm_count = scripts/security/gen_lsm_count ${gen}/lsm_count.h
+
+targets += lsm_count.h
+
+${gen}/lsm_count.h: FORCE
+	$(call if_changed,lsm_count)