[PATCH 0/4] MIPS: Prevent users killing the kernel or spamming its log

[PATCH 0/4] MIPS: Prevent users killing the kernel or spamming its log

[Paul Burton]

This series fixes up an issue discovered running crashme - user code can
cause the kernel to die() by performing unaligned accesses to the GIC
user page. The first patch fixes this by allowing the kernel to recover
from bus errors resulting from bad accesses to the GIC user page (or
anywhere else we expect might fault). The remaining 3 patches silence
kernel log output from such bus errors in cases where they are somewhat
expected.

It'd be great to get these into v4.14, but especially patch 1 in order
to prevent users killing the kernel.

Thanks,
    Paul

Paul Burton (4):
  MIPS: Search main exception table for data bus errors
  MIPS: Don't dump CM error state for fixed up bus errors
  MIPS: Allow bus error handlers to request quiet behaviour
  MIPS: Silence kernel log output for GIC user page bus errors

 arch/mips/include/asm/mips-cm.h | 11 ++++++++---
 arch/mips/include/asm/traps.h   |  1 +
 arch/mips/kernel/mips-cm.c      | 39 ++++++++++++++++++++++++++++++++++-----
 arch/mips/kernel/traps.c        | 15 +++++++++------
 4 files changed, 52 insertions(+), 14 deletions(-)

-- 
2.14.1

[PATCH 0/4] MIPS: Prevent users killing the kernel or spamming its log

[Paul Burton]

This series fixes up an issue discovered running crashme - user code can
cause the kernel to die() by performing unaligned accesses to the GIC
user page. The first patch fixes this by allowing the kernel to recover
from bus errors resulting from bad accesses to the GIC user page (or
anywhere else we expect might fault). The remaining 3 patches silence
kernel log output from such bus errors in cases where they are somewhat
expected.

It'd be great to get these into v4.14, but especially patch 1 in order
to prevent users killing the kernel.

Thanks,
    Paul

Paul Burton (4):
  MIPS: Search main exception table for data bus errors
  MIPS: Don't dump CM error state for fixed up bus errors
  MIPS: Allow bus error handlers to request quiet behaviour
  MIPS: Silence kernel log output for GIC user page bus errors

 arch/mips/include/asm/mips-cm.h | 11 ++++++++---
 arch/mips/include/asm/traps.h   |  1 +
 arch/mips/kernel/mips-cm.c      | 39 ++++++++++++++++++++++++++++++++++-----
 arch/mips/kernel/traps.c        | 15 +++++++++------
 4 files changed, 52 insertions(+), 14 deletions(-)

-- 
2.14.1

[PATCH 1/4] MIPS: Search main exception table for data bus errors

[Paul Burton]

We have 2 exception tables in MIPS kernels:

  - __ex_table which is the main exception table used in places where
    the kernel might fault accessing a user address.

  - __dbe_table which is used in various platform & driver code that
    expects that it might trigger a bus error exception.

When a data bus error exception occurs we only search __dbe_table, and
thus we have the expectation that access to user addresses will not
trigger bus errors.

Sadly, this expectation is not true - at least not since we began
mapping the GIC user page for use with the VDSO in commit a7f4df4e21dd
("MIPS: VDSO: Add implementations of gettimeofday() and
clock_gettime()"). The GIC user page provides user code with direct
access to a hardware-provided memory mapped register interface, albeit a
very simple one containing a single register. Like many register
interfaces however it has limitations - notably like the rest of the GIC
register interface it requires that accesses to it are either 32 bit or
64 bit. Any smaller accesses generate a data bus error exception. Herein
our bug lies - we have no such restrictions upon kernel access to user
memory, and users can freely cause the kernel to attempt smaller than 32
bit accesses in various ways:

  - Perform an unaligned memory access. In cases where this isn't
    handled by the CPU, such as when accessing uncached memory like the
    GIC register interface, we'll proceed to attempt to emulate the
    unaligned access via do_ade() using byte-sized loads or stores on
    MIPSr6 systems.

  - Cause the kernel to invoke __copy_from_user(), __copy_to_user() or
    one of their variants acting upon uncached memory with either a
    non-32bit-aligned address or size. Similarly this will cause the
    kernel to perform smaller than 32 bit memory accesses. Many syscalls
    will allow this to be triggered.

When the kernel attempts smaller than 32 bit access to the GIC user page
via any of these means, it generates a bus error exception. We then
check __dbe_table for a fixup, find none & call die_if_kernel() from
do_be(). Essentially we allow user code to kill the kernel, or rather to
cause the kernel to kill itself.

This patch fixes this problem rather simply by searching __ex_table for
fixups if we take a data bus error exception which has no fixup in
__dbe_table. All of the vulnerable user memory accesses should already
have entries in __ex_table, and making use of them seems reasonable.

I have marked this for stable backport as far as v4.4 which introduced
the VDSO, and provided users with access to the GIC user page in commit
a7f4df4e21dd ("MIPS: VDSO: Add implementations of gettimeofday() and
clock_gettime()"). Searching __ex_table may have made sense prior to
that, but I'm currently unaware of any other cases in which it could
cause problems.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: a7f4df4e21dd ("MIPS: VDSO: Add implementations of gettimeofday() and clock_gettime()")
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: stable <stable@vger.kernel.org> # v4.4+
---

 arch/mips/kernel/traps.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 5669d3b8bd38..e7190e5ae427 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -432,6 +432,8 @@ static const struct exception_table_entry *search_dbe_tables(unsigned long addr)
 			   __stop___dbe_table - __start___dbe_table, addr);
 	if (!e)
 		e = search_module_dbetables(addr);
+	if (!e)
+		e = search_exception_tables(addr);
 	return e;
 }
 
