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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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: signature.asc
Description: This is a digitally signed message part.


[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux