• Mel Gorman's avatar
    x86/fpu: Drop fpregs lock before inheriting FPU permissions · 36b03879
    Mel Gorman authored
    Mike Galbraith reported the following against an old fork of preempt-rt
    but the same issue also applies to the current preempt-rt tree.
    
       BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
       in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1, name: systemd
       preempt_count: 1, expected: 0
       RCU nest depth: 0, expected: 0
       Preemption disabled at:
       fpu_clone
       CPU: 6 PID: 1 Comm: systemd Tainted: G            E       (unreleased)
       Call Trace:
        <TASK>
        dump_stack_lvl
        ? fpu_clone
        __might_resched
        rt_spin_lock
        fpu_clone
        ? copy_thread
        ? copy_process
        ? shmem_alloc_inode
        ? kmem_cache_alloc
        ? kernel_clone
        ? __do_sys_clone
        ? do_syscall_64
        ? __x64_sys_rt_sigprocmask
        ? syscall_exit_to_user_mode
        ? do_syscall_64
        ? syscall_exit_to_user_mode
        ? do_syscall_64
        ? syscall_exit_to_user_mode
        ? do_syscall_64
        ? exc_page_fault
        ? entry_SYSCALL_64_after_hwframe
        </TASK>
    
    Mike says:
    
      The splat comes from fpu_inherit_perms() being called under fpregs_lock(),
      and us reaching the spin_lock_irq() therein due to fpu_state_size_dynamic()
      returning true despite static key __fpu_state_size_dynamic having never
      been enabled.
    
    Mike's assessment looks correct. fpregs_lock on a PREEMPT_RT kernel disables
    preemption so calling spin_lock_irq() in fpu_inherit_perms() is unsafe. This
    problem exists since commit
    
      9e798e9a ("x86/fpu: Prepare fpu_clone() for dynamically enabled features").
    
    Even though the original bug report should not have enabled the paths at
    all, the bug still exists.
    
    fpregs_lock is necessary when editing the FPU registers or a task's FP
    state but it is not necessary for fpu_inherit_perms(). The only write
    of any FP state in fpu_inherit_perms() is for the new child which is
    not running yet and cannot context switch or be borrowed by a kernel
    thread yet. Hence, fpregs_lock is not protecting anything in the new
    child until clone() completes and can be dropped earlier. The siglock
    still needs to be acquired by fpu_inherit_perms() as the read of the
    parent's permissions has to be serialised.
    
      [ bp: Cleanup splat. ]
    
    Fixes: 9e798e9a ("x86/fpu: Prepare fpu_clone() for dynamically enabled features")
    Reported-by: default avatarMike Galbraith <efault@gmx.de>
    Signed-off-by: default avatarMel Gorman <mgorman@techsingularity.net>
    Signed-off-by: default avatarBorislav Petkov <bp@suse.de>
    Reviewed-by: default avatarThomas Gleixner <tglx@linutronix.de>
    Cc: <stable@vger.kernel.org>
    Link: https://lore.kernel.org/r/20221110124400.zgymc2lnwqjukgfh@techsingularity.net
    36b03879
core.c 23.4 KB