@@ -444,7 +446,6 @@ asmlinkage void do_be(struct pt_regs *regs)
 	enum ctx_state prev_state;
 
 	prev_state = exception_enter();
-	/* XXX For now.	 Fixme, this searches the wrong table ...  */
 	if (data && !user_mode(regs))
 		fixup = search_dbe_tables(exception_epc(regs));
 
-- 
2.14.1

[PATCH 1/4] MIPS: Search main exception table for data bus errors

[Paul Burton]

We have 2 exception tables in MIPS kernels:

  - __ex_table which is the main exception table used in places where
    the kernel might fault accessing a user address.

  - __dbe_table which is used in various platform & driver code that
    expects that it might trigger a bus error exception.

When a data bus error exception occurs we only search __dbe_table, and
thus we have the expectation that access to user addresses will not
trigger bus errors.

Sadly, this expectation is not true - at least not since we began
mapping the GIC user page for use with the VDSO in commit a7f4df4e21dd
("MIPS: VDSO: Add implementations of gettimeofday() and
clock_gettime()"). The GIC user page provides user code with direct
access to a hardware-provided memory mapped register interface, albeit a
very simple one containing a single register. Like many register
interfaces however it has limitations - notably like the rest of the GIC
register interface it requires that accesses to it are either 32 bit or
64 bit. Any smaller accesses generate a data bus error exception. Herein
our bug lies - we have no such restrictions upon kernel access to user
memory, and users can freely cause the kernel to attempt smaller than 32
bit accesses in various ways:

  - Perform an unaligned memory access. In cases where this isn't
    handled by the CPU, such as when accessing uncached memory like the
    GIC register interface, we'll proceed to attempt to emulate the
    unaligned access via do_ade() using byte-sized loads or stores on
    MIPSr6 systems.

  - Cause the kernel to invoke __copy_from_user(), __copy_to_user() or
    one of their variants acting upon uncached memory with either a
    non-32bit-aligned address or size. Similarly this will cause the
    kernel to perform smaller than 32 bit memory accesses. Many syscalls
    will allow this to be triggered.

When the kernel attempts smaller than 32 bit access to the GIC user page
via any of these means, it generates a bus error exception. We then
check __dbe_table for a fixup, find none & call die_if_kernel() from
do_be(). Essentially we allow user code to kill the kernel, or rather to
cause the kernel to kill itself.

This patch fixes this problem rather simply by searching __ex_table for
fixups if we take a data bus error exception which has no fixup in
__dbe_table. All of the vulnerable user memory accesses should already
have entries in __ex_table, and making use of them seems reasonable.

I have marked this for stable backport as far as v4.4 which introduced
the VDSO, and provided users with access to the GIC user page in commit
a7f4df4e21dd ("MIPS: VDSO: Add implementations of gettimeofday() and
clock_gettime()"). Searching __ex_table may have made sense prior to
that, but I'm currently unaware of any other cases in which it could
cause problems.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: a7f4df4e21dd ("MIPS: VDSO: Add implementations of gettimeofday() and clock_gettime()")
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: stable <stable@vger.kernel.org> # v4.4+
---

 arch/mips/kernel/traps.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 5669d3b8bd38..e7190e5ae427 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -432,6 +432,8 @@ static const struct exception_table_entry *search_dbe_tables(unsigned long addr)
 			   __stop___dbe_table - __start___dbe_table, addr);
 	if (!e)
 		e = search_module_dbetables(addr);
+	if (!e)
+		e = search_exception_tables(addr);
 	return e;
 }
 
@@ -444,7 +446,6 @@ asmlinkage void do_be(struct pt_regs *regs)
 	enum ctx_state prev_state;
 
 	prev_state = exception_enter();
-	/* XXX For now.	 Fixme, this searches the wrong table ...  */
 	if (data && !user_mode(regs))
 		fixup = search_dbe_tables(exception_epc(regs));
 
-- 
2.14.1

[PATCH 2/4] MIPS: Don't dump CM error state for fixed up bus errors

[Paul Burton]

If a bus error has a fixup then this implies that it is somewhat
expected. Therefore avoid printing out CM error state when a fixup
exists.

One case in which user code can trigger this is performing unaligned
accesses to the GIC user page, which the kernel then attempts to emulate
with byte sized memory accesses that trigger bus errors. These are fixed
up and ultimately harmless, so spamming the kernel log with CM error
information for these cases seems inappropriate.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---

 arch/mips/kernel/traps.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index e7190e5ae427..259e2d259204 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -449,11 +449,10 @@ asmlinkage void do_be(struct pt_regs *regs)
 	if (data && !user_mode(regs))
 		fixup = search_dbe_tables(exception_epc(regs));
 
