From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751320AbcBLBQW (ORCPT <rfc822;w@1wt.eu>);
	Thu, 11 Feb 2016 20:16:22 -0500
Received: from mail-ob0-f180.google.com ([209.85.214.180]:33320 "EHLO
	mail-ob0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750825AbcBLBQU convert rfc822-to-8bit (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 11 Feb 2016 20:16:20 -0500
MIME-Version: 1.0
In-Reply-To: <CALCETrVSChX9BFyLn2mf4q3a0WxyNgFzcb4A4nFfAQnbfO02Pg@mail.gmail.com>
References: <20160211192741.GG5565@pd.tnic> <CALCETrVSChX9BFyLn2mf4q3a0WxyNgFzcb4A4nFfAQnbfO02Pg@mail.gmail.com>
From: Andy Lutomirski <luto@amacapital.net>
Date: Thu, 11 Feb 2016 17:16:00 -0800
Message-ID: <CALCETrVU8RvcDAUPfwoW9FVvgyn3z-5R86+4-mXtubpTd4YiKg@mail.gmail.com>
Subject: Re: WARNING: CPU: 0 PID: 3031 at ./arch/x86/include/asm/fpu/internal.h:530
 fpu__restore+0x90/0x130()
To: Borislav Petkov <bp@alien8.de>
Cc: x86-ml <x86@kernel.org>, lkml <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Feb 11, 2016 at 3:47 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Feb 11, 2016 at 11:27 AM, Borislav Petkov <bp@alien8.de> wrote:
>> Hey Andy,
>>
>> can you make any sense of this:
>>
>> [   90.573923] ------------[ cut here ]------------
>> [   90.574977] WARNING: CPU: 0 PID: 3031 at ./arch/x86/include/asm/fpu/internal.h:530 fpu__restore+0x90/0x130()
>> [   90.576108] Modules linked in: hid_generic usbhid hid snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic iTCO_wdt iTCO_vendor_support arc4 x86_pkg_temp_thermal coretemp kvm_intel kvm irqbypass crc32_pclmul crc32c_intel iwldvm mac80211 aesni_intel xts snd_hda_intel input_leds aes_i586 snd_hda_codec sdhci_pci lrw iwlwifi snd_hwdep gf128mul snd_hda_core ablk_helper cryptd ehci_pci pcspkr serio_raw xhci_pci sdhci snd_pcm sg mmc_core i2c_i801 cfg80211 lpc_ich mfd_core e1000e snd_timer ehci_hcd xhci_hcd thinkpad_acpi nvram wmi snd battery soundcore led_class ac thermal
>> [   90.580570] CPU: 0 PID: 3031 Comm: bash Not tainted 4.5.0-rc3+ #1
>> [   90.581380] Hardware name: LENOVO 2320CTO/2320CTO, BIOS G2ET86WW (2.06 ) 11/13/2012
>> [   90.582325]  00000000 00000286 f158be4c c12cce56 00000000 00000000 f158be80 c10567fb
>> [   90.583179]  c1866c2c 00000000 00000bd7 c1859e8c 00000212 c1025ab0 00000212 c1025ab0
>> [   90.584142]  f2012b00 f2011f00 f2012d80 f158be90 c10568d2 00000009 00000000 f158bea4
>> [   90.585002] Call Trace:
>> [   90.585854]  [<c12cce56>] dump_stack+0x5f/0x89
>> [   90.586703]  [<c10567fb>] warn_slowpath_common+0x8b/0xc0
>> [   90.587559]  [<c1025ab0>] ? fpu__restore+0x90/0x130
>> [   90.588520]  [<c1025ab0>] ? fpu__restore+0x90/0x130
>> [   90.589353]  [<c10568d2>] warn_slowpath_null+0x22/0x30
>> [   90.590175]  [<c1025ab0>] fpu__restore+0x90/0x130
>> [   90.590993]  [<c1027098>] __fpu__restore_sig+0x268/0x4c0
>> [   90.591816]  [<c102751f>] fpu__restore_sig+0x2f/0x50
>> [   90.592636]  [<c101a6c9>] restore_sigcontext+0xe9/0x110
>> [   90.593449]  [<c101af3c>] sys_sigreturn+0x9c/0xb0
>> [   90.594263]  [<c1001bd9>] do_syscall_32_irqs_on+0x59/0x340
>> [   90.595079]  [<c169979d>] entry_INT80_32+0x31/0x31
>> [   90.595922] ---[ end trace be617bef2982f473 ]---
>>
>> This is rc3 + latest tip/master and it happened when I did "make
>> mrproper" in the kernel repo.
>>
>> From a quick stare, it looks to me we're running do_syscall_32_irqs_on()
>> with IRQs on, sys_sigreturn() does current_pt_regs() but
>> __fpu__restore_sig() derefs current again and could be that that second
>> "current" is another current which already has ->fpregs_active set ?
>>
>> FPU + signal handling code in a single backtrace. My favourite!
>>
>> :-\
>
> Ugh.
>
> Can you send all the fpu info that the kernel prints really early when it boots?
>

Are you running 32-bit userspace by any chance?  I'm guessing you're
hitting this in __fpu_restore_sig:

        fpu__drop(fpu);
        if (__copy_from_user(&fpu->state.xsave, buf_fx, state_size) ||
            __copy_from_user(&env, buf, sizeof(env))) {
            fpstate_init(&fpu->state);
            err = -1;
        } else {
            sanitize_restored_xstate(tsk, &env, xfeatures, fx_only);
        }

        fpu->fpstate_active = 1;

<-- preempted right here

        if (use_eager_fpu()) {
            preempt_disable();
            fpu__restore(fpu);
            preempt_enable();
        }

I don't see why this code deserves to work.  If I'm right, it can be
fixed by pulling the preempt_disable out of the if (use_eager_fpu())
to right above the fpstate_active = 1 line.  Don't bother trying to
optimize the !use_eager_fpu() case.

Once someone gets around to eagerly *allocating* the FPU context and
dropping CR0.TS usage entirely, then even that won't be enough unless
we do my suggesting of deferring FPU restore to
prepare_exit_to_usermode.  (Doing that will make all of this much,
much more sane.)


--Andy