mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: x86@kernel.org, linux-kernel@vger.kernel.org
Cc: "Brian Gerst" <brgerst@gmail.com>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"Borislav Petkov" <bp@alien8.de>,
	"Frédéric Weisbecker" <fweisbec@gmail.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Andy Lutomirski" <luto@kernel.org>
Subject: [PATCH v3 1/5] x86/entry/64: Fix irqflag tracing wrt context tracking
Date: Thu, 12 Nov 2015 12:59:00 -0800	[thread overview]
Message-ID: <86237e362390dfa6fec12de4d75a238acb0ae787.1447361906.git.luto@kernel.org> (raw)
In-Reply-To: <cover.1447361906.git.luto@kernel.org>
In-Reply-To: <cover.1447361906.git.luto@kernel.org>

Paolo pointed out that enter_from_user_mode could be called while
irqflags were traced as though IRQs were on.

In principle, this could confuse lockdep.  It doesn't cause any
problems that I've seen in any configuration, but if I build with
CONFIG_DEBUG_LOCKDEP=y, enable a nohz_full CPU, and add code like:

	if (irqs_disabled()) {
		spin_lock(&something);
		spin_unlock(&something);
	}

to the top of enter_from_user_mode, then lockdep will complain
without this fix.  It seems that lockdep's irqflags sanity checks
are too weak to detect this bug without forcing the issue.

This patch adds one byte to normal kernels, and it's IMO a bit ugly.
I haven't spotted a better way to do this yet, though.  The issue is
that we can't do TRACE_IRQS_OFF until after SWAPGS (if needed), but
we're also supposed to do it before calling C code.

An alternative approach would be to call trace_hardirqs_off in
enter_from_user_mode.  That would be less code and would not bloat
normal kernels at all, but it would be harder to see how the code
worked.

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/entry_64.S | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 53616ca03244..a55697d19824 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -509,6 +509,17 @@ END(irq_entries_start)
 	 * tracking that we're in kernel mode.
 	 */
 	SWAPGS
+
+	/*
+	 * We need to tell lockdep that IRQs are off.  We can't do this until
+	 * we fix gsbase, and we should do it before enter_from_user_mode
+	 * (which can take locks).  Since TRACE_IRQS_OFF idempotent,
+	 * the simplest way to handle it is to just call it twice if
+	 * we enter from user mode.  There's no reason to optimize this since
+	 * TRACE_IRQS_OFF is a no-op if lockdep is off.
+	 */
+	TRACE_IRQS_OFF
+
 #ifdef CONFIG_CONTEXT_TRACKING
 	call enter_from_user_mode
 #endif
@@ -1049,12 +1060,18 @@ ENTRY(error_entry)
 	SWAPGS
 
 .Lerror_entry_from_usermode_after_swapgs:
+	/*
+	 * We need to tell lockdep that IRQs are off.  We can't do this until
+	 * we fix gsbase, and we should do it before enter_from_user_mode
+	 * (which can take locks).
+	 */
+	TRACE_IRQS_OFF
 #ifdef CONFIG_CONTEXT_TRACKING
 	call enter_from_user_mode
 #endif
+	ret
 
 .Lerror_entry_done:
-
 	TRACE_IRQS_OFF
 	ret
 
-- 
2.5.0


  reply	other threads:[~2015-11-12 20:59 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-12 20:58 [PATCH v3 0/5] x86 entry stuff, maybe for 4.4 Andy Lutomirski
2015-11-12 20:59 ` Andy Lutomirski [this message]
2015-11-24  9:35   ` [tip:x86/asm] x86/entry/64: Fix irqflag tracing wrt context tracking tip-bot for Andy Lutomirski
2015-11-12 20:59 ` [PATCH v3 2/5] context_tracking: Switch to new static_branch API Andy Lutomirski
2015-11-24  9:35   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-11-12 20:59 ` [PATCH v3 3/5] x86/asm: Error out if asm/jump_label.h is included inappropriately Andy Lutomirski
2015-11-13 14:13   ` Thomas Gleixner
2015-11-24  9:35   ` [tip:x86/asm] x86/asm: Error out if asm/ jump_label.h " tip-bot for Andy Lutomirski
2015-11-12 20:59 ` [PATCH v3 4/5] x86/asm: Add asm macros for static keys/jump labels Andy Lutomirski
2015-11-13 14:22   ` Thomas Gleixner
2015-11-24  9:36   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-11-12 20:59 ` [PATCH v3 5/5] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots Andy Lutomirski
2015-11-13 14:23   ` Thomas Gleixner
2015-11-13 15:26   ` Frederic Weisbecker
2015-11-16 19:10     ` Andy Lutomirski
2015-11-16 22:50       ` Frederic Weisbecker
2015-11-16 23:57         ` Andy Lutomirski
2015-11-19  0:57           ` Frederic Weisbecker
2015-11-24  9:36   ` [tip:x86/asm] " tip-bot for Andy Lutomirski

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=86237e362390dfa6fec12de4d75a238acb0ae787.1447361906.git.luto@kernel.org \
    --to=luto@kernel.org \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®