All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
To: Jonathan Corbet <corbet@lwn.net>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vivek Goyal <vgoyal@redhat.com>
Cc: linux-doc@vger.kernel.org, x86@kernel.org,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	Michal Hocko <mhocko@kernel.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Subject: [V2 PATCH 2/3] kexec: Fix race between panic() and crash_kexec() called directly
Date: Mon, 27 Jul 2015 10:58:50 +0900	[thread overview]
Message-ID: <20150727015850.4928.15194.stgit@softrs> (raw)
In-Reply-To: <20150727015850.4928.87717.stgit@softrs>

Currently, panic() and crash_kexec() can be called at the same time.
For example (x86 case):

CPU 0:
  oops_end()
    crash_kexec()
      mutex_trylock() // acquired
        nmi_shootdown_cpus() // stop other cpus

CPU 1:
  panic()
    crash_kexec()
      mutex_trylock() // failed to acquire
    smp_send_stop() // stop other cpus
    infinite loop

If CPU 1 calls smp_send_stop() before nmi_shootdown_cpus(), kdump
fails.

In another case:

CPU 0:
  oops_end()
    crash_kexec()
      mutex_trylock() // acquired
        <NMI>
        io_check_error()
          panic()
            crash_kexec()
              mutex_trylock() // failed to acquire
            infinite loop

Clearly, this is an undesirable result.

To fix this problem, this patch changes crash_kexec() to exclude
others by using atomic_t panicking_cpu.

V2:
- Use atomic_cmpxchg() instead of spin_trylock() on panic_lock
  to exclude concurrent accesses
- Don't introduce no-lock version of crash_kexec()

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 kernel/kexec.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index a785c10..ca40a19 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1472,6 +1472,18 @@ void __weak crash_unmap_reserved_pages(void)
 
 void crash_kexec(struct pt_regs *regs)
 {
+	int old_cpu, this_cpu;
+
+	/*
+	 * `old_cpu == -1' means we are the first comer and crash_kexec()
+	 * was called without entering panic().
+	 * `old_cpu == this_cpu' means crash_kexec() was called from panic().
+	 */
+	this_cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panicking_cpu, -1, this_cpu);
+	if (old_cpu != -1 && old_cpu != this_cpu)
+		return;
+
 	/* Take the kexec_mutex here to prevent sys_kexec_load
 	 * running on one cpu from replacing the crash kernel
 	 * we are using after a panic on a different cpu.
@@ -1491,6 +1503,14 @@ void crash_kexec(struct pt_regs *regs)
 		}
 		mutex_unlock(&kexec_mutex);
 	}
+
+	/*
+	 * If we came here from panic(), we have to keep panicking_cpu
+	 * to prevent other cpus from entering panic().  Otherwise,
+	 * resetting it so that other cpus can enter panic()/crash_kexec().
+	 */
+	if (old_cpu == this_cpu)
+		atomic_set(&panicking_cpu, -1);
 }
 
 size_t crash_get_memory_size(void)



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
To: Jonathan Corbet <corbet@lwn.net>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vivek Goyal <vgoyal@redhat.com>
Cc: linux-doc@vger.kernel.org, x86@kernel.org,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	Michal Hocko <mhocko@kernel.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Subject: [V2 PATCH 2/3] kexec: Fix race between panic() and crash_kexec() called directly
Date: Mon, 27 Jul 2015 10:58:50 +0900	[thread overview]
Message-ID: <20150727015850.4928.15194.stgit@softrs> (raw)
In-Reply-To: <20150727015850.4928.87717.stgit@softrs>

Currently, panic() and crash_kexec() can be called at the same time.
For example (x86 case):

CPU 0:
  oops_end()
    crash_kexec()
      mutex_trylock() // acquired
        nmi_shootdown_cpus() // stop other cpus

CPU 1:
  panic()
    crash_kexec()
      mutex_trylock() // failed to acquire
    smp_send_stop() // stop other cpus
    infinite loop

If CPU 1 calls smp_send_stop() before nmi_shootdown_cpus(), kdump
fails.

In another case:

CPU 0:
  oops_end()
    crash_kexec()
      mutex_trylock() // acquired
        <NMI>
        io_check_error()
          panic()
            crash_kexec()
              mutex_trylock() // failed to acquire
            infinite loop

Clearly, this is an undesirable result.

To fix this problem, this patch changes crash_kexec() to exclude
others by using atomic_t panicking_cpu.

V2:
- Use atomic_cmpxchg() instead of spin_trylock() on panic_lock
  to exclude concurrent accesses
- Don't introduce no-lock version of crash_kexec()

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 kernel/kexec.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index a785c10..ca40a19 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1472,6 +1472,18 @@ void __weak crash_unmap_reserved_pages(void)
 
 void crash_kexec(struct pt_regs *regs)
 {
+	int old_cpu, this_cpu;
+
+	/*
+	 * `old_cpu == -1' means we are the first comer and crash_kexec()
+	 * was called without entering panic().
+	 * `old_cpu == this_cpu' means crash_kexec() was called from panic().
+	 */
+	this_cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panicking_cpu, -1, this_cpu);
+	if (old_cpu != -1 && old_cpu != this_cpu)
+		return;
+
 	/* Take the kexec_mutex here to prevent sys_kexec_load
 	 * running on one cpu from replacing the crash kernel
 	 * we are using after a panic on a different cpu.
@@ -1491,6 +1503,14 @@ void crash_kexec(struct pt_regs *regs)
 		}
 		mutex_unlock(&kexec_mutex);
 	}
+
+	/*
+	 * If we came here from panic(), we have to keep panicking_cpu
+	 * to prevent other cpus from entering panic().  Otherwise,
+	 * resetting it so that other cpus can enter panic()/crash_kexec().
+	 */
+	if (old_cpu == this_cpu)
+		atomic_set(&panicking_cpu, -1);
 }
 
 size_t crash_get_memory_size(void)



  reply	other threads:[~2015-07-27  5:11 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-27  1:58 [V2 PATCH 0/3] x86: Fix panic vs. NMI issues Hidehiro Kawai
2015-07-27  1:58 ` Hidehiro Kawai
2015-07-27  1:58 ` Hidehiro Kawai [this message]
2015-07-27  1:58   ` [V2 PATCH 2/3] kexec: Fix race between panic() and crash_kexec() called directly Hidehiro Kawai
2015-07-27 14:55   ` Michal Hocko
2015-07-27 14:55     ` Michal Hocko
2015-07-28  2:15     ` Hidehiro Kawai
2015-07-28  2:15       ` Hidehiro Kawai
2015-07-27  1:58 ` [V2 PATCH 3/3] x86/apic: Introduce noextnmi boot option Hidehiro Kawai
2015-07-27  1:58   ` Hidehiro Kawai
2015-07-27  1:58 ` [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI Hidehiro Kawai
2015-07-27  1:58   ` Hidehiro Kawai
2015-07-27 14:34   ` Michal Hocko
2015-07-27 14:34     ` Michal Hocko
2015-07-28  2:02     ` Hidehiro Kawai
2015-07-28  2:02       ` Hidehiro Kawai
2015-07-28  8:01       ` Michal Hocko
2015-07-28  8:01         ` Michal Hocko
2015-07-29  5:48       ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-29  5:48         ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-29  8:23         ` Michal Hocko
2015-07-29  8:23           ` Michal Hocko
2015-07-29  9:09           ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-29  9:09             ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-29  9:21             ` Michal Hocko
2015-07-29  9:21               ` Michal Hocko
2015-07-30  1:45               ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  1:45                 ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  7:33                 ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  7:33                   ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  7:55                   ` Michal Hocko
2015-07-30  7:55                     ` Michal Hocko
2015-07-30  8:06                     ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  8:06                       ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  7:48                 ` Michal Hocko
2015-07-30  7:48                   ` Michal Hocko
2015-07-30 11:55                   ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30 11:55                     ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30 12:27                     ` Michal Hocko
2015-07-30 12:27                       ` Michal Hocko
2015-07-31 11:23                       ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-31 11:23                         ` 河合英宏 / KAWAI,HIDEHIRO
2015-08-04  8:56                         ` Michal Hocko
2015-08-04  8:56                           ` Michal Hocko
2015-08-04 11:53                           ` 河合英宏 / KAWAI,HIDEHIRO
2015-08-04 11:53                             ` 河合英宏 / KAWAI,HIDEHIRO

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=20150727015850.4928.15194.stgit@softrs \
    --to=hidehiro.kawai.ez@hitachi.com \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=ebiederm@xmission.com \
    --cc=hpa@zytor.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mhocko@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=vgoyal@redhat.com \
    --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 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.