On 04/16, Borislav Petkov wrote:
>
> Box exploded with the splat at the end because we do cache
> &target->thread.fpu.state->xsave into the local variable xsave but
> that pointer is NULL at that time and it gets initialized later, in
> init_fpu(), see
>
> e7f180dcd8ab ("x86/fpu: Change xstateregs_get()/set() to use
> ->xsave.i387 rather than ->fxsave")
Yes, my fault.
Thanks a lot Borislav!
Acked-by: Oleg Nesterov <[email protected]>
> Fix is simple: load xsave *after* init_fpu() has run.
>
> BUG: unable to handle kernel NULL pointer dereference at 00000000000001d0
> IP: [<ffffffff8100fe5a>] xstateregs_get+0x7a/0x120
> PGD 7a884067 PUD 7a9aa067 PMD 0
> Oops: 0002 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 4239 Comm: gdb Not tainted 4.0.0+ #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.7.5-20140531_083030-gandalf 04/01/2014
> task: ffff880079112740 ti: ffff88007afe4000 task.ti: ffff88007afe4000
> RIP: 0010:[<ffffffff8100fe5a>] [<ffffffff8100fe5a>] xstateregs_get+0x7a/0x120
> RSP: 0018:ffff88007afe7dc8 EFLAGS: 00010202
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: 0000000000000007 RSI: 0000000000000000 RDI: ffff88007a0063c0
> RBP: ffff88007afe7e08 R08: 00000000001da820 R09: ffff88007a006080
> R10: ffffffff8100fde0 R11: 0000000000000001 R12: 0000000000000240
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> FS: 00007ffff7fdd700(0000) GS:ffff88007c200000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00000000000001d0 CR3: 000000007a80e000 CR4: 00000000000406f0
> Stack:
> ffff88007afe7dd8 00007fffffffe160 ffff880079112740 0000000000004204
> ffff8800791113a0 ffff88007afe7e48 0000000000000202 00007fffffffe3a0
> ffff88007afe7e38 ffffffff81061505 ffff8800791113a0 00007fffffffe3a0
> Call Trace:
> ptrace_regset
> ptrace_request
> ? wait_task_inactive
> ? preempt_count_sub
> arch_ptrace
> ? ptrace_get_task_struct
> SyS_ptrace
> system_call_fastpath
> [ 137.340152] Code: 41 89 c7 74 1c 44 89 f8 48 8b 5d d8 4c 8b 65 e0 4c 8b 6d
> e8 4c 8b 75 f0 4c 8b 7d f8 c9 c3 0f 1f 00 48 8b 15 c9 3d d2 00 45 85 e4 <49>
> 89 96 d0 01 00 00 48 8b 15 c0 3d d2 00 49 89 96 d8 01 00 00
> RIP [<ffffffff8100fe5a>] xstateregs_get+0x7a/0x120
> RSP <ffff88007afe7dc8>
> CR2: 00000000000001d0
> ---[ end trace a1613adc60469c76 ]---
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: Oleg Nesterov <[email protected]>
> Cc: Rik van Riel <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Tavis Ormandy <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> arch/x86/kernel/i387.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
> index 367f39d35e9c..009183276bb7 100644
> --- a/arch/x86/kernel/i387.c
> +++ b/arch/x86/kernel/i387.c
> @@ -341,7 +341,7 @@ int xstateregs_get(struct task_struct *target, const
> struct user_regset *regset,
> unsigned int pos, unsigned int count,
> void *kbuf, void __user *ubuf)
> {
> - struct xsave_struct *xsave = &target->thread.fpu.state->xsave;
> + struct xsave_struct *xsave;
> int ret;
>
> if (!cpu_has_xsave)
> @@ -351,6 +351,8 @@ int xstateregs_get(struct task_struct *target, const
> struct user_regset *regset,
> if (ret)
> return ret;
>
> + xsave = &target->thread.fpu.state->xsave;
> +
> /*
> * Copy the 48bytes defined by the software first into the xstate
> * memory layout in the thread struct, so that we can copy the entire
> @@ -369,7 +371,7 @@ int xstateregs_set(struct task_struct *target, const
> struct user_regset *regset,
> unsigned int pos, unsigned int count,
> const void *kbuf, const void __user *ubuf)
> {
> - struct xsave_struct *xsave = &target->thread.fpu.state->xsave;
> + struct xsave_struct *xsave;
> int ret;
>
> if (!cpu_has_xsave)
> @@ -379,6 +381,8 @@ int xstateregs_set(struct task_struct *target, const
> struct user_regset *regset,
> if (ret)
> return ret;
>
> + xsave = &target->thread.fpu.state->xsave;
> +
> ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
> /*
> * mxcsr reserved bits must be masked to zero for security reasons.
> --
> 2.3.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
