Hi David,
On Wed, Dec 04, 2019 at 11:14:08AM +0000, David Laight wrote:
From: Paul Burton
Sent: 03 December 2019 20:50 Our FPU emulator currently uses __get_user() & __put_user() to perform emulated loads & stores. This is problematic because __get_user() & __put_user() are only suitable for naturally aligned memory accesses, and the address we're accessing is entirely under the control of userland.
This allows userland to cause a kernel panic by simply performing an unaligned floating point load or store - the kernel will handle the address error exception by attempting to emulate the instruction, and in the process it may generate another address error exception itself. This time the exception is taken with EPC pointing at the kernels FPU emulation code, and we hit a die_if_kernel() in emulate_load_store_insn().
Won't this be true of almost all code that uses get_user() and put_user() (with or without the leading __).
Only if the address being accessed is under the control of userland to the extent that it can create an unaligned address. You're right that may be more widespread though; it needs checking...
We used to have separate get_user_unaligned() & put_user_unaligned() which would suggest that it's expected that get_user() & put_user() require their accesses be aligned, but they were removed by commit 3170d8d226c2 ("kill {__,}{get,put}_user_unaligned()") in v4.13.
But perhaps we should just take the second AdEL exception & recover via the fixups table. We definitely don't right now... Needs further investigation...
Fix this up by using __copy_from_user() instead of __get_user() and __copy_to_user() instead of __put_user(). These replacements will handle arbitrary alignment without problems.
They'll also kill performance.....
Sure they're heavier, but if you're hitting the FPU emulator you're already slow - trapping to the kernel for instruction emulation is hardly a hot path. If you care about performance at all then this is already a code path to avoid at all costs.
Thanks, Paul