Re: Possible race in copy of fpu->state in copy_process against the exeve'ing parent?

From: Jianyu Zhan
Date: Wed Apr 13 2016 - 03:24:22 EST


On Wed, Apr 13, 2016 at 2:09 PM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> So I'm not sure I understand the suggested race. Separate tasks have separate
> fpu->state states, so a parallel execve() and clone() has no effect on each other.
> There's no FPU state sharing.


Hi, Ingo, thans for reply.

Let me try describe the situation clearly :


>From the panic stack trace, we can infer the call path before panic:


sys_clone
do_fork
copy_process
dup_task_struct(current)
prepare_to_copy(current)
unlazy_fpu(current)
__save_init_fpu(current)
fpu_save_init(current)
fpu_xsave(&current->thread.fpu) <---- PANIC


In this case , &thread.fpu.state is NULL, so it caused a write to
NULL address fault,
which of course is invalid and resulted in a panic.

After reviewing the code, I found only one place in kernel(v3.2.33) that
could make the fpu.state NULL is
from this call path:

sys_execve
do_execve
do_execve_common
search_binary_handler
load_elf_binary
start_thread
start_thread_common
free_thread_xstate(current)
fpu_free(&current->thread.fpu)
fpu->state = NULL


And I also learned that after the first time fpu is used, init_fpu will be
called to allocate a new fpu->state.


So I suspect if this is a problem : we call sys_clone right after
sys_execve, but right before init_fpu()
is called for the first time to allocate a struct fpu for current,
so a NULL fpu->state is seen.


And commit 304bceda6a18(" x86, fpu: use non-lazy fpu restore for
processors supporting xsave") seems
unintendedly fix this ?


Regards,
Jianyu Zhan