Commit 0abdeff5 authored by Dave Martin's avatar Dave Martin Committed by Catalin Marinas

arm64: fpsimd: Fix state leakage when migrating after sigreturn

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: default avatarGeert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: default avatarDave Martin <Dave.Martin@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: default avatarCatalin Marinas <catalin.marinas@arm.com>
parent 29d9bef1
...@@ -71,7 +71,7 @@ extern void fpsimd_flush_thread(void); ...@@ -71,7 +71,7 @@ extern void fpsimd_flush_thread(void);
extern void fpsimd_signal_preserve_current_state(void); extern void fpsimd_signal_preserve_current_state(void);
extern void fpsimd_preserve_current_state(void); extern void fpsimd_preserve_current_state(void);
extern void fpsimd_restore_current_state(void); extern void fpsimd_restore_current_state(void);
extern void fpsimd_update_current_state(struct fpsimd_state *state); extern void fpsimd_update_current_state(struct user_fpsimd_state const *state);
extern void fpsimd_flush_task_state(struct task_struct *target); extern void fpsimd_flush_task_state(struct task_struct *target);
extern void sve_flush_cpu_state(void); extern void sve_flush_cpu_state(void);
......
...@@ -1036,14 +1036,14 @@ void fpsimd_restore_current_state(void) ...@@ -1036,14 +1036,14 @@ void fpsimd_restore_current_state(void)
* flag that indicates that the FPSIMD register contents are the most recent * flag that indicates that the FPSIMD register contents are the most recent
* FPSIMD state of 'current' * FPSIMD state of 'current'
*/ */
void fpsimd_update_current_state(struct fpsimd_state *state) void fpsimd_update_current_state(struct user_fpsimd_state const *state)
{ {
if (!system_supports_fpsimd()) if (!system_supports_fpsimd())
return; return;
local_bh_disable(); local_bh_disable();
current->thread.fpsimd_state = *state; current->thread.fpsimd_state.user_fpsimd = *state;
if (system_supports_sve() && test_thread_flag(TIF_SVE)) if (system_supports_sve() && test_thread_flag(TIF_SVE))
fpsimd_to_sve(current); fpsimd_to_sve(current);
......
...@@ -178,7 +178,8 @@ static void __user *apply_user_offset( ...@@ -178,7 +178,8 @@ static void __user *apply_user_offset(
static int preserve_fpsimd_context(struct fpsimd_context __user *ctx) static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
{ {
struct fpsimd_state *fpsimd = &current->thread.fpsimd_state; struct user_fpsimd_state const *fpsimd =
&current->thread.fpsimd_state.user_fpsimd;
int err; int err;
/* copy the FP and status/control registers */ /* copy the FP and status/control registers */
...@@ -195,7 +196,7 @@ static int preserve_fpsimd_context(struct fpsimd_context __user *ctx) ...@@ -195,7 +196,7 @@ static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
static int restore_fpsimd_context(struct fpsimd_context __user *ctx) static int restore_fpsimd_context(struct fpsimd_context __user *ctx)
{ {
struct fpsimd_state fpsimd; struct user_fpsimd_state fpsimd;
__u32 magic, size; __u32 magic, size;
int err = 0; int err = 0;
...@@ -266,7 +267,7 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user) ...@@ -266,7 +267,7 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user)
{ {
int err; int err;
unsigned int vq; unsigned int vq;
struct fpsimd_state fpsimd; struct user_fpsimd_state fpsimd;
struct sve_context sve; struct sve_context sve;
if (__copy_from_user(&sve, user->sve, sizeof(sve))) if (__copy_from_user(&sve, user->sve, sizeof(sve)))
......
...@@ -228,7 +228,8 @@ union __fpsimd_vreg { ...@@ -228,7 +228,8 @@ union __fpsimd_vreg {
static int compat_preserve_vfp_context(struct compat_vfp_sigframe __user *frame) static int compat_preserve_vfp_context(struct compat_vfp_sigframe __user *frame)
{ {
struct fpsimd_state *fpsimd = &current->thread.fpsimd_state; struct user_fpsimd_state const *fpsimd =
&current->thread.fpsimd_state.user_fpsimd;
compat_ulong_t magic = VFP_MAGIC; compat_ulong_t magic = VFP_MAGIC;
compat_ulong_t size = VFP_STORAGE_SIZE; compat_ulong_t size = VFP_STORAGE_SIZE;
compat_ulong_t fpscr, fpexc; compat_ulong_t fpscr, fpexc;
...@@ -277,7 +278,7 @@ static int compat_preserve_vfp_context(struct compat_vfp_sigframe __user *frame) ...@@ -277,7 +278,7 @@ static int compat_preserve_vfp_context(struct compat_vfp_sigframe __user *frame)
static int compat_restore_vfp_context(struct compat_vfp_sigframe __user *frame) static int compat_restore_vfp_context(struct compat_vfp_sigframe __user *frame)
{ {
struct fpsimd_state fpsimd; struct user_fpsimd_state fpsimd;
compat_ulong_t magic = VFP_MAGIC; compat_ulong_t magic = VFP_MAGIC;
compat_ulong_t size = VFP_STORAGE_SIZE; compat_ulong_t size = VFP_STORAGE_SIZE;
compat_ulong_t fpscr; compat_ulong_t fpscr;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment