[PATCH] arm64: fpsimd: Fix state leakage when migrating after sigreturn

Catalin Marinas catalin.marinas at arm.com
Thu Jan 4 10:19:11 PST 2018


On Fri, Dec 15, 2017 at 06:34:38PM +0000, Dave P Martin wrote:
> When refactoring the sigreturn code to handle SVE, I changed the
> sigreturn implementation to store the new FPSIMD state from the
> user sigframe into task_struct before reloading the state into the
> CPU regs.  This makes it easier to convert the data for SVE when
> needed.
> 
> However, it turns out that the fpsimd_state structure passed into
> fpsimd_update_current_state is not fully initialised, so assigning
> the structure as a whole corrupts current->thread.fpsimd_state.cpu
> with uninitialised data.
> 
> This means that if the garbage data written to .cpu happens to be a
> valid cpu number, and the task is subsequently migrated to the cpu
> identified by the that number, and then tries to enter userspace,
> the CPU FPSIMD regs will be assumed to be correct for the task and
> not reloaded as they should be.  This can result in returning to
> userspace with the FPSIMD registers containing data that is stale or
> that belongs to another task or to the kernel.
> 
> Knowingly handing around a kernel structure that is incompletely
> initialised with user data is a potential source of mistakes,
> especially across source file boundaries.  To help avoid a repeat
> of this issue, this patch adapts the relevant internal API to hand
> around the user-accessible subset only: struct user_fpsimd_state.
> 
> To avoid future surprises, this patch also converts all uses of
> struct fpsimd_state that really only access the user subset, to use
> struct user_fpsimd_state.  A few missing consts are added to
> function prototypes for good measure.
> 
> Thanks to Will for spotting the cause of the bug here.
> 
> Reported-by: Geert Uytterhoeven <geert at linux-m68k.org>
> Fixes: 8cd969d28fd2 ("arm64/sve: Signal handling support")
> Signed-off-by: Dave Martin <Dave.Martin at arm.com>
> Cc: Will Deacon <will.deacon at arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>

I've queued this patch for 4.16 but do we actually need "Fixes:" here?
AFAICT, mainline already has the fix, the latest being a45448313706
"arm64: fpsimd: Fix copying of FP state from signal frame into task
struct" in -rc4 (for-next/core is based on -rc3).

-- 
Catalin



More information about the linux-arm-kernel mailing list