* Re: [PATCH V34 21/29] Lock down /proc/kcore
From: Kees Cook @ 2019-06-23 0:05 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Matthew Garrett
In-Reply-To: <20190622000358.19895-22-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:50PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> Disallow access to /proc/kcore when the kernel is locked down to prevent
> access to cryptographic data. This is limited to lockdown
> confidentiality mode and is still permitted in integrity mode.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
> fs/proc/kcore.c | 5 +++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
> index d29d869abec1..4e95edb1e282 100644
> --- a/fs/proc/kcore.c
> +++ b/fs/proc/kcore.c
> @@ -31,6 +31,7 @@
> #include <linux/ioport.h>
> #include <linux/memory.h>
> #include <linux/sched/task.h>
> +#include <linux/security.h>
> #include <asm/sections.h>
> #include "internal.h"
>
> @@ -545,6 +546,10 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)
>
> static int open_kcore(struct inode *inode, struct file *filp)
> {
> + int ret = security_locked_down(LOCKDOWN_KCORE);
> +
> + if (ret)
> + return ret;
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
Another capable() check ordering fix needed. With that:
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c649cb91e762..3875f6df2ecc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -95,6 +95,7 @@ enum lockdown_reason {
> LOCKDOWN_MODULE_PARAMETERS,
> LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> + LOCKDOWN_KCORE,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index cd86ed9f4d4b..4c9b324dfc55 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -31,6 +31,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> + [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 22/29] Lock down tracing and perf kprobes when in confidentiality mode
From: Kees Cook @ 2019-06-23 0:09 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Alexei Starovoitov, Matthew Garrett,
Naveen N . Rao, Anil S Keshavamurthy, davem, Masami Hiramatsu
In-Reply-To: <20190622000358.19895-23-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:51PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> Disallow the creation of perf and ftrace kprobes when the kernel is
> locked down in confidentiality mode by preventing their registration.
> This prevents kprobes from being used to access kernel memory to steal
> crypto data, but continues to allow the use of kprobes from signed
> modules.
>
> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: davem@davemloft.net
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> include/linux/security.h | 1 +
> kernel/trace/trace_kprobe.c | 5 +++++
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3875f6df2ecc..e6e3e2403474 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -96,6 +96,7 @@ enum lockdown_reason {
> LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> + LOCKDOWN_KPROBES,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5d5129b05df7..5a76a0f79d48 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -11,6 +11,7 @@
> #include <linux/uaccess.h>
> #include <linux/rculist.h>
> #include <linux/error-injection.h>
> +#include <linux/security.h>
>
> #include "trace_dynevent.h"
> #include "trace_kprobe_selftest.h"
> @@ -415,6 +416,10 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
> {
> int i, ret;
>
> + ret = security_locked_down(LOCKDOWN_KPROBES);
> + if (ret)
> + return ret;
> +
> if (trace_probe_is_registered(&tk->tp))
> return -EINVAL;
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 4c9b324dfc55..5a08c17f224d 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -32,6 +32,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> + [LOCKDOWN_KPROBES] = "use of kprobes",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Kees Cook @ 2019-06-23 0:09 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Alexei Starovoitov, Matthew Garrett, netdev,
Chun-Yi Lee, Daniel Borkmann
In-Reply-To: <20190622000358.19895-24-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:52PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> There are some bpf functions can be used to read kernel memory:
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program and kernel memory to be altered without
> restriction. Disable them if the kernel has been locked down in
> confidentiality mode.
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: netdev@vger.kernel.org
> cc: Chun-Yi Lee <jlee@suse.com>
> cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> ---
> include/linux/security.h | 1 +
> kernel/trace/bpf_trace.c | 20 +++++++++++++++++++-
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index e6e3e2403474..de0d37b1fe79 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -97,6 +97,7 @@ enum lockdown_reason {
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> LOCKDOWN_KPROBES,
> + LOCKDOWN_BPF_READ,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d64c00afceb5..638f9b00a8df 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -137,6 +137,10 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> ret = probe_kernel_read(dst, unsafe_ptr, size);
> if (unlikely(ret < 0))
> memset(dst, 0, size);
> @@ -156,6 +160,12 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
> BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
> u32, size)
> {
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * Ensure we're in user context which is safe for the helper to
> * run. This helper has no business in a kthread.
> @@ -205,7 +215,11 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
> int fmt_cnt = 0;
> u64 unsafe_addr;
> char buf[64];
> - int i;
> + int i, ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
>
> /*
> * bpf_check()->check_func_arg()->check_stack_boundary()
> @@ -534,6 +548,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * The strncpy_from_unsafe() call will likely not fill the entire
> * buffer, but that's okay in this circumstance as we're probing
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 5a08c17f224d..2eea2cc13117 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> + [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 24/29] Lock down perf when in confidentiality mode
From: Kees Cook @ 2019-06-23 0:12 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Matthew Garrett, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo
In-Reply-To: <20190622000358.19895-25-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:53PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> Disallow the use of certain perf facilities that might allow userspace to
> access kernel data.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> ---
> include/linux/security.h | 1 +
> kernel/events/core.c | 7 +++++++
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 9 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index de0d37b1fe79..53ea85889a48 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -98,6 +98,7 @@ enum lockdown_reason {
> LOCKDOWN_KCORE,
> LOCKDOWN_KPROBES,
> LOCKDOWN_BPF_READ,
> + LOCKDOWN_PERF,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 72d06e302e99..77f36551756e 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10731,6 +10731,13 @@ SYSCALL_DEFINE5(perf_event_open,
> return -EINVAL;
> }
>
> + err = security_locked_down(LOCKDOWN_PERF);
> + if (err && (attr.sample_type & PERF_SAMPLE_REGS_INTR))
> + /* REGS_INTR can leak data, lockdown must prevent this */
> + return err;
> + else
> + err = 0;
> +
> /* Only privileged users can get physical addresses */
> if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
> perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
With moar capable() ordering fixed...
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 2eea2cc13117..a7e75c614416 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -34,6 +34,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
> + [LOCKDOWN_PERF] = "unsafe use of perf",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 28/29] efi: Restrict efivar_ssdt_load when the kernel is locked down
From: Kees Cook @ 2019-06-23 0:14 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Matthew Garrett, Ard Biesheuvel, linux-efi
In-Reply-To: <20190622000358.19895-29-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:57PM -0700, Matthew Garrett wrote:
> efivar_ssdt_load allows the kernel to import arbitrary ACPI code from an
> EFI variable, which gives arbitrary code execution in ring 0. Prevent
> that when the kernel is locked down.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: linux-efi@vger.kernel.org
> ---
> drivers/firmware/efi/efi.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 55b77c576c42..9f92a013ab27 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -31,6 +31,7 @@
> #include <linux/acpi.h>
> #include <linux/ucs2_string.h>
> #include <linux/memblock.h>
> +#include <linux/security.h>
>
> #include <asm/early_ioremap.h>
>
> @@ -242,6 +243,11 @@ static void generic_ops_unregister(void)
> static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata;
> static int __init efivar_ssdt_setup(char *str)
> {
> + int ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
> +
> + if (ret)
> + return ret;
> +
> if (strlen(str) < sizeof(efivar_ssdt))
> memcpy(efivar_ssdt, str, strlen(str));
> else
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 29/29] lockdown: Print current->comm in restriction messages
From: Kees Cook @ 2019-06-23 0:25 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Matthew Garrett
In-Reply-To: <20190622000358.19895-30-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:58PM -0700, Matthew Garrett wrote:
> Print the content of current->comm in messages generated by lockdown to
> indicate a restriction that was hit. This makes it a bit easier to find
> out what caused the message.
>
> The message now patterned something like:
>
> Lockdown: <comm>: <what> is restricted; see man kernel_lockdown.7
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
> security/lockdown/lockdown.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 98f9ee0026d5..9ca6f442fbc7 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -83,8 +83,8 @@ static int lockdown_is_locked_down(enum lockdown_reason what)
> {
> if ((kernel_locked_down >= what)) {
To satisfy my paranoia, can you just add here:
if (WARN(what > LOCKDOWN_..._MAX))
return -EPERM;
With that:
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> if (lockdown_reasons[what])
> - pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
> - lockdown_reasons[what]);
> + pr_notice("Lockdown: %s: %s is restricted; see man kernel_lockdown.7\n",
> + current->comm, lockdown_reasons[what]);
> return -EPERM;
> }
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 22/29] Lock down tracing and perf kprobes when in confidentiality mode
From: Masami Hiramatsu @ 2019-06-23 1:57 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Alexei Starovoitov, Matthew Garrett,
Naveen N . Rao, Anil S Keshavamurthy, davem, Masami Hiramatsu
In-Reply-To: <20190622000358.19895-23-matthewgarrett@google.com>
On Fri, 21 Jun 2019 17:03:51 -0700
Matthew Garrett <matthewgarrett@google.com> wrote:
> From: David Howells <dhowells@redhat.com>
>
> Disallow the creation of perf and ftrace kprobes when the kernel is
> locked down in confidentiality mode by preventing their registration.
> This prevents kprobes from being used to access kernel memory to steal
> crypto data, but continues to allow the use of kprobes from signed
> modules.
Looks (and sounds) good to me.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you,
>
> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: davem@davemloft.net
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> include/linux/security.h | 1 +
> kernel/trace/trace_kprobe.c | 5 +++++
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3875f6df2ecc..e6e3e2403474 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -96,6 +96,7 @@ enum lockdown_reason {
> LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> + LOCKDOWN_KPROBES,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5d5129b05df7..5a76a0f79d48 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -11,6 +11,7 @@
> #include <linux/uaccess.h>
> #include <linux/rculist.h>
> #include <linux/error-injection.h>
> +#include <linux/security.h>
>
> #include "trace_dynevent.h"
> #include "trace_kprobe_selftest.h"
> @@ -415,6 +416,10 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
> {
> int i, ret;
>
> + ret = security_locked_down(LOCKDOWN_KPROBES);
> + if (ret)
> + return ret;
> +
> if (trace_probe_is_registered(&tk->tp))
> return -EINVAL;
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 4c9b324dfc55..5a08c17f224d 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -32,6 +32,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> + [LOCKDOWN_KPROBES] = "use of kprobes",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH V34 20/29] x86/mmiotrace: Lock down the testmmiotrace module
From: Thomas Gleixner @ 2019-06-23 11:08 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Matthew Garrett, Steven Rostedt, Ingo Molnar,
H. Peter Anvin, x86
In-Reply-To: <20190622000358.19895-21-matthewgarrett@google.com>
On Fri, 21 Jun 2019, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> The testmmiotrace module shouldn't be permitted when the kernel is locked
> down as it can be used to arbitrarily read and write MMIO space. This is
> a runtime check rather than buildtime in order to allow configurations
> where the same kernel may be run in both locked down or permissive modes
> depending on local policy.
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: David Howells <dhowells@redhat.com
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: Thomas Gleixner <tglx@linutronix.de>
> cc: Steven Rostedt <rostedt@goodmis.org>
> cc: Ingo Molnar <mingo@kernel.org>
> cc: "H. Peter Anvin" <hpa@zytor.com>
> cc: x86@kernel.org
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply
* Re: [RFC PATCH v3 09/12] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX
From: Dr. Greg @ 2019-06-23 17:16 UTC (permalink / raw)
To: James Morris
Cc: Jarkko Sakkinen, Sean Christopherson, linux-sgx, Dave Hansen,
Cedric Xing, Andy Lutomirski, Jethro Beekman, Stephen Smalley,
casey.schaufler, william.c.roberts, linux-security-module,
selinux
In-Reply-To: <alpine.LRH.2.21.1906200702040.28119@namei.org>
On Thu, Jun 20, 2019 at 07:13:50AM +1000, James Morris wrote:
Good morning, I hope the weekend has been going well for everyone.
> On Wed, 19 Jun 2019, Jarkko Sakkinen wrote:
>
> > Can LSM callbacks ever non-generic when it comes to hardware? This is
> > the very first time I ever see such callbacks being introduced.
> >
> > I suspect that from maintainers perspective, accepting such changes for
> > Intel hardware, could open a pandoras box.
> If there's a major distro/userbase committing to ship with these
> hooks enabled via a supported in-tree LSM, the case for inclusion is
> clear.
I see that Jarkko responded down thread that there may be a major
distribution already committed to SGX specific LSM hooks. My
apologies for providing these reflections if that is the case and
there is some type of 'deal' in place with respect to all of this.
> If the hooks could be generalized beyond just SGX, that would be
> ideal, but it's not clear if that's feasible.
We believe there is some degree of commonality that can be addressed
with respect to implementing LSM enforcement over SGX enclaves.
However, big picture, here is the challenge that we see with respect
to these conversations surrounding the integration of the SGX driver
with the LSM:
As a technology, SGX was designed to enable software to protect itself
from an adversarial operating system and hardware platform. Given
that, if we are intellectually honest, how effective can the LSM/OS be
with respect to controlling the actions of an enclave?
Without question, being able to regulate and control which identities
can intersect to load executable content into an enclave is important.
All of the infrastructure appears to be already there to accomplish
that, given the default model of a shared library implementation of an
enclave and requiring the loader to mmap file backed TEXT pages RX.
The most relevant and important control with respect to whether or not
an enclave should be allowed to execute is evaluation of the
SIGSTRUCT. Given the trajectory that platform security is on, SGX is
not going to be the last technology of its type nor the only
technology that makes use of cryptographically based code provenance.
As a result, if we are content with handing an opaque pointer of a
descriptive struture to an LSM routine, a generic hook that is tasked
with verifying code or execution environment provenance doesn't seem
like it would need to be technology specific nor controversial.
That leaves as the last thorny issue the question of dynamic
allocation of memory for executable content. As we have stated
before, and at the outset of this note, from a security perspective
this is only, effectively, a binary question for the platform owner as
to whether or not the concept should be allowed.
A generic LSM hook, appropriately named, could execute that decision
without being SGX specific. Arguably, the hook should be named to
indicate that it is seeking approval for allocating memory to be used
for anonymous executable content, since that is what it would be
effectively requesting approval for, in the case of SGX.
For completeness a third generic hook may be useful. The purpose of
that hook would be to verify a block of memory as being
measured or signed for consideration as executable content. Arguably
that will have utility far beyond SGX.
In the case of SGX it would address the issue as to whether or not a
block of executable content in untrusted space is eligible for
anonymous execution. That may be a useful security measure in order
to provide some control over an enclave being used as a random
execution oracle.
It obviously has no security utility against the enclave author since,
as we have noted before, it is possible for the enclave author to
simply pull whatever code is desired over an encrypted network
connection.
> James Morris
Hopefully these comments are a useful basis for further discussion.
Best wishes for a productive week to everyone.
Dr. Greg
As always,
Dr. Greg Wettstein, Ph.D, Worker
IDfusion, LLC
4206 N. 19th Ave. Implementing measured information privacy
Fargo, ND 58102 and integrity architectures.
PH: 701-281-1686
FAX: 701-281-3949 EMAIL: gw@idfusion.org
------------------------------------------------------------------------------
"My thoughts on trusting Open-Source? A quote I once saw said it
best: 'Remember, Amateurs built the ark. Professionals built the
Titanic.' Perhaps most significantly the ark was one guy, there were
no doubt committees involved with the Titanic project."
-- Dr. G.W. Wettstein
Resurrection
^ permalink raw reply
* Re: [PATCH V31 07/25] kexec_file: Restrict at runtime if the kernel is locked down
From: Dave Young @ 2019-06-24 1:52 UTC (permalink / raw)
To: Matthew Garrett
Cc: James Morris, Jiri Bohac, Linux API, kexec,
Linux Kernel Mailing List, David Howells, LSM List,
Andy Lutomirski
In-Reply-To: <CACdnJut=J1YTpM4s6g5XWCEs+=X0Jvf8otfMg+w=_oqSZmf01Q@mail.gmail.com>
On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> On Thu, Jun 20, 2019 at 11:43 PM Dave Young <dyoung@redhat.com> wrote:
> >
> > On 03/26/19 at 11:27am, Matthew Garrett wrote:
> > > From: Jiri Bohac <jbohac@suse.cz>
> > >
> > > When KEXEC_SIG is not enabled, kernel should not load images through
> > > kexec_file systemcall if the kernel is locked down.
> > >
> > > [Modified by David Howells to fit with modifications to the previous patch
> > > and to return -EPERM if the kernel is locked down for consistency with
> > > other lockdowns. Modified by Matthew Garrett to remove the IMA
> > > integration, which will be replaced by integrating with the IMA
> > > architecture policy patches.]
> > >
> > > Signed-off-by: Jiri Bohac <jbohac@suse.cz>
> > > Signed-off-by: David Howells <dhowells@redhat.com>
> > > Signed-off-by: Matthew Garrett <mjg59@google.com>
> > > Reviewed-by: Jiri Bohac <jbohac@suse.cz>
> > > cc: kexec@lists.infradead.org
> > > ---
> > > kernel/kexec_file.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > > index 67f3a866eabe..a1cc37c8b43b 100644
> > > --- a/kernel/kexec_file.c
> > > +++ b/kernel/kexec_file.c
> > > @@ -239,6 +239,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> > > }
> > >
> > > ret = 0;
> > > +
> > > + if (kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY)) {
> > > + ret = -EPERM;
> > > + goto out;
> > > + }
> > > +
> >
> > Checking here is late, it would be good to move the check to earlier
> > code around below code:
> > /* We only trust the superuser with rebooting the system. */
> > if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> > return -EPERM;
>
> I don't think so - we want it to be possible to load images if they
> have a valid signature.
I know it works like this way because of the previous patch. But from
the patch log "When KEXEC_SIG is not enabled, kernel should not load
images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY) instead of depending
on the late code to verify signature. In that way, easier to
understand the logic, no?
Thanks
Dave
^ permalink raw reply
* Re: [PATCH V34 08/29] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
From: Dave Young @ 2019-06-24 2:01 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Jiri Bohac, David Howells, Matthew Garrett, kexec
In-Reply-To: <20190622000358.19895-9-matthewgarrett@google.com>
On 06/21/19 at 05:03pm, Matthew Garrett wrote:
> From: Jiri Bohac <jbohac@suse.cz>
>
> This is a preparatory patch for kexec_file_load() lockdown. A locked down
> kernel needs to prevent unsigned kernel images from being loaded with
> kexec_file_load(). Currently, the only way to force the signature
> verification is compiling with KEXEC_VERIFY_SIG. This prevents loading
> usigned images even when the kernel is not locked down at runtime.
>
> This patch splits KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE.
> Analogous to the MODULE_SIG and MODULE_SIG_FORCE for modules, KEXEC_SIG
> turns on the signature verification but allows unsigned images to be
> loaded. KEXEC_SIG_FORCE disallows images without a valid signature.
>
> [Modified by David Howells such that:
>
> (1) verify_pefile_signature() differentiates between no-signature and
> sig-didn't-match in its returned errors.
>
> (2) kexec fails with EKEYREJECTED if there is a signature for which we
> have a key, but signature doesn't match - even if in non-forcing mode.
>
> (3) kexec fails with EBADMSG or some other error if there is a signature
> which cannot be parsed - even if in non-forcing mode.
>
> (4) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
> the signature - even if in non-forcing mode.
>
> ]
Seems I do not see EBADMSG and ELIBBAD in this patch, also kexec fails
with proper errno instead of EKEYREJECTED only.
I may missed something? Other than the patch log issue:
Reviewed-by: Dave Young <dyoung@redhat.com>
>
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Reviewed-by: Jiri Bohac <jbohac@suse.cz>
> cc: kexec@lists.infradead.org
> ---
> arch/x86/Kconfig | 20 ++++++++---
> crypto/asymmetric_keys/verify_pefile.c | 4 ++-
> include/linux/kexec.h | 4 +--
> kernel/kexec_file.c | 47 ++++++++++++++++++++++----
> 4 files changed, 60 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c1f9b3cf437c..84381dd60760 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2012,20 +2012,30 @@ config KEXEC_FILE
> config ARCH_HAS_KEXEC_PURGATORY
> def_bool KEXEC_FILE
>
> -config KEXEC_VERIFY_SIG
> +config KEXEC_SIG
> bool "Verify kernel signature during kexec_file_load() syscall"
> depends on KEXEC_FILE
> ---help---
> - This option makes kernel signature verification mandatory for
> - the kexec_file_load() syscall.
>
> - In addition to that option, you need to enable signature
> + This option makes the kexec_file_load() syscall check for a valid
> + signature of the kernel image. The image can still be loaded without
> + a valid signature unless you also enable KEXEC_SIG_FORCE, though if
> + there's a signature that we can check, then it must be valid.
> +
> + In addition to this option, you need to enable signature
> verification for the corresponding kernel image type being
> loaded in order for this to work.
>
> +config KEXEC_SIG_FORCE
> + bool "Require a valid signature in kexec_file_load() syscall"
> + depends on KEXEC_SIG
> + ---help---
> + This option makes kernel signature verification mandatory for
> + the kexec_file_load() syscall.
> +
> config KEXEC_BZIMAGE_VERIFY_SIG
> bool "Enable bzImage signature verification support"
> - depends on KEXEC_VERIFY_SIG
> + depends on KEXEC_SIG
> depends on SIGNED_PE_FILE_VERIFICATION
> select SYSTEM_TRUSTED_KEYRING
> ---help---
> diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
> index d178650fd524..4473cea1e877 100644
> --- a/crypto/asymmetric_keys/verify_pefile.c
> +++ b/crypto/asymmetric_keys/verify_pefile.c
> @@ -100,7 +100,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
>
> if (!ddir->certs.virtual_address || !ddir->certs.size) {
> pr_debug("Unsigned PE binary\n");
> - return -EKEYREJECTED;
> + return -ENODATA;
> }
>
> chkaddr(ctx->header_size, ddir->certs.virtual_address,
> @@ -408,6 +408,8 @@ static int pefile_digest_pe(const void *pebuf, unsigned int pelen,
> * (*) 0 if at least one signature chain intersects with the keys in the trust
> * keyring, or:
> *
> + * (*) -ENODATA if there is no signature present.
> + *
> * (*) -ENOPKG if a suitable crypto module couldn't be found for a check on a
> * chain.
> *
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index b9b1bc5f9669..58b27c7bdc2b 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -125,7 +125,7 @@ typedef void *(kexec_load_t)(struct kimage *image, char *kernel_buf,
> unsigned long cmdline_len);
> typedef int (kexec_cleanup_t)(void *loader_data);
>
> -#ifdef CONFIG_KEXEC_VERIFY_SIG
> +#ifdef CONFIG_KEXEC_SIG
> typedef int (kexec_verify_sig_t)(const char *kernel_buf,
> unsigned long kernel_len);
> #endif
> @@ -134,7 +134,7 @@ struct kexec_file_ops {
> kexec_probe_t *probe;
> kexec_load_t *load;
> kexec_cleanup_t *cleanup;
> -#ifdef CONFIG_KEXEC_VERIFY_SIG
> +#ifdef CONFIG_KEXEC_SIG
> kexec_verify_sig_t *verify_sig;
> #endif
> };
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index f1d0e00a3971..eec7e5bb2a08 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -90,7 +90,7 @@ int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
> return kexec_image_post_load_cleanup_default(image);
> }
>
> -#ifdef CONFIG_KEXEC_VERIFY_SIG
> +#ifdef CONFIG_KEXEC_SIG
> static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
> unsigned long buf_len)
> {
> @@ -188,7 +188,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> const char __user *cmdline_ptr,
> unsigned long cmdline_len, unsigned flags)
> {
> - int ret = 0;
> + const char *reason;
> + int ret;
> void *ldata;
> loff_t size;
>
> @@ -207,15 +208,47 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> if (ret)
> goto out;
>
> -#ifdef CONFIG_KEXEC_VERIFY_SIG
> +#ifdef CONFIG_KEXEC_SIG
> ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
> image->kernel_buf_len);
> - if (ret) {
> - pr_debug("kernel signature verification failed.\n");
> +#else
> + ret = -ENODATA;
> +#endif
> +
> + switch (ret) {
> + case 0:
> + break;
> +
> + /* Certain verification errors are non-fatal if we're not
> + * checking errors, provided we aren't mandating that there
> + * must be a valid signature.
> + */
> + case -ENODATA:
> + reason = "kexec of unsigned image";
> + goto decide;
> + case -ENOPKG:
> + reason = "kexec of image with unsupported crypto";
> + goto decide;
> + case -ENOKEY:
> + reason = "kexec of image with unavailable key";
> + decide:
> + if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
> + pr_notice("%s rejected\n", reason);
> + goto out;
> + }
> +
> + ret = 0;
> + break;
> +
> + /* All other errors are fatal, including nomem, unparseable
> + * signatures and signature check failures - even if signatures
> + * aren't required.
> + */
> + default:
> + pr_notice("kernel signature verification failed (%d).\n", ret);
> goto out;
> }
> - pr_debug("kernel signature verification successful.\n");
> -#endif
> +
> /* It is possible that there no initramfs is being loaded */
> if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
> ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply
* [PATCH V10 0/3] Add support for measuring the boot command line during kexec_file_load
From: Prakhar Srivastava @ 2019-06-24 6:23 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
The kexec boot command line arguments are not currently being
measured.
Currently during soft reboot(kexec)
- the PCRS are not reset
- the command line arguments used for the next kernel are not measured.
This gives the impression to the secure boot attestation that a cold boot took
place.
For secure boot attestation, it is necessary to measure the kernel
command line. For cold boot, the boot loader can be enhanced to measure
these parameters.
(https://mjg59.dreamwidth.org/48897.html)
This patch set aims to address measuring the boot command line during
soft reboot(kexec_file_load).
To achive the above the patch series does the following
-Add a new ima hook: ima_kexec_cmdline which measures the cmdline args
into the ima log, behind a new ima policy entry KEXEC_CMDLINE.
The kexec cmdline hash is stored in the "d-ng" field of the template data.
-Since the cmldine args cannot be appraised, a new template field(buf) is
added. The template field contains the buffer passed(cmldine args), which
can be used to appraise/attest at a later stage.
The kexec cmdline buffer is stored as HEX in the buf field of the event_data.
-Call the ima_kexec_cmdline(...) hook from kexec_file_load call.
The ima logs need to be carried over to the next kernel, which will be followed
up by other patchsets for x86_64 and arm64.
The kexec cmdline hash is stored in the "d-ng" field of the template data.
and can be verified using
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
grep kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum
Changelog:
V10(since V9):
-rebased over next-queued-integrity
-code cleanup
V9(since V8):
- code cleanup
V8(since V7):
- added a new ima template name "ima-buf"
- code cleanup
V7:
- rebased to next-queued-testing
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/log/?h=next-queued-testing
V6:
-add a new ima hook and policy to measure the cmdline
args(ima_kexec_cmdline)
-add a new template field buf to contain the buffer measured.
[suggested by Mimi Zohar]
add new fields to ima_event_data to store/read buffer data.
[suggested by Roberto]
-call ima_kexec_cmdline from kexec_file_load path
v5:
-add a new ima hook and policy to measure the cmdline
args(ima_kexec_cmdline)
-add a new template field buf to contain the buffer measured.
[suggested by Mimi Zohar]
-call ima_kexec_cmdline from kexec_file_load path
v4:
- per feedback from LSM community, removed the LSM hook and renamed the
IMA policy to KEXEC_CMDLINE
v3: (rebase changes to next-general)
- Add policy checks for buffer[suggested by Mimi Zohar]
- use the IMA_XATTR to add buffer
- Add kexec_cmdline used for kexec file load
- Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]
v2:
- Add policy checks for buffer[suggested by Mimi Zohar]
- Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]
- use the IMA_XATTR to add buffer instead of sig template
v1:
-Add kconfigs to control the ima_buffer_check
-measure the cmdline args suffixed with the kernel file name
-add the buffer to the template sig field.
Prakhar Srivastava (3):
Add a new ima hook ima_kexec_cmdline to measure cmdline args
add a new ima template field buf
call ima_kexec_cmdline to measure the cmdline args
Documentation/ABI/testing/ima_policy | 1 +
Documentation/security/IMA-templates.rst | 2 +-
include/linux/ima.h | 2 +
kernel/kexec_file.c | 8 ++-
security/integrity/ima/ima.h | 3 +
security/integrity/ima/ima_api.c | 5 +-
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_main.c | 80 +++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 9 +++
security/integrity/ima/ima_template.c | 2 +
security/integrity/ima/ima_template_lib.c | 20 ++++++
security/integrity/ima/ima_template_lib.h | 4 ++
12 files changed, 131 insertions(+), 7 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH V10 3/3] KEXEC: Call ima_kexec_cmdline to measure the boot command line args
From: Prakhar Srivastava @ 2019-06-24 6:23 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
In-Reply-To: <20190624062331.388-1-prsriva02@gmail.com>
During soft reboot(kexec_file_load) boot command line
arguments are not measured.
Call ima hook ima_kexec_cmdline to measure the boot command line
arguments into IMA measurement list.
- call ima_kexec_cmdline from kexec_file_load.
- move the call ima_add_kexec_buffer after the cmdline
args have been measured.
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
Acked-by: Dave Young <dyoung@redhat.com>
---
kernel/kexec_file.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 072b6ee55e3f..b0c724e5d86c 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -198,9 +198,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
return ret;
image->kernel_buf_len = size;
- /* IMA needs to pass the measurement list to the next kernel. */
- ima_add_kexec_buffer(image);
-
/* Call arch image probe handlers */
ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
image->kernel_buf_len);
@@ -241,8 +238,14 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
ret = -EINVAL;
goto out;
}
+
+ ima_kexec_cmdline(image->cmdline_buf,
+ image->cmdline_buf_len - 1);
}
+ /* IMA needs to pass the measurement list to the next kernel. */
+ ima_add_kexec_buffer(image);
+
/* Call arch image load handlers */
ldata = arch_kexec_kernel_image_load(image);
--
2.19.1
^ permalink raw reply related
* [PATCH V10 2/3] IMA: Define a new template field buf
From: Prakhar Srivastava @ 2019-06-24 6:23 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
In-Reply-To: <20190624062331.388-1-prsriva02@gmail.com>
A buffer(kexec boot command line arguments) measured into IMA
measuremnt list cannot be appraised, without already being
aware of the buffer contents. Since hashes are non-reversible,
raw buffer is needed for validation or regenerating hash for
appraisal/attestation.
Add support to store/read the buffer contents in HEX.
The kexec cmdline hash is stored in the "d-ng" field of the
template data,it can be verified using
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
grep kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum
- Add two new fields to ima_event_data to hold the buf and
buf_len [Suggested by Roberto]
- Add a new temaplte field 'buf' to be used to store/read
the buffer data.[Suggested by Mimi]
- Updated process_buffer_meaurement to add the buffer to
ima_event_data. process_buffer_measurement added in
"Define a new IMA hook to measure the boot command line
arguments"
- Add a new template policy name ima-buf to represent
'd-ng|n-ng|buf'
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
---
Documentation/security/IMA-templates.rst | 7 ++++---
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_main.c | 4 +++-
security/integrity/ima/ima_template.c | 3 +++
security/integrity/ima/ima_template_lib.c | 21 +++++++++++++++++++++
security/integrity/ima/ima_template_lib.h | 4 ++++
6 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 2cd0e273cc9a..3d1cca287aa4 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -69,15 +69,16 @@ descriptors by adding their identifier to the format string
algorithm (field format: [<hash algo>:]digest, where the digest
prefix is shown only if the hash algorithm is not SHA1 or MD5);
- 'n-ng': the name of the event, without size limitations;
- - 'sig': the file signature.
+ - 'sig': the file signature;
+ - 'buf': the buffer data that was used to generate the hash without size limitations;
Below, there is the list of defined template descriptors:
- "ima": its format is ``d|n``;
- "ima-ng" (default): its format is ``d-ng|n-ng``;
- - "ima-sig": its format is ``d-ng|n-ng|sig``.
-
+ - "ima-sig": its format is ``d-ng|n-ng|sig``;
+ - "ima-buf": its format is ``d-ng|n-ng|buf``;
Use
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index bdca641f9e51..6aa28ab53d27 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -65,6 +65,8 @@ struct ima_event_data {
struct evm_ima_xattr_data *xattr_value;
int xattr_len;
const char *violation;
+ const void *buf;
+ int buf_len;
};
/* IMA template field data definition */
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2507bee1b762..317c4b6f2c18 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -627,7 +627,9 @@ static void process_buffer_measurement(const void *buf, int size,
struct ima_template_entry *entry = NULL;
struct integrity_iint_cache iint = {};
struct ima_event_data event_data = {.iint = &iint,
- .filename = eventname};
+ .filename = eventname,
+ .buf = buf,
+ .buf_len = size};
struct ima_template_desc *template_desc = NULL;
struct {
struct ima_digest_data hdr;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 00dd5a434689..a01a17e5c581 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -26,6 +26,7 @@ static struct ima_template_desc builtin_templates[] = {
{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
{.name = "ima-ng", .fmt = "d-ng|n-ng"},
{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
+ {.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
{.name = "", .fmt = ""}, /* placeholder for a custom format */
};
@@ -43,6 +44,8 @@ static const struct ima_template_field supported_fields[] = {
.field_show = ima_show_template_string},
{.field_id = "sig", .field_init = ima_eventsig_init,
.field_show = ima_show_template_sig},
+ {.field_id = "buf", .field_init = ima_eventbuf_init,
+ .field_show = ima_show_template_buf},
};
#define MAX_TEMPLATE_NAME_LEN 15
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 513b457ae900..baf4de45c5aa 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -162,6 +162,12 @@ void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
}
+void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data)
+{
+ ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
+}
+
/**
* ima_parse_buf() - Parses lengths and data from an input buffer
* @bufstartp: Buffer start address.
@@ -389,3 +395,18 @@ int ima_eventsig_init(struct ima_event_data *event_data,
return ima_write_template_field_data(xattr_value, event_data->xattr_len,
DATA_FMT_HEX, field_data);
}
+
+/*
+ * ima_eventbuf_init - include the buffer(kexec-cmldine) as part of the
+ * template data.
+ */
+int ima_eventbuf_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ if ((!event_data->buf) || (event_data->buf_len == 0))
+ return 0;
+
+ return ima_write_template_field_data(event_data->buf,
+ event_data->buf_len, DATA_FMT_HEX,
+ field_data);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6a3d8b831deb..12f1a8578b31 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -29,6 +29,8 @@ void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
+void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data);
int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
int maxfields, struct ima_field_data *fields, int *curfields,
unsigned long *len_mask, int enforce_mask, char *bufname);
@@ -42,4 +44,6 @@ int ima_eventname_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventsig_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventbuf_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
#endif /* __LINUX_IMA_TEMPLATE_LIB_H */
--
2.19.1
^ permalink raw reply related
* [PATCH V10 1/3] IMA: Define a new hook to measure the kexec boot command line arguments
From: Prakhar Srivastava @ 2019-06-24 6:23 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
In-Reply-To: <20190624062331.388-1-prsriva02@gmail.com>
Currently during soft reboot(kexec_file_load) boot command line
arguments are not measured. Define hooks needed to measure kexec
command line arguments during soft reboot(kexec_file_load).
- A new ima hook ima_kexec_cmdline is defined to be called by the
kexec code.
- A new function process_buffer_measurement is defined to measure
the buffer hash into the IMA measurement list.
- A new func policy KEXEC_CMDLINE is defined to control the
measurement.[Suggested by Mimi]
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
Documentation/ABI/testing/ima_policy | 1 +
include/linux/ima.h | 2 +
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 1 +
security/integrity/ima/ima_main.c | 72 ++++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 7 +++
6 files changed, 84 insertions(+)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index b383c1763610..fc376a323908 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -28,6 +28,7 @@ Description:
base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
+ [KEXEC_CMDLINE]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
diff --git a/include/linux/ima.h b/include/linux/ima.h
index fd9f7cf4cdf5..b42f5a006042 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -26,6 +26,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
enum kernel_read_file_id id);
extern void ima_post_path_mknod(struct dentry *dentry);
+extern void ima_kexec_cmdline(const void *buf, int size);
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
@@ -92,6 +93,7 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
return;
}
+static inline void ima_kexec_cmdline(const void *buf, int size) {}
#endif /* CONFIG_IMA */
#ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e7b9ea7732d9..bdca641f9e51 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -190,6 +190,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
hook(KEXEC_KERNEL_CHECK) \
hook(KEXEC_INITRAMFS_CHECK) \
hook(POLICY_CHECK) \
+ hook(KEXEC_CMDLINE) \
hook(MAX_CHECK)
#define __ima_hook_enumify(ENUM) ENUM,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c0cf4bcfc82f..d426d4d1fe04 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -178,6 +178,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* subj=, obj=, type=, func=, mask=, fsmagic=
* subj,obj, and type: are LSM specific.
* func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
+ * | KEXEC_CMDLINE
* mask: contains the permission mask
* fsmagic: hex value
*
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index a7e7e2d7224c..2507bee1b762 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -609,6 +609,78 @@ int ima_load_data(enum kernel_load_data_id id)
return 0;
}
+/*
+ * process_buffer_measurement - Measure the buffer to ima log.
+ * @buf: pointer to the buffer that needs to be added to the log.
+ * @size: size of buffer(in bytes).
+ * @eventname: event name to be used for the buffer entry.
+ * @cred: a pointer to a credentials structure for user validation.
+ * @secid: the secid of the task to be validated.
+ *
+ * Based on policy, the buffer is measured into the ima log.
+ */
+static void process_buffer_measurement(const void *buf, int size,
+ const char *eventname,
+ const struct cred *cred, u32 secid)
+{
+ int ret = 0;
+ struct ima_template_entry *entry = NULL;
+ struct integrity_iint_cache iint = {};
+ struct ima_event_data event_data = {.iint = &iint,
+ .filename = eventname};
+ struct ima_template_desc *template_desc = NULL;
+ struct {
+ struct ima_digest_data hdr;
+ char digest[IMA_MAX_DIGEST_SIZE];
+ } hash = {};
+ int violation = 0;
+ int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+ int action = 0;
+
+ action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr,
+ &template_desc);
+ if (!(action & IMA_MEASURE))
+ return;
+
+ iint.ima_hash = &hash.hdr;
+ iint.ima_hash->algo = ima_hash_algo;
+ iint.ima_hash->length = hash_digest_size[ima_hash_algo];
+
+ ret = ima_calc_buffer_hash(buf, size, iint.ima_hash);
+ if (ret < 0)
+ goto out;
+
+ ret = ima_alloc_init_template(&event_data, &entry, template_desc);
+ if (ret < 0)
+ goto out;
+
+ ret = ima_store_template(entry, violation, NULL, buf, pcr);
+
+ if (ret < 0)
+ ima_free_template_entry(entry);
+
+out:
+ return;
+}
+
+/**
+ * ima_kexec_cmdline - measure kexec cmdline boot args
+ * @buf: pointer to buffer
+ * @size: size of buffer
+ *
+ * Buffers can only be measured, not appraised.
+ */
+void ima_kexec_cmdline(const void *buf, int size)
+{
+ u32 secid;
+
+ if (buf && size != 0) {
+ security_task_getsecid(current, &secid);
+ process_buffer_measurement(buf, size, "kexec-cmdline",
+ current_cred(), secid);
+ }
+}
+
static int __init init_ima(void)
{
int error;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 98c289559079..a3058b03a955 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -374,6 +374,11 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
{
int i;
+ if (func == KEXEC_CMDLINE) {
+ if ((rule->flags & IMA_FUNC) && (rule->func == func))
+ return true;
+ return false;
+ }
if ((rule->flags & IMA_FUNC) &&
(rule->func != func && func != POST_SETATTR))
return false;
@@ -956,6 +961,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->func = KEXEC_INITRAMFS_CHECK;
else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
entry->func = POLICY_CHECK;
+ else if (strcmp(args[0].from, "KEXEC_CMDLINE") == 0)
+ entry->func = KEXEC_CMDLINE;
else
result = -EINVAL;
if (!result)
--
2.19.1
^ permalink raw reply related
* Re: [PATCH V10 1/3] IMA: Define a new hook to measure the kexec boot command line arguments
From: Mimi Zohar @ 2019-06-24 12:39 UTC (permalink / raw)
To: Prakhar Srivastava, linux-integrity, linux-security-module,
linux-kernel
Cc: roberto.sassu, vgoyal
In-Reply-To: <20190624062331.388-2-prsriva02@gmail.com>
Hi Prakhar,
On Sun, 2019-06-23 at 23:23 -0700, Prakhar Srivastava wrote:
> Currently during soft reboot(kexec_file_load) boot command line
> arguments are not measured. Define hooks needed to measure kexec
> command line arguments during soft reboot(kexec_file_load).
>
> - A new ima hook ima_kexec_cmdline is defined to be called by the
> kexec code.
> - A new function process_buffer_measurement is defined to measure
> the buffer hash into the IMA measurement list.
> - A new func policy KEXEC_CMDLINE is defined to control the
> measurement.[Suggested by Mimi]
>
> Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
Thanks! This patch set is now queued in the next-queued-testing
branch for any last minute comments or Reviews/Acks, before being
staged in the next-integrity branch.
Mimi
^ permalink raw reply
* Re: [PATCH V34 10/29] hibernate: Disable when the kernel is locked down
From: Jiri Kosina @ 2019-06-24 13:21 UTC (permalink / raw)
To: Pavel Machek
Cc: Matthew Garrett, jmorris, linux-security-module, linux-kernel,
linux-api, Josh Boyer, David Howells, Matthew Garrett, rjw,
Joey Lee, linux-pm
In-Reply-To: <20190622175208.GB30317@amd>
On Sat, 22 Jun 2019, Pavel Machek wrote:
> > There is currently no way to verify the resume image when returning
> > from hibernate. This might compromise the signed modules trust model,
> > so until we can work with signed hibernate images we disable it when the
> > kernel is locked down.
>
> I keep getting these...
>
> IIRC suse has patches to verify the images.
Yeah, Joey Lee is taking care of those. CCing.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Daniel Borkmann @ 2019-06-24 15:15 UTC (permalink / raw)
To: Matthew Garrett, jmorris
Cc: linux-security-module, linux-kernel, linux-api, David Howells,
Alexei Starovoitov, Matthew Garrett, netdev, Chun-Yi Lee, jannh,
bpf
In-Reply-To: <20190622000358.19895-24-matthewgarrett@google.com>
On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> There are some bpf functions can be used to read kernel memory:
Nit: that
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
Please explain how bpf_probe_write_user reads kernel memory ... ?!
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program and kernel memory to be altered without
... and while we're at it, also how they allow "kernel memory to be
altered without restriction". I've been pointing this false statement
out long ago.
> restriction. Disable them if the kernel has been locked down in
> confidentiality mode.
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: netdev@vger.kernel.org
> cc: Chun-Yi Lee <jlee@suse.com>
> cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
Nacked-by: Daniel Borkmann <daniel@iogearbox.net>
[...]
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d64c00afceb5..638f9b00a8df 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -137,6 +137,10 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
This whole thing is still buggy as has been pointed out before by
Jann. For helpers like above and few others below, error conditions
must clear the buffer ...
> ret = probe_kernel_read(dst, unsafe_ptr, size);
> if (unlikely(ret < 0))
> memset(dst, 0, size);
> @@ -156,6 +160,12 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
> BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
> u32, size)
> {
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * Ensure we're in user context which is safe for the helper to
> * run. This helper has no business in a kthread.
> @@ -205,7 +215,11 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
> int fmt_cnt = 0;
> u64 unsafe_addr;
> char buf[64];
> - int i;
> + int i, ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
>
> /*
> * bpf_check()->check_func_arg()->check_stack_boundary()
> @@ -534,6 +548,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * The strncpy_from_unsafe() call will likely not fill the entire
> * buffer, but that's okay in this circumstance as we're probing
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 5a08c17f224d..2eea2cc13117 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> + [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
>
^ permalink raw reply
* Re: [PATCH v3 10/24] Use lsmblob in security_ipc_getsecid
From: Casey Schaufler @ 2019-06-24 16:39 UTC (permalink / raw)
To: Kees Cook
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, sds
In-Reply-To: <201906221545.43D54F0F@keescook>
On 6/22/2019 3:48 PM, Kees Cook wrote:
> On Fri, Jun 21, 2019 at 11:52:19AM -0700, Casey Schaufler wrote:
>> There may be more than one LSM that provides IPC data
>> for auditing. Change security_ipc_getsecid() to fill in
>> a lsmblob structure instead of the u32 secid. The
>> audit data structure containing the secid will be updated
>> later, so there is a bit of scaffolding here.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> One thought below...
>
>> ---
>> include/linux/security.h | 7 ++++---
>> kernel/auditsc.c | 5 ++++-
>> security/security.c | 9 ++++++---
>> 3 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index c6cddeff8a17..0d5e172341fc 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -413,7 +413,7 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
>> unsigned long arg4, unsigned long arg5);
>> void security_task_to_inode(struct task_struct *p, struct inode *inode);
>> int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);
>> -void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid);
>> +void security_ipc_getsecid(struct kern_ipc_perm *ipcp, struct lsmblob *blob);
>> int security_msg_msg_alloc(struct msg_msg *msg);
>> void security_msg_msg_free(struct msg_msg *msg);
>> int security_msg_queue_alloc(struct kern_ipc_perm *msq);
>> @@ -1098,9 +1098,10 @@ static inline int security_ipc_permission(struct kern_ipc_perm *ipcp,
>> return 0;
>> }
>>
>> -static inline void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
>> +static inline void security_ipc_getsecid(struct kern_ipc_perm *ipcp,
>> + struct lsmblob *blob)
>> {
>> - *secid = 0;
>> + lsmblob_init(blob, 0);
>> }
>>
>> static inline int security_msg_msg_alloc(struct msg_msg *msg)
>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> index d31914088a82..148733ec3c72 100644
>> --- a/kernel/auditsc.c
>> +++ b/kernel/auditsc.c
>> @@ -2268,11 +2268,14 @@ void __audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat)
>> void __audit_ipc_obj(struct kern_ipc_perm *ipcp)
>> {
>> struct audit_context *context = audit_context();
>> + struct lsmblob blob;
>> context->ipc.uid = ipcp->uid;
>> context->ipc.gid = ipcp->gid;
>> context->ipc.mode = ipcp->mode;
>> context->ipc.has_perm = 0;
>> - security_ipc_getsecid(ipcp, &context->ipc.osid);
>> + security_ipc_getsecid(ipcp, &blob);
>> + /* scaffolding on the [0] - change "osid" to a lsmblob */
>> + context->ipc.osid = blob.secid[0];
>> context->type = AUDIT_IPC;
>> }
>>
>> diff --git a/security/security.c b/security/security.c
>> index 5ab07631df75..d55f01041f05 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -1812,10 +1812,13 @@ int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
>> return call_int_hook(ipc_permission, 0, ipcp, flag);
>> }
>>
>> -void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
>> +void security_ipc_getsecid(struct kern_ipc_perm *ipcp, struct lsmblob *blob)
>> {
>> - *secid = 0;
>> - call_void_hook(ipc_getsecid, ipcp, secid);
>> + struct security_hook_list *hp;
>> +
>> + lsmblob_init(blob, 0);
>> + hlist_for_each_entry(hp, &security_hook_heads.ipc_getsecid, list)
>> + hp->hook.ipc_getsecid(ipcp, &blob->secid[hp->slot]);
> Just for sanity when using hp->slot, it might be good to do something
> like this in the places it gets used. Like for here:
>
> if (!WARN_ON(hp->slot < 0 || hp->slot >= LSMBLOB_COUNT))
> hp->hook.ipc_getsecid(ipcp, &blob->secid[hp->slot]);
>
> This _should_ be overkill, but since lists of hooks that trigger slot
> assignment is hardcoded, it seems nice to cover any future problems or
> mismatches.
How about a CONFIG_LSM_SLOT_CHECK around a function lsm_slot_check()?
If configured, it does the WARN_ON, and if not it's a static inline
true return. As you say, it's probably overkill, but it would be available
for the paranoid/debug/bringup situation.
^ permalink raw reply
* Re: [PATCH v3 10/24] Use lsmblob in security_ipc_getsecid
From: Casey Schaufler @ 2019-06-24 17:20 UTC (permalink / raw)
To: Kees Cook
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, sds, casey
In-Reply-To: <a70ad13e-bc69-ef03-1f9a-3378c38cae23@schaufler-ca.com>
On 6/24/2019 9:39 AM, Casey Schaufler wrote:
> On 6/22/2019 3:48 PM, Kees Cook wrote:
>> On Fri, Jun 21, 2019 at 11:52:19AM -0700, Casey Schaufler wrote:
>>> There may be more than one LSM that provides IPC data
>>> for auditing. Change security_ipc_getsecid() to fill in
>>> a lsmblob structure instead of the u32 secid. The
>>> audit data structure containing the secid will be updated
>>> later, so there is a bit of scaffolding here.
>>>
>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>
>> One thought below...
>>
>>> ---
>>> include/linux/security.h | 7 ++++---
>>> kernel/auditsc.c | 5 ++++-
>>> security/security.c | 9 ++++++---
>>> 3 files changed, 14 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>> index c6cddeff8a17..0d5e172341fc 100644
>>> --- a/include/linux/security.h
>>> +++ b/include/linux/security.h
>>> @@ -413,7 +413,7 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
>>> unsigned long arg4, unsigned long arg5);
>>> void security_task_to_inode(struct task_struct *p, struct inode *inode);
>>> int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);
>>> -void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid);
>>> +void security_ipc_getsecid(struct kern_ipc_perm *ipcp, struct lsmblob *blob);
>>> int security_msg_msg_alloc(struct msg_msg *msg);
>>> void security_msg_msg_free(struct msg_msg *msg);
>>> int security_msg_queue_alloc(struct kern_ipc_perm *msq);
>>> @@ -1098,9 +1098,10 @@ static inline int security_ipc_permission(struct kern_ipc_perm *ipcp,
>>> return 0;
>>> }
>>>
>>> -static inline void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
>>> +static inline void security_ipc_getsecid(struct kern_ipc_perm *ipcp,
>>> + struct lsmblob *blob)
>>> {
>>> - *secid = 0;
>>> + lsmblob_init(blob, 0);
>>> }
>>>
>>> static inline int security_msg_msg_alloc(struct msg_msg *msg)
>>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>>> index d31914088a82..148733ec3c72 100644
>>> --- a/kernel/auditsc.c
>>> +++ b/kernel/auditsc.c
>>> @@ -2268,11 +2268,14 @@ void __audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat)
>>> void __audit_ipc_obj(struct kern_ipc_perm *ipcp)
>>> {
>>> struct audit_context *context = audit_context();
>>> + struct lsmblob blob;
>>> context->ipc.uid = ipcp->uid;
>>> context->ipc.gid = ipcp->gid;
>>> context->ipc.mode = ipcp->mode;
>>> context->ipc.has_perm = 0;
>>> - security_ipc_getsecid(ipcp, &context->ipc.osid);
>>> + security_ipc_getsecid(ipcp, &blob);
>>> + /* scaffolding on the [0] - change "osid" to a lsmblob */
>>> + context->ipc.osid = blob.secid[0];
>>> context->type = AUDIT_IPC;
>>> }
>>>
>>> diff --git a/security/security.c b/security/security.c
>>> index 5ab07631df75..d55f01041f05 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -1812,10 +1812,13 @@ int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
>>> return call_int_hook(ipc_permission, 0, ipcp, flag);
>>> }
>>>
>>> -void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
>>> +void security_ipc_getsecid(struct kern_ipc_perm *ipcp, struct lsmblob *blob)
>>> {
>>> - *secid = 0;
>>> - call_void_hook(ipc_getsecid, ipcp, secid);
>>> + struct security_hook_list *hp;
>>> +
>>> + lsmblob_init(blob, 0);
>>> + hlist_for_each_entry(hp, &security_hook_heads.ipc_getsecid, list)
>>> + hp->hook.ipc_getsecid(ipcp, &blob->secid[hp->slot]);
>> Just for sanity when using hp->slot, it might be good to do something
>> like this in the places it gets used. Like for here:
>>
>> if (!WARN_ON(hp->slot < 0 || hp->slot >= LSMBLOB_COUNT))
>> hp->hook.ipc_getsecid(ipcp, &blob->secid[hp->slot]);
>>
>> This _should_ be overkill, but since lists of hooks that trigger slot
>> assignment is hardcoded, it seems nice to cover any future problems or
>> mismatches.
> How about a CONFIG_LSM_SLOT_CHECK around a function lsm_slot_check()?
> If configured, it does the WARN_ON, and if not it's a static inline
> true return. As you say, it's probably overkill, but it would be available
> for the paranoid/debug/bringup situation.
... or maybe it would be better if security_add_hooks() had an
explicit parameter whereby an LSM can tell the infrastructure that
it wants a slot. The only future problems would come if the LSM
writer got the parameter wrong or if secid dependence got added
to an existing interface and a LSM that didn't used to need a
slot suddenly did. I don't see either of those as compelling.
^ permalink raw reply
* [PATCH v3 01/24] LSM: Infrastructure management of the superblock
From: John Johansen @ 2019-06-24 18:31 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-2-casey@schaufler-ca.com>
On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Move management of the superblock->sb_security blob out
> of the individual security modules and into the security
> infrastructure. Instead of allocating the blobs from within
> the modules the modules tell the infrastructure how much
> space is required, and the space is allocated there.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
with small fix below
Reviewed-by:John Johansen <john.johansen@canonical.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/security.c | 46 ++++++++++++++++++++----
> security/selinux/hooks.c | 58 ++++++++++++-------------------
> security/selinux/include/objsec.h | 6 ++++
> security/selinux/ss/services.c | 3 +-
> security/smack/smack.h | 6 ++++
> security/smack/smack_lsm.c | 35 +++++--------------
> 7 files changed, 85 insertions(+), 70 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index a240a3fc5fc4..f9222a04968d 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2047,6 +2047,7 @@ struct lsm_blob_sizes {
> int lbs_cred;
> int lbs_file;
> int lbs_inode;
> + int lbs_superblock;
> int lbs_ipc;
> int lbs_msg_msg;
> int lbs_task;
> diff --git a/security/security.c b/security/security.c
> index 23cbb1a295a3..550988a0f024 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -172,6 +172,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
> lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
> lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
> lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> + lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
> lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
> }
>
> @@ -300,12 +301,13 @@ static void __init ordered_lsm_init(void)
> for (lsm = ordered_lsms; *lsm; lsm++)
> prepare_lsm(*lsm);
>
> - init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
> - init_debug("file blob size = %d\n", blob_sizes.lbs_file);
> - init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
> - init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
> - init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
> - init_debug("task blob size = %d\n", blob_sizes.lbs_task);
> + init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
> + init_debug("file blob size = %d\n", blob_sizes.lbs_file);
> + init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
> + init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
> + init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
> + init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> + init_debug("task blob size = %d\n", blob_sizes.lbs_task);
>
> /*
> * Create any kmem_caches needed for blobs
> @@ -603,6 +605,27 @@ static void __init lsm_early_task(struct task_struct *task)
> panic("%s: Early task alloc failed.\n", __func__);
> }
>
> +/**
> + * lsm_superblock_alloc - allocate a composite superblock blob
> + * @sb: the superblock that needs a blob
> + *
> + * Allocate the superblock blob for all the modules
> + *
> + * Returns 0, or -ENOMEM if memory can't be allocated.
> + */
> +int lsm_superblock_alloc(struct super_block *sb)
should be static
> +{
> + if (blob_sizes.lbs_superblock == 0) {
> + sb->s_security = NULL;
> + return 0;
> + }
> +
> + sb->s_security = kzalloc(blob_sizes.lbs_superblock, GFP_KERNEL);
> + if (sb->s_security == NULL)
> + return -ENOMEM;
> + return 0;
> +}
> +
> /*
> * Hook list operation macros.
> *
> @@ -776,12 +799,21 @@ int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *
>
> int security_sb_alloc(struct super_block *sb)
> {
> - return call_int_hook(sb_alloc_security, 0, sb);
> + int rc = lsm_superblock_alloc(sb);
> +
> + if (unlikely(rc))
> + return rc;
> + rc = call_int_hook(sb_alloc_security, 0, sb);
> + if (unlikely(rc))
> + security_sb_free(sb);
> + return rc;
> }
>
> void security_sb_free(struct super_block *sb)
> {
> call_void_hook(sb_free_security, sb);
> + kfree(sb->s_security);
> + sb->s_security = NULL;
> }
>
> void security_free_mnt_opts(void **mnt_opts)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 1d0b37af2444..7478d8eda00a 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -335,7 +335,7 @@ static void inode_free_security(struct inode *inode)
>
> if (!isec)
> return;
> - sbsec = inode->i_sb->s_security;
> + sbsec = selinux_superblock(inode->i_sb);
> /*
> * As not all inode security structures are in a list, we check for
> * empty list outside of the lock to make sure that we won't waste
> @@ -366,11 +366,7 @@ static int file_alloc_security(struct file *file)
>
> static int superblock_alloc_security(struct super_block *sb)
> {
> - struct superblock_security_struct *sbsec;
> -
> - sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL);
> - if (!sbsec)
> - return -ENOMEM;
> + struct superblock_security_struct *sbsec = selinux_superblock(sb);
>
> mutex_init(&sbsec->lock);
> INIT_LIST_HEAD(&sbsec->isec_head);
> @@ -379,18 +375,10 @@ static int superblock_alloc_security(struct super_block *sb)
> sbsec->sid = SECINITSID_UNLABELED;
> sbsec->def_sid = SECINITSID_FILE;
> sbsec->mntpoint_sid = SECINITSID_UNLABELED;
> - sb->s_security = sbsec;
>
> return 0;
> }
>
> -static void superblock_free_security(struct super_block *sb)
> -{
> - struct superblock_security_struct *sbsec = sb->s_security;
> - sb->s_security = NULL;
> - kfree(sbsec);
> -}
> -
> struct selinux_mnt_opts {
> const char *fscontext, *context, *rootcontext, *defcontext;
> };
> @@ -507,7 +495,7 @@ static int selinux_is_genfs_special_handling(struct super_block *sb)
>
> static int selinux_is_sblabel_mnt(struct super_block *sb)
> {
> - struct superblock_security_struct *sbsec = sb->s_security;
> + struct superblock_security_struct *sbsec = selinux_superblock(sb);
>
> /*
> * IMPORTANT: Double-check logic in this function when adding a new
> @@ -535,7 +523,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb)
>
> static int sb_finish_set_opts(struct super_block *sb)
> {
> - struct superblock_security_struct *sbsec = sb->s_security;
> + struct superblock_security_struct *sbsec = selinux_superblock(sb);
> struct dentry *root = sb->s_root;
> struct inode *root_inode = d_backing_inode(root);
> int rc = 0;
> @@ -648,7 +636,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> unsigned long *set_kern_flags)
> {
> const struct cred *cred = current_cred();
> - struct superblock_security_struct *sbsec = sb->s_security;
> + struct superblock_security_struct *sbsec = selinux_superblock(sb);
> struct dentry *root = sbsec->sb->s_root;
> struct selinux_mnt_opts *opts = mnt_opts;
> struct inode_security_struct *root_isec;
> @@ -881,8 +869,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> static int selinux_cmp_sb_context(const struct super_block *oldsb,
> const struct super_block *newsb)
> {
> - struct superblock_security_struct *old = oldsb->s_security;
> - struct superblock_security_struct *new = newsb->s_security;
> + struct superblock_security_struct *old = selinux_superblock(oldsb);
> + struct superblock_security_struct *new = selinux_superblock(newsb);
> char oldflags = old->flags & SE_MNTMASK;
> char newflags = new->flags & SE_MNTMASK;
>
> @@ -914,8 +902,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> unsigned long *set_kern_flags)
> {
> int rc = 0;
> - const struct superblock_security_struct *oldsbsec = oldsb->s_security;
> - struct superblock_security_struct *newsbsec = newsb->s_security;
> + const struct superblock_security_struct *oldsbsec =
> + selinux_superblock(oldsb);
> + struct superblock_security_struct *newsbsec = selinux_superblock(newsb);
>
> int set_fscontext = (oldsbsec->flags & FSCONTEXT_MNT);
> int set_context = (oldsbsec->flags & CONTEXT_MNT);
> @@ -1085,7 +1074,7 @@ static int show_sid(struct seq_file *m, u32 sid)
>
> static int selinux_sb_show_options(struct seq_file *m, struct super_block *sb)
> {
> - struct superblock_security_struct *sbsec = sb->s_security;
> + struct superblock_security_struct *sbsec = selinux_superblock(sb);
> int rc;
>
> if (!(sbsec->flags & SE_SBINITIALIZED))
> @@ -1377,7 +1366,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
> if (isec->sclass == SECCLASS_FILE)
> isec->sclass = inode_mode_to_security_class(inode->i_mode);
>
> - sbsec = inode->i_sb->s_security;
> + sbsec = selinux_superblock(inode->i_sb);
> if (!(sbsec->flags & SE_SBINITIALIZED)) {
> /* Defer initialization until selinux_complete_init,
> after the initial policy is loaded and the security
> @@ -1767,7 +1756,8 @@ selinux_determine_inode_label(const struct task_security_struct *tsec,
> const struct qstr *name, u16 tclass,
> u32 *_new_isid)
> {
> - const struct superblock_security_struct *sbsec = dir->i_sb->s_security;
> + const struct superblock_security_struct *sbsec =
> + selinux_superblock(dir->i_sb);
>
> if ((sbsec->flags & SE_SBINITIALIZED) &&
> (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) {
> @@ -1798,7 +1788,7 @@ static int may_create(struct inode *dir,
> int rc;
>
> dsec = inode_security(dir);
> - sbsec = dir->i_sb->s_security;
> + sbsec = selinux_superblock(dir->i_sb);
>
> sid = tsec->sid;
>
> @@ -1947,7 +1937,7 @@ static int superblock_has_perm(const struct cred *cred,
> struct superblock_security_struct *sbsec;
> u32 sid = cred_sid(cred);
>
> - sbsec = sb->s_security;
> + sbsec = selinux_superblock(sb);
> return avc_has_perm(&selinux_state,
> sid, sbsec->sid, SECCLASS_FILESYSTEM, perms, ad);
> }
> @@ -2578,11 +2568,6 @@ static int selinux_sb_alloc_security(struct super_block *sb)
> return superblock_alloc_security(sb);
> }
>
> -static void selinux_sb_free_security(struct super_block *sb)
> -{
> - superblock_free_security(sb);
> -}
> -
> static inline int opt_len(const char *s)
> {
> bool open_quote = false;
> @@ -2653,7 +2638,7 @@ static int selinux_sb_eat_lsm_opts(char *options, void **mnt_opts)
> static int selinux_sb_remount(struct super_block *sb, void *mnt_opts)
> {
> struct selinux_mnt_opts *opts = mnt_opts;
> - struct superblock_security_struct *sbsec = sb->s_security;
> + struct superblock_security_struct *sbsec = selinux_superblock(sb);
> u32 sid;
> int rc;
>
> @@ -2877,7 +2862,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> int rc;
> char *context;
>
> - sbsec = dir->i_sb->s_security;
> + sbsec = selinux_superblock(dir->i_sb);
>
> newsid = tsec->create_sid;
>
> @@ -3115,7 +3100,7 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name,
> return dentry_has_perm(current_cred(), dentry, FILE__SETATTR);
> }
>
> - sbsec = inode->i_sb->s_security;
> + sbsec = selinux_superblock(inode->i_sb);
> if (!(sbsec->flags & SBLABEL_MNT))
> return -EOPNOTSUPP;
>
> @@ -3296,13 +3281,14 @@ static int selinux_inode_setsecurity(struct inode *inode, const char *name,
> const void *value, size_t size, int flags)
> {
> struct inode_security_struct *isec = inode_security_novalidate(inode);
> - struct superblock_security_struct *sbsec = inode->i_sb->s_security;
> + struct superblock_security_struct *sbsec;
> u32 newsid;
> int rc;
>
> if (strcmp(name, XATTR_SELINUX_SUFFIX))
> return -EOPNOTSUPP;
>
> + sbsec = selinux_superblock(inode->i_sb);
> if (!(sbsec->flags & SBLABEL_MNT))
> return -EOPNOTSUPP;
>
> @@ -6647,6 +6633,7 @@ struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
> .lbs_inode = sizeof(struct inode_security_struct),
> .lbs_ipc = sizeof(struct ipc_security_struct),
> .lbs_msg_msg = sizeof(struct msg_security_struct),
> + .lbs_superblock = sizeof(struct superblock_security_struct),
> };
>
> static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> @@ -6675,7 +6662,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(fs_context_parse_param, selinux_fs_context_parse_param),
>
> LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
> - LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
> LSM_HOOK_INIT(sb_eat_lsm_opts, selinux_sb_eat_lsm_opts),
> LSM_HOOK_INIT(sb_free_mnt_opts, selinux_free_mnt_opts),
> LSM_HOOK_INIT(sb_remount, selinux_sb_remount),
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index 231262d8eac9..d08d7e5d2f93 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -188,4 +188,10 @@ static inline struct ipc_security_struct *selinux_ipc(
> return ipc->security + selinux_blob_sizes.lbs_ipc;
> }
>
> +static inline struct superblock_security_struct *selinux_superblock(
> + const struct super_block *superblock)
> +{
> + return superblock->s_security + selinux_blob_sizes.lbs_superblock;
> +}
> +
> #endif /* _SELINUX_OBJSEC_H_ */
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index ec62918521b1..e3f5d6aece66 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -50,6 +50,7 @@
> #include <linux/audit.h>
> #include <linux/mutex.h>
> #include <linux/vmalloc.h>
> +#include <linux/lsm_hooks.h>
> #include <net/netlabel.h>
>
> #include "flask.h"
> @@ -2751,7 +2752,7 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb)
> struct sidtab *sidtab;
> int rc = 0;
> struct ocontext *c;
> - struct superblock_security_struct *sbsec = sb->s_security;
> + struct superblock_security_struct *sbsec = selinux_superblock(sb);
> const char *fstype = sb->s_type->name;
>
> read_lock(&state->ss->policy_rwlock);
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index cf52af77d15e..caecbcba9942 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -375,6 +375,12 @@ static inline struct smack_known **smack_ipc(const struct kern_ipc_perm *ipc)
> return ipc->security + smack_blob_sizes.lbs_ipc;
> }
>
> +static inline struct superblock_smack *smack_superblock(
> + const struct super_block *superblock)
> +{
> + return superblock->s_security + smack_blob_sizes.lbs_superblock;
> +}
> +
> /*
> * Is the directory transmuting?
> */
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 5c1613519d5a..807eff2ccce9 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -540,12 +540,7 @@ static int smack_syslog(int typefrom_file)
> */
> static int smack_sb_alloc_security(struct super_block *sb)
> {
> - struct superblock_smack *sbsp;
> -
> - sbsp = kzalloc(sizeof(struct superblock_smack), GFP_KERNEL);
> -
> - if (sbsp == NULL)
> - return -ENOMEM;
> + struct superblock_smack *sbsp = smack_superblock(sb);
>
> sbsp->smk_root = &smack_known_floor;
> sbsp->smk_default = &smack_known_floor;
> @@ -554,22 +549,10 @@ static int smack_sb_alloc_security(struct super_block *sb)
> /*
> * SMK_SB_INITIALIZED will be zero from kzalloc.
> */
> - sb->s_security = sbsp;
>
> return 0;
> }
>
> -/**
> - * smack_sb_free_security - free a superblock blob
> - * @sb: the superblock getting the blob
> - *
> - */
> -static void smack_sb_free_security(struct super_block *sb)
> -{
> - kfree(sb->s_security);
> - sb->s_security = NULL;
> -}
> -
> struct smack_mnt_opts {
> const char *fsdefault, *fsfloor, *fshat, *fsroot, *fstransmute;
> };
> @@ -781,7 +764,7 @@ static int smack_set_mnt_opts(struct super_block *sb,
> {
> struct dentry *root = sb->s_root;
> struct inode *inode = d_backing_inode(root);
> - struct superblock_smack *sp = sb->s_security;
> + struct superblock_smack *sp = smack_superblock(sb);
> struct inode_smack *isp;
> struct smack_known *skp;
> struct smack_mnt_opts *opts = mnt_opts;
> @@ -880,7 +863,7 @@ static int smack_set_mnt_opts(struct super_block *sb,
> */
> static int smack_sb_statfs(struct dentry *dentry)
> {
> - struct superblock_smack *sbp = dentry->d_sb->s_security;
> + struct superblock_smack *sbp = smack_superblock(dentry->d_sb);
> int rc;
> struct smk_audit_info ad;
>
> @@ -917,7 +900,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
> if (isp->smk_task == NULL || isp->smk_task == bsp->smk_task)
> return 0;
>
> - sbsp = inode->i_sb->s_security;
> + sbsp = smack_superblock(inode->i_sb);
> if ((sbsp->smk_flags & SMK_SB_UNTRUSTED) &&
> isp->smk_task != sbsp->smk_root)
> return 0;
> @@ -1168,7 +1151,7 @@ static int smack_inode_rename(struct inode *old_inode,
> */
> static int smack_inode_permission(struct inode *inode, int mask)
> {
> - struct superblock_smack *sbsp = inode->i_sb->s_security;
> + struct superblock_smack *sbsp = smack_superblock(inode->i_sb);
> struct smk_audit_info ad;
> int no_block = mask & MAY_NOT_BLOCK;
> int rc;
> @@ -1410,7 +1393,7 @@ static int smack_inode_removexattr(struct dentry *dentry, const char *name)
> */
> if (strcmp(name, XATTR_NAME_SMACK) == 0) {
> struct super_block *sbp = dentry->d_sb;
> - struct superblock_smack *sbsp = sbp->s_security;
> + struct superblock_smack *sbsp = smack_superblock(sbp);
>
> isp->smk_inode = sbsp->smk_default;
> } else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0)
> @@ -1680,7 +1663,7 @@ static int smack_mmap_file(struct file *file,
> isp = smack_inode(file_inode(file));
> if (isp->smk_mmap == NULL)
> return 0;
> - sbsp = file_inode(file)->i_sb->s_security;
> + sbsp = smack_superblock(file_inode(file)->i_sb);
> if (sbsp->smk_flags & SMK_SB_UNTRUSTED &&
> isp->smk_mmap != sbsp->smk_root)
> return -EACCES;
> @@ -3288,7 +3271,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
> goto unlockandout;
>
> sbp = inode->i_sb;
> - sbsp = sbp->s_security;
> + sbsp = smack_superblock(sbp);
> /*
> * We're going to use the superblock default label
> * if there's no label on the file.
> @@ -4575,6 +4558,7 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
> .lbs_inode = sizeof(struct inode_smack),
> .lbs_ipc = sizeof(struct smack_known *),
> .lbs_msg_msg = sizeof(struct smack_known *),
> + .lbs_superblock = sizeof(struct superblock_smack),
> };
>
> static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
> @@ -4586,7 +4570,6 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(fs_context_parse_param, smack_fs_context_parse_param),
>
> LSM_HOOK_INIT(sb_alloc_security, smack_sb_alloc_security),
> - LSM_HOOK_INIT(sb_free_security, smack_sb_free_security),
> LSM_HOOK_INIT(sb_free_mnt_opts, smack_free_mnt_opts),
> LSM_HOOK_INIT(sb_eat_lsm_opts, smack_sb_eat_lsm_opts),
> LSM_HOOK_INIT(sb_statfs, smack_sb_statfs),
>
^ permalink raw reply
* [PATCH v3 02/24] LSM: Infrastructure management of the sock security
From: John Johansen @ 2019-06-24 18:33 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-3-casey@schaufler-ca.com>
On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Move management of the sock->sk_security blob out
> of the individual security modules and into the security
> infrastructure. Instead of allocating the blobs from within
> the modules the modules tell the infrastructure how much
> space is required, and the space is allocated there.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
a couple small things below
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/apparmor/include/net.h | 6 ++-
> security/apparmor/lsm.c | 38 ++++-----------
> security/security.c | 36 +++++++++++++-
> security/selinux/hooks.c | 78 +++++++++++++++----------------
> security/selinux/include/objsec.h | 5 ++
> security/selinux/netlabel.c | 23 ++++-----
> security/smack/smack.h | 5 ++
> security/smack/smack_lsm.c | 64 ++++++++++++-------------
> security/smack/smack_netfilter.c | 8 ++--
> 10 files changed, 144 insertions(+), 120 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index f9222a04968d..b353482ea348 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2047,6 +2047,7 @@ struct lsm_blob_sizes {
> int lbs_cred;
> int lbs_file;
> int lbs_inode;
> + int lbs_sock;
> int lbs_superblock;
> int lbs_ipc;
> int lbs_msg_msg;
> diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h
> index 7334ac966d01..adac04e3b3cc 100644
> --- a/security/apparmor/include/net.h
> +++ b/security/apparmor/include/net.h
> @@ -55,7 +55,11 @@ struct aa_sk_ctx {
> struct aa_label *peer;
> };
>
> -#define SK_CTX(X) ((X)->sk_security)
> +static inline struct aa_sk_ctx *aa_sock(const struct sock *sk)
> +{
> + return sk->sk_security + apparmor_blob_sizes.lbs_sock;
> +}
> +
> #define SOCK_ctx(X) SOCK_INODE(X)->i_security
> #define DEFINE_AUDIT_NET(NAME, OP, SK, F, T, P) \
> struct lsm_network_audit NAME ## _net = { .sk = (SK), \
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 49d664ddff44..2716e7731279 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -757,33 +757,15 @@ static int apparmor_task_kill(struct task_struct *target, struct kernel_siginfo
> return error;
> }
>
> -/**
> - * apparmor_sk_alloc_security - allocate and attach the sk_security field
> - */
> -static int apparmor_sk_alloc_security(struct sock *sk, int family, gfp_t flags)
> -{
> - struct aa_sk_ctx *ctx;
> -
> - ctx = kzalloc(sizeof(*ctx), flags);
> - if (!ctx)
> - return -ENOMEM;
> -
> - SK_CTX(sk) = ctx;
> -
> - return 0;
> -}
> -
> /**
> * apparmor_sk_free_security - free the sk_security field
> */
> static void apparmor_sk_free_security(struct sock *sk)
> {
> - struct aa_sk_ctx *ctx = SK_CTX(sk);
> + struct aa_sk_ctx *ctx = aa_sock(sk);
>
> - SK_CTX(sk) = NULL;
> aa_put_label(ctx->label);
> aa_put_label(ctx->peer);
> - kfree(ctx);
> }
>
> /**
> @@ -792,8 +774,8 @@ static void apparmor_sk_free_security(struct sock *sk)
> static void apparmor_sk_clone_security(const struct sock *sk,
> struct sock *newsk)
> {
> - struct aa_sk_ctx *ctx = SK_CTX(sk);
> - struct aa_sk_ctx *new = SK_CTX(newsk);
> + struct aa_sk_ctx *ctx = aa_sock(sk);
> + struct aa_sk_ctx *new = aa_sock(newsk);
>
> new->label = aa_get_label(ctx->label);
> new->peer = aa_get_label(ctx->peer);
> @@ -844,7 +826,7 @@ static int apparmor_socket_post_create(struct socket *sock, int family,
> label = aa_get_current_label();
>
> if (sock->sk) {
> - struct aa_sk_ctx *ctx = SK_CTX(sock->sk);
> + struct aa_sk_ctx *ctx = aa_sock(sock->sk);
>
> aa_put_label(ctx->label);
> ctx->label = aa_get_label(label);
> @@ -1029,7 +1011,7 @@ static int apparmor_socket_shutdown(struct socket *sock, int how)
> */
> static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
> {
> - struct aa_sk_ctx *ctx = SK_CTX(sk);
> + struct aa_sk_ctx *ctx = aa_sock(sk);
>
> if (!skb->secmark)
> return 0;
> @@ -1042,7 +1024,7 @@ static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
>
> static struct aa_label *sk_peer_label(struct sock *sk)
> {
> - struct aa_sk_ctx *ctx = SK_CTX(sk);
> + struct aa_sk_ctx *ctx = aa_sock(sk);
>
> if (ctx->peer)
> return ctx->peer;
> @@ -1126,7 +1108,7 @@ static int apparmor_socket_getpeersec_dgram(struct socket *sock,
> */
> static void apparmor_sock_graft(struct sock *sk, struct socket *parent)
> {
> - struct aa_sk_ctx *ctx = SK_CTX(sk);
> + struct aa_sk_ctx *ctx = aa_sock(sk);
>
> if (!ctx->label)
> ctx->label = aa_get_current_label();
> @@ -1136,7 +1118,7 @@ static void apparmor_sock_graft(struct sock *sk, struct socket *parent)
> static int apparmor_inet_conn_request(struct sock *sk, struct sk_buff *skb,
> struct request_sock *req)
> {
> - struct aa_sk_ctx *ctx = SK_CTX(sk);
> + struct aa_sk_ctx *ctx = aa_sock(sk);
>
> if (!skb->secmark)
> return 0;
> @@ -1153,6 +1135,7 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
> .lbs_cred = sizeof(struct aa_task_ctx *),
> .lbs_file = sizeof(struct aa_file_ctx),
> .lbs_task = sizeof(struct aa_task_ctx),
> + .lbs_sock = sizeof(struct aa_sk_ctx),
> };
>
> static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
> @@ -1189,7 +1172,6 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
> LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),
>
> - LSM_HOOK_INIT(sk_alloc_security, apparmor_sk_alloc_security),
> LSM_HOOK_INIT(sk_free_security, apparmor_sk_free_security),
> LSM_HOOK_INIT(sk_clone_security, apparmor_sk_clone_security),
>
> @@ -1581,7 +1563,7 @@ static unsigned int apparmor_ip_postroute(void *priv,
> if (sk == NULL)
> return NF_ACCEPT;
>
> - ctx = SK_CTX(sk);
> + ctx = aa_sock(sk);
> if (!apparmor_secmark_check(ctx->label, OP_SENDMSG, AA_MAY_SEND,
> skb->secmark, sk))
> return NF_ACCEPT;
> diff --git a/security/security.c b/security/security.c
> index 550988a0f024..e32b7180282e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -32,6 +32,7 @@
> #include <linux/string.h>
> #include <linux/msg.h>
> #include <net/flow.h>
> +#include <net/sock.h>
>
> #define MAX_LSM_EVM_XATTR 2
>
> @@ -172,6 +173,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
> lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
> lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
> lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> + lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
> lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
> lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
> }
> @@ -306,6 +308,7 @@ static void __init ordered_lsm_init(void)
> init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
> init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
> init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
> + init_debug("sock blob size = %d\n", blob_sizes.lbs_sock);
> init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> init_debug("task blob size = %d\n", blob_sizes.lbs_task);
>
> @@ -605,6 +608,28 @@ static void __init lsm_early_task(struct task_struct *task)
> panic("%s: Early task alloc failed.\n", __func__);
> }
>
> +/**
> + * lsm_sock_alloc - allocate a composite sock blob
> + * @sock: the sock that needs a blob
> + * @priority: allocation mode
> + *
> + * Allocate the sock blob for all the modules
> + *
> + * Returns 0, or -ENOMEM if memory can't be allocated.
> + */
> +int lsm_sock_alloc(struct sock *sock, gfp_t priority)
should be static
> +{
> + if (blob_sizes.lbs_sock == 0) {
> + sock->sk_security = NULL;
> + return 0;
> + }
> +
> + sock->sk_security = kzalloc(blob_sizes.lbs_sock, priority);
> + if (sock->sk_security == NULL)
> + return -ENOMEM;
> + return 0;
> +}
> +
> /**
> * lsm_superblock_alloc - allocate a composite superblock blob
> * @sb: the superblock that needs a blob
> @@ -2048,12 +2073,21 @@ EXPORT_SYMBOL(security_socket_getpeersec_dgram);
>
> int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
> {
> - return call_int_hook(sk_alloc_security, 0, sk, family, priority);
> + int rc = lsm_sock_alloc(sk, priority);
> +
> + if (unlikely(rc))
> + return rc;
> + rc = call_int_hook(sk_alloc_security, 0, sk, family, priority);
> + if (unlikely(rc))
> + security_sk_free(sk);
> + return rc;
> }
>
> void security_sk_free(struct sock *sk)
> {
> call_void_hook(sk_free_security, sk);
> + kfree(sk->sk_security);
> + sk->sk_security = NULL;
> }
>
> void security_sk_clone(const struct sock *sk, struct sock *newsk)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7478d8eda00a..f38a6f484613 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4319,7 +4319,7 @@ static int socket_sockcreate_sid(const struct task_security_struct *tsec,
>
> static int sock_has_perm(struct sock *sk, u32 perms)
> {
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> struct common_audit_data ad;
> struct lsm_network_audit net = {0,};
>
> @@ -4376,7 +4376,7 @@ static int selinux_socket_post_create(struct socket *sock, int family,
> isec->initialized = LABEL_INITIALIZED;
>
> if (sock->sk) {
> - sksec = sock->sk->sk_security;
> + sksec = selinux_sock(sock->sk);
> sksec->sclass = sclass;
> sksec->sid = sid;
> /* Allows detection of the first association on this socket */
> @@ -4392,8 +4392,8 @@ static int selinux_socket_post_create(struct socket *sock, int family,
> static int selinux_socket_socketpair(struct socket *socka,
> struct socket *sockb)
> {
> - struct sk_security_struct *sksec_a = socka->sk->sk_security;
> - struct sk_security_struct *sksec_b = sockb->sk->sk_security;
> + struct sk_security_struct *sksec_a = selinux_sock(socka->sk);
> + struct sk_security_struct *sksec_b = selinux_sock(sockb->sk);
>
> sksec_a->peer_sid = sksec_b->sid;
> sksec_b->peer_sid = sksec_a->sid;
> @@ -4408,7 +4408,7 @@ static int selinux_socket_socketpair(struct socket *socka,
> static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen)
> {
> struct sock *sk = sock->sk;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> u16 family;
> int err;
>
> @@ -4540,7 +4540,7 @@ static int selinux_socket_connect_helper(struct socket *sock,
> struct sockaddr *address, int addrlen)
> {
> struct sock *sk = sock->sk;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> int err;
>
> err = sock_has_perm(sk, SOCKET__CONNECT);
> @@ -4711,9 +4711,9 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
> struct sock *other,
> struct sock *newsk)
> {
> - struct sk_security_struct *sksec_sock = sock->sk_security;
> - struct sk_security_struct *sksec_other = other->sk_security;
> - struct sk_security_struct *sksec_new = newsk->sk_security;
> + struct sk_security_struct *sksec_sock = selinux_sock(sock);
> + struct sk_security_struct *sksec_other = selinux_sock(other);
> + struct sk_security_struct *sksec_new = selinux_sock(newsk);
> struct common_audit_data ad;
> struct lsm_network_audit net = {0,};
> int err;
> @@ -4745,8 +4745,8 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
> static int selinux_socket_unix_may_send(struct socket *sock,
> struct socket *other)
> {
> - struct sk_security_struct *ssec = sock->sk->sk_security;
> - struct sk_security_struct *osec = other->sk->sk_security;
> + struct sk_security_struct *ssec = selinux_sock(sock->sk);
> + struct sk_security_struct *osec = selinux_sock(other->sk);
> struct common_audit_data ad;
> struct lsm_network_audit net = {0,};
>
> @@ -4788,7 +4788,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
> u16 family)
> {
> int err = 0;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> u32 sk_sid = sksec->sid;
> struct common_audit_data ad;
> struct lsm_network_audit net = {0,};
> @@ -4821,7 +4821,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
> static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
> {
> int err;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> u16 family = sk->sk_family;
> u32 sk_sid = sksec->sid;
> struct common_audit_data ad;
> @@ -4889,13 +4889,15 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
> return err;
> }
>
> -static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *optval,
> - int __user *optlen, unsigned len)
> +static int selinux_socket_getpeersec_stream(struct socket *sock,
> + __user char *optval,
> + __user int *optlen,
> + unsigned int len)
any particular reason for changing the order from char __user to __user char
> {
> int err = 0;
> char *scontext;
> u32 scontext_len;
> - struct sk_security_struct *sksec = sock->sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sock->sk);
> u32 peer_sid = SECSID_NULL;
>
> if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
> @@ -4955,34 +4957,27 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *
>
> static int selinux_sk_alloc_security(struct sock *sk, int family, gfp_t priority)
> {
> - struct sk_security_struct *sksec;
> -
> - sksec = kzalloc(sizeof(*sksec), priority);
> - if (!sksec)
> - return -ENOMEM;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> sksec->peer_sid = SECINITSID_UNLABELED;
> sksec->sid = SECINITSID_UNLABELED;
> sksec->sclass = SECCLASS_SOCKET;
> selinux_netlbl_sk_security_reset(sksec);
> - sk->sk_security = sksec;
>
> return 0;
> }
>
> static void selinux_sk_free_security(struct sock *sk)
> {
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> - sk->sk_security = NULL;
> selinux_netlbl_sk_security_free(sksec);
> - kfree(sksec);
> }
>
> static void selinux_sk_clone_security(const struct sock *sk, struct sock *newsk)
> {
> - struct sk_security_struct *sksec = sk->sk_security;
> - struct sk_security_struct *newsksec = newsk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> + struct sk_security_struct *newsksec = selinux_sock(newsk);
>
> newsksec->sid = sksec->sid;
> newsksec->peer_sid = sksec->peer_sid;
> @@ -4996,7 +4991,7 @@ static void selinux_sk_getsecid(struct sock *sk, u32 *secid)
> if (!sk)
> *secid = SECINITSID_ANY_SOCKET;
> else {
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> *secid = sksec->sid;
> }
> @@ -5006,7 +5001,7 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
> {
> struct inode_security_struct *isec =
> inode_security_novalidate(SOCK_INODE(parent));
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6 ||
> sk->sk_family == PF_UNIX)
> @@ -5021,7 +5016,7 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
> static int selinux_sctp_assoc_request(struct sctp_endpoint *ep,
> struct sk_buff *skb)
> {
> - struct sk_security_struct *sksec = ep->base.sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(ep->base.sk);
> struct common_audit_data ad;
> struct lsm_network_audit net = {0,};
> u8 peerlbl_active;
> @@ -5172,8 +5167,8 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
> static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
> struct sock *newsk)
> {
> - struct sk_security_struct *sksec = sk->sk_security;
> - struct sk_security_struct *newsksec = newsk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> + struct sk_security_struct *newsksec = selinux_sock(newsk);
>
> /* If policy does not support SECCLASS_SCTP_SOCKET then call
> * the non-sctp clone version.
> @@ -5190,7 +5185,7 @@ static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
> static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb,
> struct request_sock *req)
> {
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> int err;
> u16 family = req->rsk_ops->family;
> u32 connsid;
> @@ -5211,7 +5206,7 @@ static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb,
> static void selinux_inet_csk_clone(struct sock *newsk,
> const struct request_sock *req)
> {
> - struct sk_security_struct *newsksec = newsk->sk_security;
> + struct sk_security_struct *newsksec = selinux_sock(newsk);
>
> newsksec->sid = req->secid;
> newsksec->peer_sid = req->peer_secid;
> @@ -5228,7 +5223,7 @@ static void selinux_inet_csk_clone(struct sock *newsk,
> static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
> {
> u16 family = sk->sk_family;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> /* handle mapped IPv4 packets arriving via IPv6 sockets */
> if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
> @@ -5312,7 +5307,7 @@ static int selinux_tun_dev_attach_queue(void *security)
> static int selinux_tun_dev_attach(struct sock *sk, void *security)
> {
> struct tun_security_struct *tunsec = security;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> /* we don't currently perform any NetLabel based labeling here and it
> * isn't clear that we would want to do so anyway; while we could apply
> @@ -5353,7 +5348,7 @@ static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
> int err = 0;
> u32 perm;
> struct nlmsghdr *nlh;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> if (skb->len < NLMSG_HDRLEN) {
> err = -EINVAL;
> @@ -5494,7 +5489,7 @@ static unsigned int selinux_ip_output(struct sk_buff *skb,
> return NF_ACCEPT;
>
> /* standard practice, label using the parent socket */
> - sksec = sk->sk_security;
> + sksec = selinux_sock(sk);
> sid = sksec->sid;
> } else
> sid = SECINITSID_KERNEL;
> @@ -5533,7 +5528,7 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
>
> if (sk == NULL)
> return NF_ACCEPT;
> - sksec = sk->sk_security;
> + sksec = selinux_sock(sk);
>
> ad.type = LSM_AUDIT_DATA_NET;
> ad.u.net = &net;
> @@ -5625,7 +5620,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb,
> u32 skb_sid;
> struct sk_security_struct *sksec;
>
> - sksec = sk->sk_security;
> + sksec = selinux_sock(sk);
> if (selinux_skb_peerlbl_sid(skb, family, &skb_sid))
> return NF_DROP;
> /* At this point, if the returned skb peerlbl is SECSID_NULL
> @@ -5654,7 +5649,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb,
> } else {
> /* Locally generated packet, fetch the security label from the
> * associated socket. */
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> peer_sid = sksec->sid;
> secmark_perm = PACKET__SEND;
> }
> @@ -6633,6 +6628,7 @@ struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
> .lbs_inode = sizeof(struct inode_security_struct),
> .lbs_ipc = sizeof(struct ipc_security_struct),
> .lbs_msg_msg = sizeof(struct msg_security_struct),
> + .lbs_sock = sizeof(struct sk_security_struct),
> .lbs_superblock = sizeof(struct superblock_security_struct),
> };
>
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index d08d7e5d2f93..29f02b8f8f31 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -194,4 +194,9 @@ static inline struct superblock_security_struct *selinux_superblock(
> return superblock->s_security + selinux_blob_sizes.lbs_superblock;
> }
>
> +static inline struct sk_security_struct *selinux_sock(const struct sock *sock)
> +{
> + return sock->sk_security + selinux_blob_sizes.lbs_sock;
> +}
> +
> #endif /* _SELINUX_OBJSEC_H_ */
> diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
> index 186e727b737b..c40914a157b7 100644
> --- a/security/selinux/netlabel.c
> +++ b/security/selinux/netlabel.c
> @@ -31,6 +31,7 @@
> #include <linux/gfp.h>
> #include <linux/ip.h>
> #include <linux/ipv6.h>
> +#include <linux/lsm_hooks.h>
> #include <net/sock.h>
> #include <net/netlabel.h>
> #include <net/ip.h>
> @@ -81,7 +82,7 @@ static int selinux_netlbl_sidlookup_cached(struct sk_buff *skb,
> static struct netlbl_lsm_secattr *selinux_netlbl_sock_genattr(struct sock *sk)
> {
> int rc;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> struct netlbl_lsm_secattr *secattr;
>
> if (sksec->nlbl_secattr != NULL)
> @@ -114,7 +115,7 @@ static struct netlbl_lsm_secattr *selinux_netlbl_sock_getattr(
> const struct sock *sk,
> u32 sid)
> {
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> struct netlbl_lsm_secattr *secattr = sksec->nlbl_secattr;
>
> if (secattr == NULL)
> @@ -249,7 +250,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
> * being labeled by it's parent socket, if it is just exit */
> sk = skb_to_full_sk(skb);
> if (sk != NULL) {
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> if (sksec->nlbl_state != NLBL_REQSKB)
> return 0;
> @@ -287,7 +288,7 @@ int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
> {
> int rc;
> struct netlbl_lsm_secattr secattr;
> - struct sk_security_struct *sksec = ep->base.sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(ep->base.sk);
> struct sockaddr *addr;
> struct sockaddr_in addr4;
> #if IS_ENABLED(CONFIG_IPV6)
> @@ -370,7 +371,7 @@ int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family)
> */
> void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family)
> {
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> if (family == PF_INET)
> sksec->nlbl_state = NLBL_LABELED;
> @@ -388,8 +389,8 @@ void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family)
> */
> void selinux_netlbl_sctp_sk_clone(struct sock *sk, struct sock *newsk)
> {
> - struct sk_security_struct *sksec = sk->sk_security;
> - struct sk_security_struct *newsksec = newsk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> + struct sk_security_struct *newsksec = selinux_sock(newsk);
>
> newsksec->nlbl_state = sksec->nlbl_state;
> }
> @@ -407,7 +408,7 @@ void selinux_netlbl_sctp_sk_clone(struct sock *sk, struct sock *newsk)
> int selinux_netlbl_socket_post_create(struct sock *sk, u16 family)
> {
> int rc;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> struct netlbl_lsm_secattr *secattr;
>
> if (family != PF_INET && family != PF_INET6)
> @@ -522,7 +523,7 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
> {
> int rc = 0;
> struct sock *sk = sock->sk;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> struct netlbl_lsm_secattr secattr;
>
> if (selinux_netlbl_option(level, optname) &&
> @@ -560,7 +561,7 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
> struct sockaddr *addr)
> {
> int rc;
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
> struct netlbl_lsm_secattr *secattr;
>
> /* connected sockets are allowed to disconnect when the address family
> @@ -599,7 +600,7 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
> int selinux_netlbl_socket_connect_locked(struct sock *sk,
> struct sockaddr *addr)
> {
> - struct sk_security_struct *sksec = sk->sk_security;
> + struct sk_security_struct *sksec = selinux_sock(sk);
>
> if (sksec->nlbl_state != NLBL_REQSKB &&
> sksec->nlbl_state != NLBL_CONNLABELED)
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index caecbcba9942..4ac4bf3310d7 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -375,6 +375,11 @@ static inline struct smack_known **smack_ipc(const struct kern_ipc_perm *ipc)
> return ipc->security + smack_blob_sizes.lbs_ipc;
> }
>
> +static inline struct socket_smack *smack_sock(const struct sock *sock)
> +{
> + return sock->sk_security + smack_blob_sizes.lbs_sock;
> +}
> +
> static inline struct superblock_smack *smack_superblock(
> const struct super_block *superblock)
> {
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 807eff2ccce9..fd69e1bd841b 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -1439,7 +1439,7 @@ static int smack_inode_getsecurity(struct inode *inode,
> if (sock == NULL || sock->sk == NULL)
> return -EOPNOTSUPP;
>
> - ssp = sock->sk->sk_security;
> + ssp = smack_sock(sock->sk);
>
> if (strcmp(name, XATTR_SMACK_IPIN) == 0)
> isp = ssp->smk_in;
> @@ -1821,7 +1821,7 @@ static int smack_file_receive(struct file *file)
>
> if (inode->i_sb->s_magic == SOCKFS_MAGIC) {
> sock = SOCKET_I(inode);
> - ssp = sock->sk->sk_security;
> + ssp = smack_sock(sock->sk);
> tsp = smack_cred(current_cred());
> /*
> * If the receiving process can't write to the
> @@ -2231,11 +2231,7 @@ static void smack_task_to_inode(struct task_struct *p, struct inode *inode)
> static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
> {
> struct smack_known *skp = smk_of_current();
> - struct socket_smack *ssp;
> -
> - ssp = kzalloc(sizeof(struct socket_smack), gfp_flags);
> - if (ssp == NULL)
> - return -ENOMEM;
> + struct socket_smack *ssp = smack_sock(sk);
>
> /*
> * Sockets created by kernel threads receive web label.
> @@ -2249,11 +2245,10 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
> }
> ssp->smk_packet = NULL;
>
> - sk->sk_security = ssp;
> -
> return 0;
> }
>
> +#ifdef SMACK_IPV6_PORT_LABELING
> /**
> * smack_sk_free_security - Free a socket blob
> * @sk: the socket
> @@ -2262,7 +2257,6 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
> */
> static void smack_sk_free_security(struct sock *sk)
> {
> -#ifdef SMACK_IPV6_PORT_LABELING
> struct smk_port_label *spp;
>
> if (sk->sk_family == PF_INET6) {
> @@ -2275,9 +2269,8 @@ static void smack_sk_free_security(struct sock *sk)
> }
> rcu_read_unlock();
> }
> -#endif
> - kfree(sk->sk_security);
> }
> +#endif
>
> /**
> * smack_ipv4host_label - check host based restrictions
> @@ -2395,7 +2388,7 @@ static struct smack_known *smack_ipv6host_label(struct sockaddr_in6 *sip)
> static int smack_netlabel(struct sock *sk, int labeled)
> {
> struct smack_known *skp;
> - struct socket_smack *ssp = sk->sk_security;
> + struct socket_smack *ssp = smack_sock(sk);
> int rc = 0;
>
> /*
> @@ -2440,7 +2433,7 @@ static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap)
> int rc;
> int sk_lbl;
> struct smack_known *hkp;
> - struct socket_smack *ssp = sk->sk_security;
> + struct socket_smack *ssp = smack_sock(sk);
> struct smk_audit_info ad;
>
> rcu_read_lock();
> @@ -2516,7 +2509,7 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
> {
> struct sock *sk = sock->sk;
> struct sockaddr_in6 *addr6;
> - struct socket_smack *ssp = sock->sk->sk_security;
> + struct socket_smack *ssp = smack_sock(sock->sk);
> struct smk_port_label *spp;
> unsigned short port = 0;
>
> @@ -2603,7 +2596,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address,
> int act)
> {
> struct smk_port_label *spp;
> - struct socket_smack *ssp = sk->sk_security;
> + struct socket_smack *ssp = smack_sock(sk);
> struct smack_known *skp = NULL;
> unsigned short port;
> struct smack_known *object;
> @@ -2697,7 +2690,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
> if (sock == NULL || sock->sk == NULL)
> return -EOPNOTSUPP;
>
> - ssp = sock->sk->sk_security;
> + ssp = smack_sock(sock->sk);
>
> if (strcmp(name, XATTR_SMACK_IPIN) == 0)
> ssp->smk_in = skp;
> @@ -2745,7 +2738,7 @@ static int smack_socket_post_create(struct socket *sock, int family,
> * Sockets created by kernel threads receive web label.
> */
> if (unlikely(current->flags & PF_KTHREAD)) {
> - ssp = sock->sk->sk_security;
> + ssp = smack_sock(sock->sk);
> ssp->smk_in = &smack_known_web;
> ssp->smk_out = &smack_known_web;
> }
> @@ -2770,8 +2763,8 @@ static int smack_socket_post_create(struct socket *sock, int family,
> static int smack_socket_socketpair(struct socket *socka,
> struct socket *sockb)
> {
> - struct socket_smack *asp = socka->sk->sk_security;
> - struct socket_smack *bsp = sockb->sk->sk_security;
> + struct socket_smack *asp = smack_sock(socka->sk);
> + struct socket_smack *bsp = smack_sock(sockb->sk);
>
> asp->smk_packet = bsp->smk_out;
> bsp->smk_packet = asp->smk_out;
> @@ -2825,7 +2818,7 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap,
> return 0;
>
> #ifdef SMACK_IPV6_SECMARK_LABELING
> - ssp = sock->sk->sk_security;
> + ssp = smack_sock(sock->sk);
> #endif
>
> switch (sock->sk->sk_family) {
> @@ -3566,9 +3559,9 @@ static int smack_unix_stream_connect(struct sock *sock,
> {
> struct smack_known *skp;
> struct smack_known *okp;
> - struct socket_smack *ssp = sock->sk_security;
> - struct socket_smack *osp = other->sk_security;
> - struct socket_smack *nsp = newsk->sk_security;
> + struct socket_smack *ssp = smack_sock(sock);
> + struct socket_smack *osp = smack_sock(other);
> + struct socket_smack *nsp = smack_sock(newsk);
> struct smk_audit_info ad;
> int rc = 0;
> #ifdef CONFIG_AUDIT
> @@ -3614,8 +3607,8 @@ static int smack_unix_stream_connect(struct sock *sock,
> */
> static int smack_unix_may_send(struct socket *sock, struct socket *other)
> {
> - struct socket_smack *ssp = sock->sk->sk_security;
> - struct socket_smack *osp = other->sk->sk_security;
> + struct socket_smack *ssp = smack_sock(sock->sk);
> + struct socket_smack *osp = smack_sock(other->sk);
> struct smk_audit_info ad;
> int rc;
>
> @@ -3652,7 +3645,7 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg,
> struct sockaddr_in6 *sap = (struct sockaddr_in6 *) msg->msg_name;
> #endif
> #ifdef SMACK_IPV6_SECMARK_LABELING
> - struct socket_smack *ssp = sock->sk->sk_security;
> + struct socket_smack *ssp = smack_sock(sock->sk);
> struct smack_known *rsp;
> #endif
> int rc = 0;
> @@ -3817,7 +3810,7 @@ static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
> static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
> {
> struct netlbl_lsm_secattr secattr;
> - struct socket_smack *ssp = sk->sk_security;
> + struct socket_smack *ssp = smack_sock(sk);
> struct smack_known *skp = NULL;
> int rc = 0;
> struct smk_audit_info ad;
> @@ -3934,7 +3927,7 @@ static int smack_socket_getpeersec_stream(struct socket *sock,
> int slen = 1;
> int rc = 0;
>
> - ssp = sock->sk->sk_security;
> + ssp = smack_sock(sock->sk);
> if (ssp->smk_packet != NULL) {
> rcp = ssp->smk_packet->smk_known;
> slen = strlen(rcp) + 1;
> @@ -3984,7 +3977,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
>
> switch (family) {
> case PF_UNIX:
> - ssp = sock->sk->sk_security;
> + ssp = smack_sock(sock->sk);
> s = ssp->smk_out->smk_secid;
> break;
> case PF_INET:
> @@ -3997,7 +3990,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
> * Translate what netlabel gave us.
> */
> if (sock != NULL && sock->sk != NULL)
> - ssp = sock->sk->sk_security;
> + ssp = smack_sock(sock->sk);
> netlbl_secattr_init(&secattr);
> rc = netlbl_skbuff_getattr(skb, family, &secattr);
> if (rc == 0) {
> @@ -4035,7 +4028,7 @@ static void smack_sock_graft(struct sock *sk, struct socket *parent)
> (sk->sk_family != PF_INET && sk->sk_family != PF_INET6))
> return;
>
> - ssp = sk->sk_security;
> + ssp = smack_sock(sk);
> ssp->smk_in = skp;
> ssp->smk_out = skp;
> /* cssp->smk_packet is already set in smack_inet_csk_clone() */
> @@ -4055,7 +4048,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
> {
> u16 family = sk->sk_family;
> struct smack_known *skp;
> - struct socket_smack *ssp = sk->sk_security;
> + struct socket_smack *ssp = smack_sock(sk);
> struct netlbl_lsm_secattr secattr;
> struct sockaddr_in addr;
> struct iphdr *hdr;
> @@ -4154,7 +4147,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
> static void smack_inet_csk_clone(struct sock *sk,
> const struct request_sock *req)
> {
> - struct socket_smack *ssp = sk->sk_security;
> + struct socket_smack *ssp = smack_sock(sk);
> struct smack_known *skp;
>
> if (req->peer_secid != 0) {
> @@ -4558,6 +4551,7 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
> .lbs_inode = sizeof(struct inode_smack),
> .lbs_ipc = sizeof(struct smack_known *),
> .lbs_msg_msg = sizeof(struct smack_known *),
> + .lbs_sock = sizeof(struct socket_smack),
> .lbs_superblock = sizeof(struct superblock_smack),
> };
>
> @@ -4667,7 +4661,9 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(socket_getpeersec_stream, smack_socket_getpeersec_stream),
> LSM_HOOK_INIT(socket_getpeersec_dgram, smack_socket_getpeersec_dgram),
> LSM_HOOK_INIT(sk_alloc_security, smack_sk_alloc_security),
> +#ifdef SMACK_IPV6_PORT_LABELING
> LSM_HOOK_INIT(sk_free_security, smack_sk_free_security),
> +#endif
> LSM_HOOK_INIT(sock_graft, smack_sock_graft),
> LSM_HOOK_INIT(inet_conn_request, smack_inet_conn_request),
> LSM_HOOK_INIT(inet_csk_clone, smack_inet_csk_clone),
> diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
> index e36d17835d4f..701a1cc1bdcc 100644
> --- a/security/smack/smack_netfilter.c
> +++ b/security/smack/smack_netfilter.c
> @@ -31,8 +31,8 @@ static unsigned int smack_ipv6_output(void *priv,
> struct socket_smack *ssp;
> struct smack_known *skp;
>
> - if (sk && sk->sk_security) {
> - ssp = sk->sk_security;
> + if (sk && smack_sock(sk)) {
> + ssp = smack_sock(sk);
> skp = ssp->smk_out;
> skb->secmark = skp->smk_secid;
> }
> @@ -49,8 +49,8 @@ static unsigned int smack_ipv4_output(void *priv,
> struct socket_smack *ssp;
> struct smack_known *skp;
>
> - if (sk && sk->sk_security) {
> - ssp = sk->sk_security;
> + if (sk && smack_sock(sk)) {
> + ssp = smack_sock(sk);
> skp = ssp->smk_out;
> skb->secmark = skp->smk_secid;
> }
>
^ permalink raw reply
* [PATCH v3 03/24] LSM: Infrastructure management of the key blob
From: John Johansen @ 2019-06-24 18:33 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-4-casey@schaufler-ca.com>
On 6/21/19 11:52 AM, Casey Schaufler wrote:
> From: Casey Schaufler <cschaufler@schaufler-ca.com>
>
> Move management of the key->security blob out of the
> individual security modules and into the security
> infrastructure. Instead of allocating the blobs from within
> the modules the modules tell the infrastructure how much
> space is required, and the space is allocated there.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
One small thing below
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/security.c | 40 ++++++++++++++++++++++++++++++-
> security/selinux/hooks.c | 23 +++++-------------
> security/selinux/include/objsec.h | 7 ++++++
> security/smack/smack.h | 7 ++++++
> security/smack/smack_lsm.c | 33 ++++++++++++-------------
> 6 files changed, 75 insertions(+), 36 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index b353482ea348..3fe39abccc8f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2050,6 +2050,7 @@ struct lsm_blob_sizes {
> int lbs_sock;
> int lbs_superblock;
> int lbs_ipc;
> + int lbs_key;
> int lbs_msg_msg;
> int lbs_task;
> };
> diff --git a/security/security.c b/security/security.c
> index e32b7180282e..d05f00a40e82 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -172,6 +172,9 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
> blob_sizes.lbs_inode = sizeof(struct rcu_head);
> lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
> lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
> +#ifdef CONFIG_KEYS
> + lsm_set_blob_size(&needed->lbs_key, &blob_sizes.lbs_key);
> +#endif
> lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
> lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
> @@ -307,6 +310,9 @@ static void __init ordered_lsm_init(void)
> init_debug("file blob size = %d\n", blob_sizes.lbs_file);
> init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
> init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
> +#ifdef CONFIG_KEYS
> + init_debug("key blob size = %d\n", blob_sizes.lbs_key);
> +#endif /* CONFIG_KEYS */
> init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
> init_debug("sock blob size = %d\n", blob_sizes.lbs_sock);
> init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> @@ -573,6 +579,29 @@ static int lsm_ipc_alloc(struct kern_ipc_perm *kip)
> return 0;
> }
>
> +#ifdef CONFIG_KEYS
> +/**
> + * lsm_key_alloc - allocate a composite key blob
> + * @key: the key that needs a blob
> + *
> + * Allocate the key blob for all the modules
> + *
> + * Returns 0, or -ENOMEM if memory can't be allocated.
> + */
> +int lsm_key_alloc(struct key *key)
should be static
> +{
> + if (blob_sizes.lbs_key == 0) {
> + key->security = NULL;
> + return 0;
> + }
> +
> + key->security = kzalloc(blob_sizes.lbs_key, GFP_KERNEL);
> + if (key->security == NULL)
> + return -ENOMEM;
> + return 0;
> +}
> +#endif /* CONFIG_KEYS */
> +
> /**
> * lsm_msg_msg_alloc - allocate a composite msg_msg blob
> * @mp: the msg_msg that needs a blob
> @@ -2339,12 +2368,21 @@ EXPORT_SYMBOL(security_skb_classify_flow);
> int security_key_alloc(struct key *key, const struct cred *cred,
> unsigned long flags)
> {
> - return call_int_hook(key_alloc, 0, key, cred, flags);
> + int rc = lsm_key_alloc(key);
> +
> + if (unlikely(rc))
> + return rc;
> + rc = call_int_hook(key_alloc, 0, key, cred, flags);
> + if (unlikely(rc))
> + security_key_free(key);
> + return rc;
> }
>
> void security_key_free(struct key *key)
> {
> call_void_hook(key_free, key);
> + kfree(key->security);
> + key->security = NULL;
> }
>
> int security_key_permission(key_ref_t key_ref,
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f38a6f484613..ee840fecfebb 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6353,11 +6353,7 @@ static int selinux_key_alloc(struct key *k, const struct cred *cred,
> unsigned long flags)
> {
> const struct task_security_struct *tsec;
> - struct key_security_struct *ksec;
> -
> - ksec = kzalloc(sizeof(struct key_security_struct), GFP_KERNEL);
> - if (!ksec)
> - return -ENOMEM;
> + struct key_security_struct *ksec = selinux_key(k);
>
> tsec = selinux_cred(cred);
> if (tsec->keycreate_sid)
> @@ -6365,18 +6361,9 @@ static int selinux_key_alloc(struct key *k, const struct cred *cred,
> else
> ksec->sid = tsec->sid;
>
> - k->security = ksec;
> return 0;
> }
>
> -static void selinux_key_free(struct key *k)
> -{
> - struct key_security_struct *ksec = k->security;
> -
> - k->security = NULL;
> - kfree(ksec);
> -}
> -
> static int selinux_key_permission(key_ref_t key_ref,
> const struct cred *cred,
> unsigned perm)
> @@ -6394,7 +6381,7 @@ static int selinux_key_permission(key_ref_t key_ref,
> sid = cred_sid(cred);
>
> key = key_ref_to_ptr(key_ref);
> - ksec = key->security;
> + ksec = selinux_key(key);
>
> return avc_has_perm(&selinux_state,
> sid, ksec->sid, SECCLASS_KEY, perm, NULL);
> @@ -6402,7 +6389,7 @@ static int selinux_key_permission(key_ref_t key_ref,
>
> static int selinux_key_getsecurity(struct key *key, char **_buffer)
> {
> - struct key_security_struct *ksec = key->security;
> + struct key_security_struct *ksec = selinux_key(key);
> char *context = NULL;
> unsigned len;
> int rc;
> @@ -6627,6 +6614,9 @@ struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
> .lbs_file = sizeof(struct file_security_struct),
> .lbs_inode = sizeof(struct inode_security_struct),
> .lbs_ipc = sizeof(struct ipc_security_struct),
> +#ifdef CONFIG_KEYS
> + .lbs_key = sizeof(struct key_security_struct),
> +#endif /* CONFIG_KEYS */
> .lbs_msg_msg = sizeof(struct msg_security_struct),
> .lbs_sock = sizeof(struct sk_security_struct),
> .lbs_superblock = sizeof(struct superblock_security_struct),
> @@ -6842,7 +6832,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>
> #ifdef CONFIG_KEYS
> LSM_HOOK_INIT(key_alloc, selinux_key_alloc),
> - LSM_HOOK_INIT(key_free, selinux_key_free),
> LSM_HOOK_INIT(key_permission, selinux_key_permission),
> LSM_HOOK_INIT(key_getsecurity, selinux_key_getsecurity),
> #endif
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index 29f02b8f8f31..3b78aa4ee98f 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -194,6 +194,13 @@ static inline struct superblock_security_struct *selinux_superblock(
> return superblock->s_security + selinux_blob_sizes.lbs_superblock;
> }
>
> +#ifdef CONFIG_KEYS
> +static inline struct key_security_struct *selinux_key(const struct key *key)
> +{
> + return key->security + selinux_blob_sizes.lbs_key;
> +}
> +#endif /* CONFIG_KEYS */
> +
> static inline struct sk_security_struct *selinux_sock(const struct sock *sock)
> {
> return sock->sk_security + selinux_blob_sizes.lbs_sock;
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 4ac4bf3310d7..7cc3a3382fee 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -386,6 +386,13 @@ static inline struct superblock_smack *smack_superblock(
> return superblock->s_security + smack_blob_sizes.lbs_superblock;
> }
>
> +#ifdef CONFIG_KEYS
> +static inline struct smack_known **smack_key(const struct key *key)
> +{
> + return key->security + smack_blob_sizes.lbs_key;
> +}
> +#endif /* CONFIG_KEYS */
> +
> /*
> * Is the directory transmuting?
> */
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index fd69e1bd841b..e9560b078efe 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4179,23 +4179,13 @@ static void smack_inet_csk_clone(struct sock *sk,
> static int smack_key_alloc(struct key *key, const struct cred *cred,
> unsigned long flags)
> {
> + struct smack_known **blob = smack_key(key);
> struct smack_known *skp = smk_of_task(smack_cred(cred));
>
> - key->security = skp;
> + *blob = skp;
> return 0;
> }
>
> -/**
> - * smack_key_free - Clear the key security blob
> - * @key: the object
> - *
> - * Clear the blob pointer
> - */
> -static void smack_key_free(struct key *key)
> -{
> - key->security = NULL;
> -}
> -
> /**
> * smack_key_permission - Smack access on a key
> * @key_ref: gets to the object
> @@ -4208,6 +4198,8 @@ static void smack_key_free(struct key *key)
> static int smack_key_permission(key_ref_t key_ref,
> const struct cred *cred, unsigned perm)
> {
> + struct smack_known **blob;
> + struct smack_known *skp;
> struct key *keyp;
> struct smk_audit_info ad;
> struct smack_known *tkp = smk_of_task(smack_cred(cred));
> @@ -4227,7 +4219,9 @@ static int smack_key_permission(key_ref_t key_ref,
> * If the key hasn't been initialized give it access so that
> * it may do so.
> */
> - if (keyp->security == NULL)
> + blob = smack_key(keyp);
> + skp = *blob;
> + if (skp == NULL)
> return 0;
> /*
> * This should not occur
> @@ -4247,8 +4241,8 @@ static int smack_key_permission(key_ref_t key_ref,
> request |= MAY_READ;
> if (perm & (KEY_NEED_WRITE | KEY_NEED_LINK | KEY_NEED_SETATTR))
> request |= MAY_WRITE;
> - rc = smk_access(tkp, keyp->security, request, &ad);
> - rc = smk_bu_note("key access", tkp, keyp->security, request, rc);
> + rc = smk_access(tkp, skp, request, &ad);
> + rc = smk_bu_note("key access", tkp, skp, request, rc);
> return rc;
> }
>
> @@ -4263,11 +4257,12 @@ static int smack_key_permission(key_ref_t key_ref,
> */
> static int smack_key_getsecurity(struct key *key, char **_buffer)
> {
> - struct smack_known *skp = key->security;
> + struct smack_known **blob = smack_key(key);
> + struct smack_known *skp = *blob;
> size_t length;
> char *copy;
>
> - if (key->security == NULL) {
> + if (skp == NULL) {
> *_buffer = NULL;
> return 0;
> }
> @@ -4550,6 +4545,9 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
> .lbs_file = sizeof(struct smack_known *),
> .lbs_inode = sizeof(struct inode_smack),
> .lbs_ipc = sizeof(struct smack_known *),
> +#ifdef CONFIG_KEYS
> + .lbs_key = sizeof(struct smack_known *),
> +#endif /* CONFIG_KEYS */
> .lbs_msg_msg = sizeof(struct smack_known *),
> .lbs_sock = sizeof(struct socket_smack),
> .lbs_superblock = sizeof(struct superblock_smack),
> @@ -4671,7 +4669,6 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
> /* key management security hooks */
> #ifdef CONFIG_KEYS
> LSM_HOOK_INIT(key_alloc, smack_key_alloc),
> - LSM_HOOK_INIT(key_free, smack_key_free),
> LSM_HOOK_INIT(key_permission, smack_key_permission),
> LSM_HOOK_INIT(key_getsecurity, smack_key_getsecurity),
> #endif /* CONFIG_KEYS */
>
^ permalink raw reply
* [PATCH v3 04/24] LSM: Create and manage the lsmblob data structure.
From: John Johansen @ 2019-06-24 18:38 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-5-casey@schaufler-ca.com>
On 6/21/19 11:52 AM, Casey Schaufler wrote:
> When more than one security module is exporting data to
> audit and networking sub-systems a single 32 bit integer
> is no longer sufficient to represent the data. Add a
> structure to be used instead.
>
> The lsmblob structure is currently an array of
> u32 "secids". There is an entry for each of the
> security modules built into the system that would
> use secids if active. The system assigns the module
> a "slot" when it registers hooks. If modules are
> compiled in but not registered there will be unused
> slots.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
See below
> ---
> include/linux/lsm_hooks.h | 1 +
> include/linux/security.h | 62 +++++++++++++++++++++++++++++++++++++++
> security/security.c | 36 +++++++++++++++++++++++
> 3 files changed, 99 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 3fe39abccc8f..4d1ddf1a2aa6 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2038,6 +2038,7 @@ struct security_hook_list {
> struct hlist_head *head;
> union security_list_options hook;
> char *lsm;
> + int slot;
> } __randomize_layout;
>
I agree with Kees, *lsm and slot should be moved to the lsm_info
and a lsm_info * should take their place here. This will help
with slot assignment below. But this can come as a separate
patch later.
> /*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 49f2685324b0..0aa9417a5762 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -76,6 +76,68 @@ enum lsm_event {
> LSM_POLICY_CHANGE,
> };
>
> +/*
> + * Data exported by the security modules
> + */
> +#define LSMBLOB_ENTRIES ( \
> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0))
> +
> +struct lsmblob {
> + u32 secid[LSMBLOB_ENTRIES];
> +};
> +
> +#define LSMBLOB_INVALID -1
> +
> +/**
> + * lsmblob_init - initialize an lsmblob structure.
> + * @blob: Pointer to the data to initialize
> + * @secid: The initial secid value
> + *
> + * Set all secid for all modules to the specified value.
> + */
> +static inline void lsmblob_init(struct lsmblob *blob, u32 secid)
> +{
> + int i;
> +
> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
> + blob->secid[i] = secid;
> +}
> +
> +/**
> + * lsmblob_is_set - report if there is an value in the lsmblob
> + * @blob: Pointer to the exported LSM data
> + *
> + * Returns true if there is a secid set, false otherwise
> + */
> +static inline bool lsmblob_is_set(struct lsmblob *blob)
> +{
> + int i;
> +
> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
> + if (blob->secid[i] != 0)
> + return true;
> + return false;
> +}
> +
> +/**
> + * lsmblob_equal - report if the two lsmblob's are equal
> + * @bloba: Pointer to one LSM data
> + * @blobb: Pointer to the other LSM data
> + *
> + * Returns true if all entries in the two are equal, false otherwise
> + */
> +static inline bool lsmblob_equal(struct lsmblob *bloba, struct lsmblob *blobb)
> +{
> + int i;
> +
> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
> + if (bloba->secid[i] != blobb->secid[i])
> + return false;
> + return true;
> +}
> +
> /* These functions are in security/commoncap.c */
> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> int cap, unsigned int opts);
> diff --git a/security/security.c b/security/security.c
> index d05f00a40e82..7618c761060d 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -317,6 +317,7 @@ static void __init ordered_lsm_init(void)
> init_debug("sock blob size = %d\n", blob_sizes.lbs_sock);
> init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> init_debug("task blob size = %d\n", blob_sizes.lbs_task);
> + init_debug("lsmblob size = %lu\n", sizeof(struct lsmblob));
>
> /*
> * Create any kmem_caches needed for blobs
> @@ -420,6 +421,11 @@ static int lsm_append(char *new, char **result)
> return 0;
> }
>
> +/*
> + * Current index to use while initializing the lsmblob secid list.
> + */
> +static int lsm_slot __initdata;
> +
> /**
> * security_add_hooks - Add a modules hooks to the hook lists.
> * @hooks: the hooks to add
> @@ -427,15 +433,45 @@ static int lsm_append(char *new, char **result)
> * @lsm: the name of the security module
> *
> * Each LSM has to register its hooks with the infrastructure.
> + * If the LSM is using hooks that export secids allocate a slot
> + * for it in the lsmblob.
> */
> void __init security_add_hooks(struct security_hook_list *hooks, int count,
> char *lsm)
> {
> + int slot = LSMBLOB_INVALID;
> int i;
>
> for (i = 0; i < count; i++) {
> hooks[i].lsm = lsm;
> hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
> + /*
> + * If this is one of the hooks that uses a secid
> + * note it so that a slot can in allocated for the
> + * secid in the lsmblob structure.
> + */
> + if (hooks[i].head == &security_hook_heads.audit_rule_match ||
> + hooks[i].head == &security_hook_heads.kernel_act_as ||
> + hooks[i].head ==
> + &security_hook_heads.socket_getpeersec_dgram ||
> + hooks[i].head == &security_hook_heads.getprocattr ||
> + hooks[i].head == &security_hook_heads.setprocattr ||
> + hooks[i].head == &security_hook_heads.secctx_to_secid ||
> + hooks[i].head == &security_hook_heads.secid_to_secctx ||
> + hooks[i].head == &security_hook_heads.ipc_getsecid ||
> + hooks[i].head == &security_hook_heads.task_getsecid ||
> + hooks[i].head == &security_hook_heads.inode_getsecid ||
> + hooks[i].head == &security_hook_heads.cred_getsecid) {
> + if (slot == LSMBLOB_INVALID) {
> + slot = lsm_slot++;
> + if (slot >= LSMBLOB_ENTRIES)
> + panic("%s Too many LSMs registered.\n",
> + __func__);
> + init_debug("%s assigned lsmblob slot %d\n",
> + hooks[i].lsm, slot);
> + }
> + }
> + hooks[i].slot = slot;
This is inconsistent, it sets hooks[i].slot before the first occurrence of a hook
in the above list to LSMBLOB_INVALID and the rest of the hooks[i].slots to the
assigned slot value.
If this should only be assigned to the hooks that use a secid then move the
hooks[i].slot = slot assignment into the if. Otherwise we should make sure all
hooks get assigned the same slot value.
> }
> if (lsm_append(lsm, &lsm_names) < 0)
> panic("%s - Cannot get early memory.\n", __func__);
>
^ permalink raw reply
* [PATCH v3 06/24] LSM: Use lsmblob in security_kernel_act_as
From: John Johansen @ 2019-06-24 18:38 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-7-casey@schaufler-ca.com>
On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change the security_kernel_act_as interface to use a lsmblob
> structure in place of the single u32 secid in support of
> module stacking. Change it's only caller, set_security_override,
> to do the same. Change that one's only caller,
> set_security_override_from_ctx, to call it with the new
> parameter type.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> include/linux/cred.h | 3 ++-
> include/linux/security.h | 5 +++--
> kernel/cred.c | 10 ++++++----
> security/security.c | 12 ++++++++++--
> 4 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index efb6edf32de7..9a21c376ed97 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -22,6 +22,7 @@
>
> struct cred;
> struct inode;
> +struct lsmblob;
>
> /*
> * COW Supplementary groups list
> @@ -165,7 +166,7 @@ extern const struct cred *override_creds(const struct cred *);
> extern void revert_creds(const struct cred *);
> extern struct cred *prepare_kernel_cred(struct task_struct *);
> extern int change_create_files_as(struct cred *, struct inode *);
> -extern int set_security_override(struct cred *, u32);
> +extern int set_security_override(struct cred *, struct lsmblob *);
> extern int set_security_override_from_ctx(struct cred *, const char *);
> extern int set_create_files_as(struct cred *, struct inode *);
> extern int cred_fscmp(const struct cred *, const struct cred *);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 52d89c4a9594..4a78516cc74a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -384,7 +384,7 @@ void security_cred_free(struct cred *cred);
> int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
> void security_transfer_creds(struct cred *new, const struct cred *old);
> void security_cred_getsecid(const struct cred *c, u32 *secid);
> -int security_kernel_act_as(struct cred *new, u32 secid);
> +int security_kernel_act_as(struct cred *new, struct lsmblob *blob);
> int security_kernel_create_files_as(struct cred *new, struct inode *inode);
> int security_kernel_module_request(char *kmod_name);
> int security_kernel_load_data(enum kernel_load_data_id id);
> @@ -967,7 +967,8 @@ static inline void security_transfer_creds(struct cred *new,
> {
> }
>
> -static inline int security_kernel_act_as(struct cred *cred, u32 secid)
> +static inline int security_kernel_act_as(struct cred *cred,
> + struct lsmblob *blob)
> {
> return 0;
> }
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 45d77284aed0..71c14dda107e 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -701,14 +701,14 @@ EXPORT_SYMBOL(prepare_kernel_cred);
> /**
> * set_security_override - Set the security ID in a set of credentials
> * @new: The credentials to alter
> - * @secid: The LSM security ID to set
> + * @blob: The LSM security information to set
> *
> * Set the LSM security ID in a set of credentials so that the subjective
> * security is overridden when an alternative set of credentials is used.
> */
> -int set_security_override(struct cred *new, u32 secid)
> +int set_security_override(struct cred *new, struct lsmblob *blob)
> {
> - return security_kernel_act_as(new, secid);
> + return security_kernel_act_as(new, blob);
> }
> EXPORT_SYMBOL(set_security_override);
>
> @@ -724,6 +724,7 @@ EXPORT_SYMBOL(set_security_override);
> */
> int set_security_override_from_ctx(struct cred *new, const char *secctx)
> {
> + struct lsmblob blob;
> u32 secid;
> int ret;
>
> @@ -731,7 +732,8 @@ int set_security_override_from_ctx(struct cred *new, const char *secctx)
> if (ret < 0)
> return ret;
>
> - return set_security_override(new, secid);
> + lsmblob_init(&blob, secid);
> + return set_security_override(new, &blob);
> }
> EXPORT_SYMBOL(set_security_override_from_ctx);
>
> diff --git a/security/security.c b/security/security.c
> index 4692f44718c6..43f8018b9e13 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1646,9 +1646,17 @@ void security_cred_getsecid(const struct cred *c, u32 *secid)
> }
> EXPORT_SYMBOL(security_cred_getsecid);
>
> -int security_kernel_act_as(struct cred *new, u32 secid)
> +int security_kernel_act_as(struct cred *new, struct lsmblob *blob)
> {
> - return call_int_hook(kernel_act_as, 0, new, secid);
> + struct security_hook_list *hp;
> + int rc;
> +
> + hlist_for_each_entry(hp, &security_hook_heads.kernel_act_as, list) {
> + rc = hp->hook.kernel_act_as(new, blob->secid[hp->slot]);
> + if (rc != 0)
> + return rc;
> + }
> + return 0;
> }
>
> int security_kernel_create_files_as(struct cred *new, struct inode *inode)
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox