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: [V4 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
Date: Fri, 25 Sep 2015 20:28:09 +0900	[thread overview]
Message-ID: <20150925112809.4258.39950.stgit@softrs> (raw)
In-Reply-To: <20150925112803.4258.94241.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 panic_cpu.

V4:
- Use new __crash_kexec(), no exclusion check version of crash_kexec(),
  instead of checking if panic_cpu is the current cpu or not

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>
Cc: Michal Hocko <mhocko@kernel.org>
---
 include/linux/kexec.h |    1 +
 kernel/kexec_core.c   |   26 +++++++++++++++++++++++++-
 kernel/panic.c        |    4 ++--
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index d140b1e..f0cd2fa 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -237,6 +237,7 @@ extern int kexec_purgatory_get_set_symbol(struct kimage *image,
 					  unsigned int size, bool get_value);
 extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 					     const char *name);
+extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
 void crash_save_cpu(struct pt_regs *regs, int cpu);
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 201b453..4edb20a 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -853,7 +853,8 @@ int kimage_load_segment(struct kimage *image,
 struct kimage *kexec_crash_image;
 int kexec_load_disabled;
 
-void crash_kexec(struct pt_regs *regs)
+/* No panic_cpu check version of crash_kexec */
+void __crash_kexec(struct pt_regs *regs)
 {
 	/* Take the kexec_mutex here to prevent sys_kexec_load
 	 * running on one cpu from replacing the crash kernel
@@ -876,6 +877,29 @@ void crash_kexec(struct pt_regs *regs)
 	}
 }
 
+void crash_kexec(struct pt_regs *regs)
+{
+	int old_cpu, this_cpu;
+
+	/*
+	 * Only one CPU is allowed to execute the crash_kexec() code as with
+	 * panic().  Otherwise parallel calls of panic() and crash_kexec()
+	 * may stop each other.  To exclude them, we use panic_cpu here too.
+	 */
+	this_cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panic_cpu, -1, this_cpu);
+	if (old_cpu == -1) {
+		/* This is the 1st CPU which comes here, so go ahead. */
+		__crash_kexec(regs);
+
+		/*
+		 * Reset panic_cpu to allow another panic()/crash_kexec()
+		 * call.
+		 */
+		atomic_xchg(&panic_cpu, -1);
+	}
+}
+
 size_t crash_get_memory_size(void)
 {
 	size_t size = 0;
diff --git a/kernel/panic.c b/kernel/panic.c
index cddbfe0..994be45 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -137,7 +137,7 @@ void panic(const char *fmt, ...)
 	 * the "crash_kexec_post_notifiers" option to the kernel.
 	 */
 	if (!crash_kexec_post_notifiers)
-		crash_kexec(NULL);
+		__crash_kexec(NULL);
 
 	/*
 	 * Note smp_send_stop is the usual smp shutdown function, which
@@ -162,7 +162,7 @@ void panic(const char *fmt, ...)
 	 * more unstable, it can increase risks of the kdump failure too.
 	 */
 	if (crash_kexec_post_notifiers)
-		crash_kexec(NULL);
+		__crash_kexec(NULL);
 
 	bust_spinlocks(0);
 



_______________________________________________
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: [V4 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
Date: Fri, 25 Sep 2015 20:28:09 +0900	[thread overview]
Message-ID: <20150925112809.4258.39950.stgit@softrs> (raw)
In-Reply-To: <20150925112803.4258.94241.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 panic_cpu.

V4:
- Use new __crash_kexec(), no exclusion check version of crash_kexec(),
  instead of checking if panic_cpu is the current cpu or not

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>
Cc: Michal Hocko <mhocko@kernel.org>
---
 include/linux/kexec.h |    1 +
 kernel/kexec_core.c   |   26 +++++++++++++++++++++++++-
 kernel/panic.c        |    4 ++--
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index d140b1e..f0cd2fa 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -237,6 +237,7 @@ extern int kexec_purgatory_get_set_symbol(struct kimage *image,
 					  unsigned int size, bool get_value);
 extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 					     const char *name);
+extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
 void crash_save_cpu(struct pt_regs *regs, int cpu);
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 201b453..4edb20a 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -853,7 +853,8 @@ int kimage_load_segment(struct kimage *image,
 struct kimage *kexec_crash_image;
 int kexec_load_disabled;
 
-void crash_kexec(struct pt_regs *regs)
+/* No panic_cpu check version of crash_kexec */
+void __crash_kexec(struct pt_regs *regs)
 {
 	/* Take the kexec_mutex here to prevent sys_kexec_load
 	 * running on one cpu from replacing the crash kernel
@@ -876,6 +877,29 @@ void crash_kexec(struct pt_regs *regs)
 	}
 }
 
+void crash_kexec(struct pt_regs *regs)
+{
+	int old_cpu, this_cpu;
+
+	/*
+	 * Only one CPU is allowed to execute the crash_kexec() code as with
+	 * panic().  Otherwise parallel calls of panic() and crash_kexec()
+	 * may stop each other.  To exclude them, we use panic_cpu here too.
+	 */
+	this_cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panic_cpu, -1, this_cpu);
+	if (old_cpu == -1) {
+		/* This is the 1st CPU which comes here, so go ahead. */
+		__crash_kexec(regs);
+
+		/*
+		 * Reset panic_cpu to allow another panic()/crash_kexec()
+		 * call.
+		 */
+		atomic_xchg(&panic_cpu, -1);
+	}
+}
+
 size_t crash_get_memory_size(void)
 {
 	size_t size = 0;
diff --git a/kernel/panic.c b/kernel/panic.c
index cddbfe0..994be45 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -137,7 +137,7 @@ void panic(const char *fmt, ...)
 	 * the "crash_kexec_post_notifiers" option to the kernel.
 	 */
 	if (!crash_kexec_post_notifiers)
-		crash_kexec(NULL);
+		__crash_kexec(NULL);
 
 	/*
 	 * Note smp_send_stop is the usual smp shutdown function, which
@@ -162,7 +162,7 @@ void panic(const char *fmt, ...)
 	 * more unstable, it can increase risks of the kdump failure too.
 	 */
 	if (crash_kexec_post_notifiers)
-		crash_kexec(NULL);
+		__crash_kexec(NULL);
 
 	bust_spinlocks(0);
 



  parent reply	other threads:[~2015-09-25 12:03 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-25 11:28 [V4 PATCH 0/4] Fix race issues among panic, NMI and crash_kexec Hidehiro Kawai
2015-09-25 11:28 ` Hidehiro Kawai
2015-09-25 11:28 ` [V4 PATCH 1/4] panic/x86: Fix re-entrance problem due to panic on NMI Hidehiro Kawai
2015-09-25 11:28   ` Hidehiro Kawai
2015-09-25 12:13   ` 河合英宏 / KAWAI,HIDEHIRO
2015-09-25 12:13     ` 河合英宏 / KAWAI,HIDEHIRO
2015-09-30 11:26     ` Peter Zijlstra
2015-09-30 11:26       ` Peter Zijlstra
2015-10-01  1:02       ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-01  1:02         ` 河合英宏 / KAWAI,HIDEHIRO
2015-09-25 11:28 ` [V4 PATCH 2/4] panic/x86: Allow cpus to save registers even if they are looping in NMI context Hidehiro Kawai
2015-09-25 11:28   ` Hidehiro Kawai
2015-09-30 11:50   ` Peter Zijlstra
2015-09-30 11:50     ` Peter Zijlstra
2015-10-01  1:43     ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-01  1:43       ` 河合英宏 / KAWAI,HIDEHIRO
2015-09-25 11:28 ` Hidehiro Kawai [this message]
2015-09-25 11:28   ` [V4 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly Hidehiro Kawai
2015-09-28  3:53   ` kbuild test robot
2015-09-28  3:53     ` kbuild test robot
2015-09-28  7:08     ` 河合英宏 / KAWAI,HIDEHIRO
2015-09-28  7:08       ` 河合英宏 / KAWAI,HIDEHIRO
2015-09-30 11:53       ` Peter Zijlstra
2015-09-30 11:53         ` Peter Zijlstra
2015-10-01  2:04         ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-01  2:04           ` 河合英宏 / KAWAI,HIDEHIRO
2015-09-28  4:02   ` kbuild test robot
2015-09-28  4:02     ` kbuild test robot
2015-09-28  4:46     ` 河合英宏 / KAWAI,HIDEHIRO
2015-09-28  4:46       ` 河合英宏 / KAWAI,HIDEHIRO
2015-09-25 11:28 ` [V4 PATCH 4/4] x86/apic: Introduce noextnmi boot option Hidehiro Kawai
2015-09-25 11:28   ` Hidehiro Kawai
2015-09-30 11:55   ` Peter Zijlstra
2015-09-30 11:55     ` Peter Zijlstra
2015-10-01  2:33     ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-01  2:33       ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-01  6:27       ` Peter Zijlstra
2015-10-01  6:27         ` Peter Zijlstra
2015-10-01  7:01         ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-01  7:01           ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-01  8:43           ` Borislav Petkov
2015-10-01  8:43             ` Borislav Petkov
2015-10-01 10:24             ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-01 10:24               ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-01 11:01               ` Borislav Petkov
2015-10-01 11:01                 ` Borislav Petkov
2015-10-02  0:58                 ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-02  0:58                   ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-02  7:47                   ` Borislav Petkov
2015-10-02  7:47                     ` Borislav Petkov
2015-10-05  2:03                     ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-05  2:03                       ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-05  8:27                       ` Borislav Petkov
2015-10-05  8:27                         ` Borislav Petkov
2015-10-05  9:21                         ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-05  9:21                           ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-05 10:14                           ` Borislav Petkov
2015-10-05 10:14                             ` Borislav Petkov
2015-10-13 11:55                             ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-13 11:55                               ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-13 20:19                         ` Thomas Gleixner
2015-10-14 13:54                           ` Ingo Molnar
2015-10-14 13:54                             ` Ingo Molnar
2015-10-16  1:58                             ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-16  1:58                               ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-13 20:22   ` Thomas Gleixner
2015-10-13 20:22     ` Thomas Gleixner
2015-10-14  3:39     ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-14  3:39       ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-14  7:25       ` Thomas Gleixner
2015-10-16  2:02         ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-16  2:02           ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-27  8:46   ` Baoquan He
2015-10-27  8:46     ` Baoquan He
2015-10-27  9:01     ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-27  9:01       ` 河合英宏 / KAWAI,HIDEHIRO
2015-10-27  9:06       ` 'Baoquan He'
2015-10-27  9:06         ` 'Baoquan He'

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=20150925112809.4258.39950.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.