Messages in this thread | | | Date | Thu, 5 Mar 2015 21:11:01 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs |
| |
* Oleg Nesterov <oleg@redhat.com> wrote:
> math_state_restore() assumes it is called with irqs disabled, but > this is not true if the caller is __restore_xstate_sig(). > > This means that if ia32_fxstate == T and __copy_from_user() fails > __restore_xstate_sig() returns with irqs disabled too. This trgiggers > > BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:41 > [<ffffffff81381499>] dump_stack+0x59/0xa0 > [<ffffffff8106bd05>] ___might_sleep+0x105/0x110 > [<ffffffff8138786d>] ? _raw_spin_unlock_irqrestore+0x3d/0x70 > [<ffffffff8106bd8d>] __might_sleep+0x7d/0xb0 > [<ffffffff81385426>] down_read+0x26/0xa0 > [<ffffffff8138788a>] ? _raw_spin_unlock_irqrestore+0x5a/0x70 > [<ffffffff81136038>] print_vma_addr+0x58/0x130 > [<ffffffff8100239e>] signal_fault+0xbe/0xf0 > [<ffffffff810419aa>] sys32_rt_sigreturn+0xba/0xd0 > > Change math_state_restore() to check irqs_disabled(). > > Note: this is the minimal fix for -stable, it is horrible but simple. > We need to rewrite math_state_restore(), init_fpu(), and cleanup their > users. > > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > Cc: <stable@vger.kernel.org> > --- > arch/x86/kernel/traps.c | 9 +++++++-- > 1 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > index 51c4658..7310e0e 100644 > --- a/arch/x86/kernel/traps.c > +++ b/arch/x86/kernel/traps.c > @@ -774,7 +774,10 @@ void math_state_restore(void) > struct task_struct *tsk = current; > > if (!tsk_used_math(tsk)) { > - local_irq_enable(); > + bool disabled = irqs_disabled(); > + > + if (disabled) > + local_irq_enable(); > /* > * does a slab alloc which can sleep > */ > @@ -785,7 +788,9 @@ void math_state_restore(void) > do_group_exit(SIGKILL); > return; > } > - local_irq_disable(); > + > + if (disabled) > + local_irq_disable(); > }
Yuck!
Is there a fundamental reason why we cannot simply enable irqs and leave them enabled? Math state restore is not atomic and cannot really be atomic.
[ A potential worry would be kernel code using vector instructions in irqs-off regions - but that's totally broken anyway so not a big worry IMO, we might even want to warn about it. ]
But I might be missing something?
Thanks,
Ingo
| |