-	if (fixup)
-		action = MIPS_BE_FIXUP;
-
 	if (board_be_handler)
 		action = board_be_handler(regs, fixup != NULL);
+	else if (fixup)
+		action = MIPS_BE_FIXUP;
 	else
 		mips_cm_error_report();
 
-- 
2.14.1

[PATCH 2/4] MIPS: Don't dump CM error state for fixed up bus errors

[Paul Burton]

If a bus error has a fixup then this implies that it is somewhat
expected. Therefore avoid printing out CM error state when a fixup
exists.

One case in which user code can trigger this is performing unaligned
accesses to the GIC user page, which the kernel then attempts to emulate
with byte sized memory accesses that trigger bus errors. These are fixed
up and ultimately harmless, so spamming the kernel log with CM error
information for these cases seems inappropriate.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---

 arch/mips/kernel/traps.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index e7190e5ae427..259e2d259204 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -449,11 +449,10 @@ asmlinkage void do_be(struct pt_regs *regs)
 	if (data && !user_mode(regs))
 		fixup = search_dbe_tables(exception_epc(regs));
 
-	if (fixup)
-		action = MIPS_BE_FIXUP;
-
 	if (board_be_handler)
 		action = board_be_handler(regs, fixup != NULL);
+	else if (fixup)
+		action = MIPS_BE_FIXUP;
 	else
 		mips_cm_error_report();
 
-- 
2.14.1

[PATCH 3/4] MIPS: Allow bus error handlers to request quiet behaviour

[Paul Burton]

Add a new MIPS_BE_FATAL_QUIET value which bus error handlers can return
to indicate that a bus error triggered by user code should be treated
the same way as MIPS_BE_FATAL, ie. we should deliver a SIGBUS, but the
kernel shouldn't print information about the bus error to the kernel
log.

This will be useful in a further commit which will silence kernel log
output from bus errors caused by user code performing smaller than 32
bit accesses to the GIC user page, which generates a bus error. Notably
this will silence a ton of output from crashme, which seems to be rather
eager to access the GIC user page in weird & wonderful ways.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---

 arch/mips/include/asm/traps.h | 1 +
 arch/mips/kernel/traps.c      | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/traps.h b/arch/mips/include/asm/traps.h
index f41cf3ee82a7..7594fb496001 100644
--- a/arch/mips/include/asm/traps.h
+++ b/arch/mips/include/asm/traps.h
@@ -17,6 +17,7 @@
 #define MIPS_BE_DISCARD 0		/* return with no action */
 #define MIPS_BE_FIXUP	1		/* return to the fixup code */
 #define MIPS_BE_FATAL	2		/* treat as an unrecoverable error */
+#define MIPS_BE_FATAL_QUIET	3	/* treat as an unrecoverable error */
 
 extern void (*board_be_init)(void);
 extern int (*board_be_handler)(struct pt_regs *regs, int is_fixup);
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 259e2d259204..2f4546e4abdb 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -465,6 +465,9 @@ asmlinkage void do_be(struct pt_regs *regs)
 			goto out;
 		}
 		break;
+	case MIPS_BE_FATAL_QUIET:
+		if (user_mode(regs))
+			goto out_sigbus;
 	default:
 		break;
 	}
@@ -480,8 +483,8 @@ asmlinkage void do_be(struct pt_regs *regs)
 		goto out;
 
 	die_if_kernel("Oops", regs);
+out_sigbus:
 	force_sig(SIGBUS, current);
-
 out:
 	exception_exit(prev_state);
 }
-- 
2.14.1

[PATCH 3/4] MIPS: Allow bus error handlers to request quiet behaviour

[Paul Burton]

Add a new MIPS_BE_FATAL_QUIET value which bus error handlers can return
to indicate that a bus error triggered by user code should be treated
the same way as MIPS_BE_FATAL, ie. we should deliver a SIGBUS, but the
kernel shouldn't print information about the bus error to the kernel
log.

This will be useful in a further commit which will silence kernel log
output from bus errors caused by user code performing smaller than 32
bit accesses to the GIC user page, which generates a bus error. Notably
this will silence a ton of output from crashme, which seems to be rather
eager to access the GIC user page in weird & wonderful ways.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---

 arch/mips/include/asm/traps.h | 1 +
 arch/mips/kernel/traps.c      | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/traps.h b/arch/mips/include/asm/traps.h
index f41cf3ee82a7..7594fb496001 100644
--- a/arch/mips/include/asm/traps.h
+++ b/arch/mips/include/asm/traps.h
@@ -17,6 +17,7 @@
 #define MIPS_BE_DISCARD 0		/* return with no action */
 #define MIPS_BE_FIXUP	1		/* return to the fixup code */
 #define MIPS_BE_FATAL	2		/* treat as an unrecoverable error */
+#define MIPS_BE_FATAL_QUIET	3	/* treat as an unrecoverable error */
 
 extern void (*board_be_init)(void);
 extern int (*board_be_handler)(struct pt_regs *regs, int is_fixup);
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 259e2d259204..2f4546e4abdb 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -465,6 +465,9 @@ asmlinkage void do_be(struct pt_regs *regs)
 			goto out;
 		}
 		break;
