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

[Jianyu Zhan]

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