All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@suse.de>
To: Matt Fleming <matt@console-pimps.org>
Cc: linux-kernel@vger.kernel.org, mingo@kernel.org, hpa@zytor.com,
	ricardo.neri-calderon@linux.intel.com, tglx@linutronix.de,
	matt.fleming@intel.com, linux-tip-commits@vger.kernel.org,
	Seiji Aguchi <seiji.aguchi@hds.com>
Subject: Re: [tip:x86/efi] x86/efi: Save and restore FPU context around efi_calls (x86_64)
Date: Fri, 25 Apr 2014 13:40:10 +0200	[thread overview]
Message-ID: <20140425114010.GA20275@nazgul.tnic> (raw)
In-Reply-To: <20140425110936.GB26088@console-pimps.org>

On Fri, Apr 25, 2014 at 12:09:36PM +0100, Matt Fleming wrote:
> Hmm... note that we may call EFI runtime services from interrupt context
> in efi_pstore_write(), so it seems like it would be possible to trigger
> that WARN_ON_ONCE() there.
> 
> Seiji (Cc'd) might have some opinions on this.
> 
> Either way, if someone sends me a patch ontop of this one that swaps the
> __kernel_fpu_begin() for kernel_fpu_begin() I can try them out in my
> lab.

Well, the more I think about it, the more I'm persuaded that
you actually do *really* need that WARN_ON_ONCE check there to
make sure you're not fiddling with the FPU while in an interrupt
context and in an unsafe way (see interrupted_kernel_fpu_idle() and
interrupted_user_mode()). And so you do need the variants without the
"__" which include the check.

Anyway, here it is, do give it a good run:

---
From: Borislav Petkov <bp@suse.de>
Date: Fri, 25 Apr 2014 13:30:21 +0200
Subject: [PATCH] efi: Check for unsafe dealing with FPU state in irq ctxt

efi_call can happen in an irq context (pstore) and there we really need
to make sure we're not scribbling over FPU state while we've interrupted
a thread or kernel mode with a live FPU state. Therefore, use the
kernel_fpu_begin/end() variants which do that check.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/efi.h | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 1eb5f6433ad8..f969ce8bea24 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -67,11 +67,9 @@ extern u64 asmlinkage efi_call(void *fp, ...);
 	efi_status_t __s;						\
 									\
 	efi_sync_low_kernel_mappings();					\
-	preempt_disable();						\
-	__kernel_fpu_begin();						\
+	kernel_fpu_begin();						\
 	__s = efi_call((void *)efi.systab->runtime->f, __VA_ARGS__);	\
-	__kernel_fpu_end();						\
-	preempt_enable();						\
+	kernel_fpu_end();						\
 	__s;								\
 })
 
-- 
1.9.0.258.g00eda23


  reply	other threads:[~2014-04-25 11:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <tip-de05764e0b2a3d9559e099a2e134f8cef4500fdd@git.kernel.org>
2014-04-20  0:28 ` [tip:x86/efi] x86/efi: Save and restore FPU context around efi_calls (x86_64) Borislav Petkov
2014-04-25 11:09   ` Matt Fleming
2014-04-25 11:40     ` Borislav Petkov [this message]
2014-04-29 10:46       ` Matt Fleming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140425114010.GA20275@nazgul.tnic \
    --to=bp@suse.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=matt.fleming@intel.com \
    --cc=matt@console-pimps.org \
    --cc=mingo@kernel.org \
    --cc=ricardo.neri-calderon@linux.intel.com \
    --cc=seiji.aguchi@hds.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.