+	case MIPS_BE_FATAL_QUIET:
+		if (user_mode(regs))
+			goto out_sigbus;
 	default:
 		break;
 	}
@@ -480,8 +483,8 @@ asmlinkage void do_be(struct pt_regs *regs)
 		goto out;
 
 	die_if_kernel("Oops", regs);
+out_sigbus:
 	force_sig(SIGBUS, current);
-
 out:
 	exception_exit(prev_state);
 }
-- 
2.14.1

[PATCH 4/4] MIPS: Silence kernel log output for GIC user page bus errors

[Paul Burton]

Since commit a7f4df4e21dd ("MIPS: VDSO: Add implementations of
gettimeofday() and clock_gettime()") we have mapped the GIC user page
into user memory for use with the VDSO when a GIC is present. This
exposes a hardware-provided GIC register interface to user code, albeit
a very simple one. Unfortunately despite the simplicity of the register
interface this does present an unusual scenario - like many register
interfaces, the GIC requires accesses to meet certain criteria. In
current systems the GIC can only be accessed by non-atomic 32 bit or 64
bit loads or stores. Any other accesses, for example half-word or byte
sized accesses, will generate a bus error. Since user code has access to
this register interface it can generate such bus errors easily.

The biggest issues with this have already been fixed by earlier commits,
but user code is still able to cause the kernel to spam its log with CM
error information by simply generating a stream of bus errors. One
non-malicious example of this happening is use of crashme, which seems
prone to generate all kinds of weird & wonderful accesses to the GIC
user page - presumably its base address is commonly left in a GPR
following calls to the VDSO.

This patch silences such unwanted output in the kernel log by detecting
when a bus error was generated by user code accessing the GIC user page,
not printing the CM error information & returning MIPS_BE_FATAL_QUIET to
quietly deliver a SIGBUS in such cases. If a program does accidentally
generate such a bus error & doesn't have a SIGBUS handler registered
then we'll still dump its register & stack state when delivering the
unhandled signal. If the bus error is somewhat expected, ie. the program
has registered a handler for it as crashme will, or is being ptraced,
then the result is that we'll print nothing to the kernel log.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org

---

 arch/mips/include/asm/mips-cm.h | 11 ++++++++---
 arch/mips/kernel/mips-cm.c      | 39 ++++++++++++++++++++++++++++++++++-----
 arch/mips/kernel/traps.c        |  2 +-
 3 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/arch/mips/include/asm/mips-cm.h b/arch/mips/include/asm/mips-cm.h
index f6231b91b724..650c9db2dee9 100644
--- a/arch/mips/include/asm/mips-cm.h
+++ b/arch/mips/include/asm/mips-cm.h
@@ -17,6 +17,7 @@
 
 #include <linux/bitops.h>
 #include <linux/errno.h>
+#include <asm/traps.h>
 
 /* The base address of the CM GCR block */
 extern void __iomem *mips_gcr_base;
@@ -51,12 +52,16 @@ extern phys_addr_t __mips_cm_phys_base(void);
 extern int mips_cm_is64;
 
 /**
- * mips_cm_error_report - Report CM cache errors
+ * mips_cm_be_handler - Report CM cache errors
  */
+struct pt_regs;
 #ifdef CONFIG_MIPS_CM
-extern void mips_cm_error_report(void);
+extern int mips_cm_be_handler(struct pt_regs *regs);
 #else
-static inline void mips_cm_error_report(void) {}
+static inline int mips_cm_be_handler(struct pt_regs *regs)
+{
+	return MIPS_BE_FATAL;
+}
 #endif
 
 /**
diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index e91c8c4e2eb5..2ccb0a8b7e90 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -14,6 +14,8 @@
 
 #include <asm/mips-cps.h>
 #include <asm/mipsregs.h>
+#include <asm/ptrace.h>
+#include <asm/traps.h>
 
 void __iomem *mips_gcr_base;
 void __iomem *mips_cm_l2sync_base;
@@ -332,19 +334,44 @@ void mips_cm_unlock_other(void)
 	preempt_enable();
 }
 
-void mips_cm_error_report(void)
+int mips_cm_be_handler(struct pt_regs *regs)
 {
+	phys_addr_t gic_usr_start, gic_usr_end;
 	u64 cm_error, cm_addr, cm_other;
 	unsigned long revision;
 	int ocause, cause;
 	char buf[256];
 
 	if (!mips_cm_present())
-		return;
+		return MIPS_BE_FATAL;
+
+	cm_addr = read_gcr_error_addr();
+
+	/*
+	 * If a GIC is present then we may have exposed its user page directly
+	 * to user code. This can allow user code to generate bus errors by
+	 * performing unsupported access types to the GIC register interface -
+	 * ie. anything other than a regular non-atomic 32 or 64 bit load or
+	 * store.
+	 *
+	 * Such bus errors are essentially harmless & don't justify the kernel
+	 * spamming the log with CM error information. We'll already dump
+	 * information about the process when appropriate if it doesn't handle
+	 * the resulting SIGBUS, and if it does (eg. crashme) then we ought not
+	 * to be spamming the kernel log at all. Thus if we detect that the bus
+	 * error was caused by user code accessing the GIC user page we return
+	 * MIPS_BE_FATAL_QUIET to quietly deliver a SIGBUS.
+	 */
+	if (user_mode(regs) && mips_gic_present()) {
+		gic_usr_start = virt_to_phys(mips_gic_base) + MIPS_GIC_USER_OFS;
+		gic_usr_end = gic_usr_start + MIPS_GIC_USER_SZ;
+
+		if ((cm_addr >= gic_usr_start) && (cm_addr < gic_usr_end))
+			return MIPS_BE_FATAL_QUIET;
+	}
 
 	revision = mips_cm_revision();
 	cm_error = read_gcr_error_cause();
-	cm_addr = read_gcr_error_addr();
 	cm_other = read_gcr_error_mult();
 
 	if (revision < CM_REV_CM3) { /* CM2 */
@@ -352,7 +379,7 @@ void mips_cm_error_report(void)
 		ocause = cm_other >> __ffs(CM_GCR_ERROR_MULT_ERR2ND);
 
 		if (!cause)
-			return;
+			return MIPS_BE_FATAL;
 
 		if (cause < 16) {
 			unsigned long cca_bits = (cm_error >> 15) & 7;
@@ -395,7 +422,7 @@ void mips_cm_error_report(void)
 		ocause = cm_other >> __ffs(CM_GCR_ERROR_MULT_ERR2ND);
 
 		if (!cause)
-			return;
+			return MIPS_BE_FATAL;
 
 		/* Used by cause == {1,2,3} */
 		core_id_bits = (cm_error >> 22) & 0xf;
@@ -459,4 +486,6 @@ void mips_cm_error_report(void)
 
 	/* reprime cause register */
 	write_gcr_error_cause(0);
+
+	return MIPS_BE_FATAL;
 }
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 2f4546e4abdb..36f21e1f0488 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -454,7 +454,7 @@ asmlinkage void do_be(struct pt_regs *regs)
 	else if (fixup)
 		action = MIPS_BE_FIXUP;
 	else
-		mips_cm_error_report();
+		action = mips_cm_be_handler(regs);
 
 	switch (action) {
 	case MIPS_BE_DISCARD:
-- 
2.14.1

[PATCH 4/4] MIPS: Silence kernel log output for GIC user page bus errors

[Paul Burton]

Since commit a7f4df4e21dd ("MIPS: VDSO: Add implementations of
gettimeofday() and clock_gettime()") we have mapped the GIC user page
into user memory for use with the VDSO when a GIC is present. This
exposes a hardware-provided GIC register interface to user code, albeit
a very simple one. Unfortunately despite the simplicity of the register
interface this does present an unusual scenario - like many register
interfaces, the GIC requires accesses to meet certain criteria. In
current systems the GIC can only be accessed by non-atomic 32 bit or 64
bit loads or stores. Any other accesses, for example half-word or byte
sized accesses, will generate a bus error. Since user code has access to
this register interface it can generate such bus errors easily.

The biggest issues with this have already been fixed by earlier commits,
but user code is still able to cause the kernel to spam its log with CM
error information by simply generating a stream of bus errors. One
non-malicious example of this happening is use of crashme, which seems
prone to generate all kinds of weird & wonderful accesses to the GIC
user page - presumably its base address is commonly left in a GPR
following calls to the VDSO.

This patch silences such unwanted output in the kernel log by detecting
when a bus error was generated by user code accessing the GIC user page,
not printing the CM error information & returning MIPS_BE_FATAL_QUIET to
quietly deliver a SIGBUS in such cases. If a program does accidentally
generate such a bus error & doesn't have a SIGBUS handler registered
then we'll still dump its register & stack state when delivering the
unhandled signal. If the bus error is somewhat expected, ie. the program
has registered a handler for it as crashme will, or is being ptraced,
then the result is that we'll print nothing to the kernel log.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org

---

 arch/mips/include/asm/mips-cm.h | 11 ++++++++---
 arch/mips/kernel/mips-cm.c      | 39 ++++++++++++++++++++++++++++++++++-----
 arch/mips/kernel/traps.c        |  2 +-
 3 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/arch/mips/include/asm/mips-cm.h b/arch/mips/include/asm/mips-cm.h
index f6231b91b724..650c9db2dee9 100644
--- a/arch/mips/include/asm/mips-cm.h
+++ b/arch/mips/include/asm/mips-cm.h
@@ -17,6 +17,7 @@
 
 #include <linux/bitops.h>
 #include <linux/errno.h>
+#include <asm/traps.h>
 
 /* The base address of the CM GCR block */
 extern void __iomem *mips_gcr_base;
@@ -51,12 +52,16 @@ extern phys_addr_t __mips_cm_phys_base(void);
 extern int mips_cm_is64;
 
 /**
- * mips_cm_error_report - Report CM cache errors
+ * mips_cm_be_handler - Report CM cache errors
  */
+struct pt_regs;
 #ifdef CONFIG_MIPS_CM
-extern void mips_cm_error_report(void);
+extern int mips_cm_be_handler(struct pt_regs *regs);
 #else
-static inline void mips_cm_error_report(void) {}
+static inline int mips_cm_be_handler(struct pt_regs *regs)
+{
+	return MIPS_BE_FATAL;
+}
 #endif
 
 /**
diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index e91c8c4e2eb5..2ccb0a8b7e90 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -14,6 +14,8 @@
 
 #include <asm/mips-cps.h>
 #include <asm/mipsregs.h>
+#include <asm/ptrace.h>
+#include <asm/traps.h>
 
 void __iomem *mips_gcr_base;
 void __iomem *mips_cm_l2sync_base;
@@ -332,19 +334,44 @@ void mips_cm_unlock_other(void)
 	preempt_enable();
 }
 
-void mips_cm_error_report(void)
+int mips_cm_be_handler(struct pt_regs *regs)
 {
+	phys_addr_t gic_usr_start, gic_usr_end;
 	u64 cm_error, cm_addr, cm_other;
 	unsigned long revision;
 	int ocause, cause;
 	char buf[256];
 
 	if (!mips_cm_present())
-		return;
+		return MIPS_BE_FATAL;
+
+	cm_addr = read_gcr_error_addr();
+
+	/*
+	 * If a GIC is present then we may have exposed its user page directly
+	 * to user code. This can allow user code to generate bus errors by
+	 * performing unsupported access types to the GIC register interface -
+	 * ie. anything other than a regular non-atomic 32 or 64 bit load or
+	 * store.
+	 *
+	 * Such bus errors are essentially harmless & don't justify the kernel
+	 * spamming the log with CM error information. We'll already dump
+	 * information about the process when appropriate if it doesn't handle
+	 * the resulting SIGBUS, and if it does (eg. crashme) then we ought not
+	 * to be spamming the kernel log at all. Thus if we detect that the bus
+	 * error was caused by user code accessing the GIC user page we return
+	 * MIPS_BE_FATAL_QUIET to quietly deliver a SIGBUS.
+	 */
+	if (user_mode(regs) && mips_gic_present()) {
+		gic_usr_start = virt_to_phys(mips_gic_base) + MIPS_GIC_USER_OFS;
+		gic_usr_end = gic_usr_start + MIPS_GIC_USER_SZ;
+
+		if ((cm_addr >= gic_usr_start) && (cm_addr < gic_usr_end))
+			return MIPS_BE_FATAL_QUIET;
+	}
 
 	revision = mips_cm_revision();
 	cm_error = read_gcr_error_cause();
-	cm_addr = read_gcr_error_addr();
 	cm_other = read_gcr_error_mult();
 
 	if (revision < CM_REV_CM3) { /* CM2 */
@@ -352,7 +379,7 @@ void mips_cm_error_report(void)
 		ocause = cm_other >> __ffs(CM_GCR_ERROR_MULT_ERR2ND);
 
 		if (!cause)
-			return;
+			return MIPS_BE_FATAL;
 
 		if (cause < 16) {
 			unsigned long cca_bits = (cm_error >> 15) & 7;
@@ -395,7 +422,7 @@ void mips_cm_error_report(void)
 		ocause = cm_other >> __ffs(CM_GCR_ERROR_MULT_ERR2ND);
 
 		if (!cause)
-			return;
+			return MIPS_BE_FATAL;
 
 		/* Used by cause == {1,2,3} */
 		core_id_bits = (cm_error >> 22) & 0xf;
@@ -459,4 +486,6 @@ void mips_cm_error_report(void)
 
 	/* reprime cause register */
 	write_gcr_error_cause(0);
+
+	return MIPS_BE_FATAL;
 }
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 2f4546e4abdb..36f21e1f0488 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -454,7 +454,7 @@ asmlinkage void do_be(struct pt_regs *regs)
 	else if (fixup)
 		action = MIPS_BE_FIXUP;
 	else
-		mips_cm_error_report();
+		action = mips_cm_be_handler(regs);
 
 	switch (action) {
 	case MIPS_BE_DISCARD:
-- 
2.14.1

Re: [PATCH 1/4] MIPS: Search main exception table for data bus errors

[Ralf Baechle]

On Thu, Sep 21, 2017 at 11:44:44PM -0700, Paul Burton wrote:

> We have 2 exception tables in MIPS kernels:
> 
>   - __ex_table which is the main exception table used in places where
>     the kernel might fault accessing a user address.
> 
>   - __dbe_table which is used in various platform & driver code that
>     expects that it might trigger a bus error exception.
> 
> When a data bus error exception occurs we only search __dbe_table, and
> thus we have the expectation that access to user addresses will not
> trigger bus errors.
> 
> Sadly, this expectation is not true - at least not since we began
> mapping the GIC user page for use with the VDSO in commit a7f4df4e21dd
> ("MIPS: VDSO: Add implementations of gettimeofday() and
> clock_gettime()"). The GIC user page provides user code with direct
> access to a hardware-provided memory mapped register interface, albeit a
> very simple one containing a single register. Like many register
> interfaces however it has limitations - notably like the rest of the GIC
> register interface it requires that accesses to it are either 32 bit or
> 64 bit. Any smaller accesses generate a data bus error exception. Herein
> our bug lies - we have no such restrictions upon kernel access to user
> memory, and users can freely cause the kernel to attempt smaller than 32
> bit accesses in various ways:
> 
>   - Perform an unaligned memory access. In cases where this isn't
>     handled by the CPU, such as when accessing uncached memory like the
>     GIC register interface, we'll proceed to attempt to emulate the
>     unaligned access via do_ade() using byte-sized loads or stores on
>     MIPSr6 systems.
> 
>   - Cause the kernel to invoke __copy_from_user(), __copy_to_user() or
>     one of their variants acting upon uncached memory with either a
>     non-32bit-aligned address or size. Similarly this will cause the
>     kernel to perform smaller than 32 bit memory accesses. Many syscalls
>     will allow this to be triggered.
> 
> When the kernel attempts smaller than 32 bit access to the GIC user page
> via any of these means, it generates a bus error exception. We then
> check __dbe_table for a fixup, find none & call die_if_kernel() from
> do_be(). Essentially we allow user code to kill the kernel, or rather to
> cause the kernel to kill itself.
> 
> This patch fixes this problem rather simply by searching __ex_table for
> fixups if we take a data bus error exception which has no fixup in
> __dbe_table. All of the vulnerable user memory accesses should already
> have entries in __ex_table, and making use of them seems reasonable.
> 
> I have marked this for stable backport as far as v4.4 which introduced
> the VDSO, and provided users with access to the GIC user page in commit
> a7f4df4e21dd ("MIPS: VDSO: Add implementations of gettimeofday() and
> clock_gettime()"). Searching __ex_table may have made sense prior to
> that, but I'm currently unaware of any other cases in which it could
> cause problems.

Unfortunately the DBE exception is imprecise.  The EPC might actually point
to the far end of the kernel and have no useful relation at all to the
instruction triggering it.

As a consequence a false fixup might be used resulting in very silly and
probably bad things happening.

So this needs a different solution.

  Ralf

Re: [PATCH 1/4] MIPS: Search main exception table for data bus errors

[Paul Burton]

[-- Attachment #1: Type: text/plain, Size: 4656 bytes --]

Hi Ralf,

On Friday, 22 September 2017 02:47:27 PDT Ralf Baechle wrote:
> On Thu, Sep 21, 2017 at 11:44:44PM -0700, Paul Burton wrote:
> > We have 2 exception tables in MIPS kernels:
> >   - __ex_table which is the main exception table used in places where
> >   
> >     the kernel might fault accessing a user address.
> >   
> >   - __dbe_table which is used in various platform & driver code that
> >   
> >     expects that it might trigger a bus error exception.
> > 
> > When a data bus error exception occurs we only search __dbe_table, and
> > thus we have the expectation that access to user addresses will not
> > trigger bus errors.
> > 
> > Sadly, this expectation is not true - at least not since we began
> > mapping the GIC user page for use with the VDSO in commit a7f4df4e21dd
> > ("MIPS: VDSO: Add implementations of gettimeofday() and
> > clock_gettime()"). The GIC user page provides user code with direct
> > access to a hardware-provided memory mapped register interface, albeit a
> > very simple one containing a single register. Like many register
> > interfaces however it has limitations - notably like the rest of the GIC
> > register interface it requires that accesses to it are either 32 bit or
> > 64 bit. Any smaller accesses generate a data bus error exception. Herein
> > our bug lies - we have no such restrictions upon kernel access to user
> > memory, and users can freely cause the kernel to attempt smaller than 32
> > 
> > bit accesses in various ways:
> >   - Perform an unaligned memory access. In cases where this isn't
> >   
> >     handled by the CPU, such as when accessing uncached memory like the
> >     GIC register interface, we'll proceed to attempt to emulate the
> >     unaligned access via do_ade() using byte-sized loads or stores on
> >     MIPSr6 systems.
> >   
> >   - Cause the kernel to invoke __copy_from_user(), __copy_to_user() or
> >   
> >     one of their variants acting upon uncached memory with either a
> >     non-32bit-aligned address or size. Similarly this will cause the
> >     kernel to perform smaller than 32 bit memory accesses. Many syscalls
> >     will allow this to be triggered.
> > 
> > When the kernel attempts smaller than 32 bit access to the GIC user page
> > via any of these means, it generates a bus error exception. We then
> > check __dbe_table for a fixup, find none & call die_if_kernel() from
> > do_be(). Essentially we allow user code to kill the kernel, or rather to
> > cause the kernel to kill itself.
> > 
> > This patch fixes this problem rather simply by searching __ex_table for
> > fixups if we take a data bus error exception which has no fixup in
> > __dbe_table. All of the vulnerable user memory accesses should already
> > have entries in __ex_table, and making use of them seems reasonable.
> > 
> > I have marked this for stable backport as far as v4.4 which introduced
> > the VDSO, and provided users with access to the GIC user page in commit
> > a7f4df4e21dd ("MIPS: VDSO: Add implementations of gettimeofday() and
> > clock_gettime()"). Searching __ex_table may have made sense prior to
> > that, but I'm currently unaware of any other cases in which it could
> > cause problems.
> 
> Unfortunately the DBE exception is imprecise.  The EPC might actually point
> to the far end of the kernel and have no useful relation at all to the
> instruction triggering it.
> 
> As a consequence a false fixup might be used resulting in very silly and
> probably bad things happening.
> 
> So this needs a different solution.
> 
>   Ralf

Fair point, depending upon the CPU. That makes things difficult though...

Handling the unaligned emulation case is "easy" if we simply refuse to emulate 
unaligned access to uncached memory. Generally uncached mappings are going to 
be device I/O which the kernel probably ought not to go attempting to emulate 
without knowing the semantics required for access. Emulating unaligned 
accesses is already a slow path so adding in the check for cacheability seems 
reasonable. I've written a patch which does this & seems to work fine.

Handling copy_from_user()/copy_to_user() & company though is harder. 
Theoretically we could do the same cacheability check in __access_ok() but 
that would add a fair chunk of overhead to regular user memory access. There 
are also the user string functions (strncpy_from_user, strnlen_user) which 
don't appear to check access_ok() at all and presumably just rely on 
recovering from any exceptions via __ex_table.

I'm not sure there's a clean and efficient way to do this without letting the 
bus error happen & recovering afterwards. Any ideas?

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/4] MIPS: Search main exception table for data bus errors

[Paul Burton]

[-- Attachment #1: Type: text/plain, Size: 4656 bytes --]

Hi Ralf,

On Friday, 22 September 2017 02:47:27 PDT Ralf Baechle wrote:
> On Thu, Sep 21, 2017 at 11:44:44PM -0700, Paul Burton wrote:
> > We have 2 exception tables in MIPS kernels:
> >   - __ex_table which is the main exception table used in places where
> >   
> >     the kernel might fault accessing a user address.
> >   
> >   - __dbe_table which is used in various platform & driver code that
> >   
> >     expects that it might trigger a bus error exception.
> > 
> > When a data bus error exception occurs we only search __dbe_table, and
> > thus we have the expectation that access to user addresses will not
> > trigger bus errors.
> > 
> > Sadly, this expectation is not true - at least not since we began
> > mapping the GIC user page for use with the VDSO in commit a7f4df4e21dd
> > ("MIPS: VDSO: Add implementations of gettimeofday() and
> > clock_gettime()"). The GIC user page provides user code with direct
> > access to a hardware-provided memory mapped register interface, albeit a
> > very simple one containing a single register. Like many register
> > interfaces however it has limitations - notably like the rest of the GIC
> > register interface it requires that accesses to it are either 32 bit or
> > 64 bit. Any smaller accesses generate a data bus error exception. Herein
> > our bug lies - we have no such restrictions upon kernel access to user
> > memory, and users can freely cause the kernel to attempt smaller than 32
> > 
> > bit accesses in various ways:
> >   - Perform an unaligned memory access. In cases where this isn't
> >   
> >     handled by the CPU, such as when accessing uncached memory like the
> >     GIC register interface, we'll proceed to attempt to emulate the
> >     unaligned access via do_ade() using byte-sized loads or stores on
> >     MIPSr6 systems.
> >   
> >   - Cause the kernel to invoke __copy_from_user(), __copy_to_user() or
> >   
> >     one of their variants acting upon uncached memory with either a
> >     non-32bit-aligned address or size. Similarly this will cause the
> >     kernel to perform smaller than 32 bit memory accesses. Many syscalls
> >     will allow this to be triggered.
> > 
> > When the kernel attempts smaller than 32 bit access to the GIC user page
> > via any of these means, it generates a bus error exception. We then
> > check __dbe_table for a fixup, find none & call die_if_kernel() from
> > do_be(). Essentially we allow user code to kill the kernel, or rather to
> > cause the kernel to kill itself.
> > 
> > This patch fixes this problem rather simply by searching __ex_table for
> > fixups if we take a data bus error exception which has no fixup in
> > __dbe_table. All of the vulnerable user memory accesses should already
> > have entries in __ex_table, and making use of them seems reasonable.
> > 
> > I have marked this for stable backport as far as v4.4 which introduced
> > the VDSO, and provided users with access to the GIC user page in commit
> > a7f4df4e21dd ("MIPS: VDSO: Add implementations of gettimeofday() and
> > clock_gettime()"). Searching __ex_table may have made sense prior to
> > that, but I'm currently unaware of any other cases in which it could
> > cause problems.
> 
> Unfortunately the DBE exception is imprecise.  The EPC might actually point
> to the far end of the kernel and have no useful relation at all to the
> instruction triggering it.
> 
> As a consequence a false fixup might be used resulting in very silly and
> probably bad things happening.
> 
> So this needs a different solution.
> 
>   Ralf

Fair point, depending upon the CPU. That makes things difficult though...

Handling the unaligned emulation case is "easy" if we simply refuse to emulate 
unaligned access to uncached memory. Generally uncached mappings are going to 
be device I/O which the kernel probably ought not to go attempting to emulate 
without knowing the semantics required for access. Emulating unaligned 
accesses is already a slow path so adding in the check for cacheability seems 
reasonable. I've written a patch which does this & seems to work fine.

Handling copy_from_user()/copy_to_user() & company though is harder. 
Theoretically we could do the same cacheability check in __access_ok() but 
that would add a fair chunk of overhead to regular user memory access. There 
are also the user string functions (strncpy_from_user, strnlen_user) which 
don't appear to check access_ok() at all and presumably just rely on 
recovering from any exceptions via __ex_table.

I'm not sure there's a clean and efficient way to do this without letting the 
bus error happen & recovering afterwards. Any ideas?

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]