diff mbox series

[v2,2/2] powerpc/rtas: block error injection when locked down

Message ID 20220926131643.146502-3-nathanl@linux.ibm.com (mailing list archive)
State Handled Elsewhere
Headers show
Series powerpc/pseries: restrict error injection and DT changes when locked down | expand

Commit Message

Nathan Lynch Sept. 26, 2022, 1:16 p.m. UTC
The error injection facility on pseries VMs allows corruption of
arbitrary guest memory, potentially enabling a sufficiently privileged
user to disable lockdown or perform other modifications of the running
kernel via the rtas syscall.

Block the PAPR error injection facility from being opened or called
when locked down.

Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
 arch/powerpc/kernel/rtas.c | 25 ++++++++++++++++++++++++-
 include/linux/security.h   |  1 +
 security/security.c        |  1 +
 3 files changed, 26 insertions(+), 1 deletion(-)

Comments

Paul Moore Sept. 26, 2022, 10:41 p.m. UTC | #1
On Mon, Sep 26, 2022 at 9:18 AM Nathan Lynch <nathanl@linux.ibm.com> wrote:
>
> The error injection facility on pseries VMs allows corruption of
> arbitrary guest memory, potentially enabling a sufficiently privileged
> user to disable lockdown or perform other modifications of the running
> kernel via the rtas syscall.
>
> Block the PAPR error injection facility from being opened or called
> when locked down.
>
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
> ---
>  arch/powerpc/kernel/rtas.c | 25 ++++++++++++++++++++++++-
>  include/linux/security.h   |  1 +
>  security/security.c        |  1 +
>  3 files changed, 26 insertions(+), 1 deletion(-)

The lockdown changes are trivial, but they look fine to me.

Acked-by: Paul Moore <paul@paul-moore.com> (LSM)
Andrew Donnellan Sept. 28, 2022, 10:02 a.m. UTC | #2
On Mon, 2022-09-26 at 08:16 -0500, Nathan Lynch wrote:
> The error injection facility on pseries VMs allows corruption of
> arbitrary guest memory, potentially enabling a sufficiently
> privileged
> user to disable lockdown or perform other modifications of the
> running
> kernel via the rtas syscall.
> 
> Block the PAPR error injection facility from being opened or called
> when locked down.
> 
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>

Is there any circumstance (short of arbitrary code execution etc) where
the rtas_call() check will actually trigger rather than the sys_rtas()
check? (Not that it matters, defence in depth is good.)

Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>

