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

[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

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 --]