> ---
>  arch/powerpc/kernel/rtas.c | 25 ++++++++++++++++++++++++-
>  include/linux/security.h   |  1 +
>  security/security.c        |  1 +
>  3 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 693133972294..c2540d393f1c 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -23,6 +23,7 @@
>  #include <linux/memblock.h>
>  #include <linux/slab.h>
>  #include <linux/reboot.h>
> +#include <linux/security.h>
>  #include <linux/syscalls.h>
>  #include <linux/of.h>
>  #include <linux/of_fdt.h>
> @@ -464,6 +465,9 @@ void rtas_call_unlocked(struct rtas_args *args,
> int token, int nargs, int nret,
>         va_end(list);
>  }
>  
> +static int ibm_open_errinjct_token;
> +static int ibm_errinjct_token;
> +
>  int rtas_call(int token, int nargs, int nret, int *outputs, ...)
>  {
>         va_list list;
> @@ -476,6 +480,16 @@ int rtas_call(int token, int nargs, int nret,
> int *outputs, ...)
>         if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
>                 return -1;
>  
> +       if (token == ibm_open_errinjct_token || token ==
> ibm_errinjct_token) {
> +               /*
> +                * It would be nicer to not discard the error value
> +                * from security_locked_down(), but callers expect an
> +                * RTAS status, not an errno.
> +                */
> +               if
> (security_locked_down(LOCKDOWN_RTAS_ERROR_INJECTION))
> +                       return -1;
> +       }
> +
>         if ((mfmsr() & (MSR_IR|MSR_DR)) != (MSR_IR|MSR_DR)) {
>                 WARN_ON_ONCE(1);
>                 return -1;
> @@ -1227,6 +1241,14 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user
> *, uargs)
>         if (block_rtas_call(token, nargs, &args))
>                 return -EINVAL;
>  
> +       if (token == ibm_open_errinjct_token || token ==
> ibm_errinjct_token) {
> +               int err;
> +
> +               err =
> security_locked_down(LOCKDOWN_RTAS_ERROR_INJECTION);
> +               if (err)
> +                       return err;
> +       }
> +
>         /* Need to handle ibm,suspend_me call specially */
>         if (token == rtas_token("ibm,suspend-me")) {
>  
> @@ -1325,7 +1347,8 @@ void __init rtas_initialize(void)
>  #ifdef CONFIG_RTAS_ERROR_LOGGING
>         rtas_last_error_token = rtas_token("rtas-last-error");
>  #endif
> -
> +       ibm_open_errinjct_token = rtas_token("ibm,open-errinjct");
> +       ibm_errinjct_token = rtas_token("ibm,errinjct");
>         rtas_syscall_filter_init();
>  }
>  
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 39e7c0e403d9..70f89dc3a712 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -123,6 +123,7 @@ enum lockdown_reason {
>         LOCKDOWN_XMON_WR,
>         LOCKDOWN_BPF_WRITE_USER,
>         LOCKDOWN_DBG_WRITE_KERNEL,
> +       LOCKDOWN_RTAS_ERROR_INJECTION,
>         LOCKDOWN_INTEGRITY_MAX,
>         LOCKDOWN_KCORE,
>         LOCKDOWN_KPROBES,
> diff --git a/security/security.c b/security/security.c
> index 51bf66d4f472..eabe3ce7e74e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -61,6 +61,7 @@ const char *const
> lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
>         [LOCKDOWN_XMON_WR] = "xmon write access",
>         [LOCKDOWN_BPF_WRITE_USER] = "use of bpf to write user RAM",
>         [LOCKDOWN_DBG_WRITE_KERNEL] = "use of kgdb/kdb to write
> kernel RAM",
> +       [LOCKDOWN_RTAS_ERROR_INJECTION] = "RTAS error injection",
>         [LOCKDOWN_INTEGRITY_MAX] = "integrity",
>         [LOCKDOWN_KCORE] = "/proc/kcore access",
>         [LOCKDOWN_KPROBES] = "use of kprobes",
Nathan Lynch Sept. 28, 2022, 4:23 p.m. UTC | #3
Andrew Donnellan <ajd@linux.ibm.com> writes:

> On Mon, 2022-09-26 at 08:16 -0500, Nathan Lynch wrote:
>> The error injection facility on pseries VMs allows corruption of
>> arbitrary guest memory, potentially enabling a sufficiently
>> privileged
>> user to disable lockdown or perform other modifications of the
>> running
>> kernel via the rtas syscall.
>> 
>> Block the PAPR error injection facility from being opened or called
>> when locked down.
>> 
>> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
>
> Is there any circumstance (short of arbitrary code execution etc) where
> the rtas_call() check will actually trigger rather than the sys_rtas()
> check? (Not that it matters, defence in depth is good.)

Fair question! There are no in-kernel users of rtas_call() that pass the
error injection tokens as far as I could tell. Nor am I aware of any
out-of-tree users, for that matter. But rtas_call() is the likely most
appropriate place to have the lockdown gate should that situation change
(as it might, see https://github.com/ibm-power-utilities/librtas/issues/29).
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 693133972294..c2540d393f1c 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -23,6 +23,7 @@ 
 #include <linux/memblock.h>
 #include <linux/slab.h>
 #include <linux/reboot.h>
+#include <linux/security.h>
 #include <linux/syscalls.h>
 #include <linux/of.h>
 #include <linux/of_fdt.h>
@@ -464,6 +465,9 @@  void rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int nret,
 	va_end(list);
 }
 
+static int ibm_open_errinjct_token;
+static int ibm_errinjct_token;
+
 int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 {
 	va_list list;
@@ -476,6 +480,16 @@  int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
 		return -1;
 
+	if (token == ibm_open_errinjct_token || token == ibm_errinjct_token) {
+		/*
+		 * It would be nicer to not discard the error value
+		 * from security_locked_down(), but callers expect an
+		 * RTAS status, not an errno.
+		 */
+		if (security_locked_down(LOCKDOWN_RTAS_ERROR_INJECTION))
+			return -1;
+	}
+
 	if ((mfmsr() & (MSR_IR|MSR_DR)) != (MSR_IR|MSR_DR)) {
 		WARN_ON_ONCE(1);
 		return -1;
@@ -1227,6 +1241,14 @@  SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 	if (block_rtas_call(token, nargs, &args))
 		return -EINVAL;
 
+	if (token == ibm_open_errinjct_token || token == ibm_errinjct_token) {
+		int err;
+
+		err = security_locked_down(LOCKDOWN_RTAS_ERROR_INJECTION);
+		if (err)
+			return err;
+	}
+
 	/* Need to handle ibm,suspend_me call specially */
 	if (token == rtas_token("ibm,suspend-me")) {
 
@@ -1325,7 +1347,8 @@  void __init rtas_initialize(void)
 #ifdef CONFIG_RTAS_ERROR_LOGGING
 	rtas_last_error_token = rtas_token("rtas-last-error");
 #endif
-
+	ibm_open_errinjct_token = rtas_token("ibm,open-errinjct");
+	ibm_errinjct_token = rtas_token("ibm,errinjct");
 	rtas_syscall_filter_init();
 }
 
diff --git a/include/linux/security.h b/include/linux/security.h
index 39e7c0e403d9..70f89dc3a712 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -123,6 +123,7 @@  enum lockdown_reason {
 	LOCKDOWN_XMON_WR,
 	LOCKDOWN_BPF_WRITE_USER,
 	LOCKDOWN_DBG_WRITE_KERNEL,
+	LOCKDOWN_RTAS_ERROR_INJECTION,
 	LOCKDOWN_INTEGRITY_MAX,
 	LOCKDOWN_KCORE,
 	LOCKDOWN_KPROBES,
diff --git a/security/security.c b/security/security.c
index 51bf66d4f472..eabe3ce7e74e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -61,6 +61,7 @@  const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
 	[LOCKDOWN_XMON_WR] = "xmon write access",
 	[LOCKDOWN_BPF_WRITE_USER] = "use of bpf to write user RAM",
 	[LOCKDOWN_DBG_WRITE_KERNEL] = "use of kgdb/kdb to write kernel RAM",
+	[LOCKDOWN_RTAS_ERROR_INJECTION] = "RTAS error injection",
 	[LOCKDOWN_INTEGRITY_MAX] = "integrity",
 	[LOCKDOWN_KCORE] = "/proc/kcore access",
 	[LOCKDOWN_KPROBES] = "use of kprobes",