Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1277144

Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly

From Borislav Petkov <bp@alien8.de>
Newsgroups linux.kernel
Subject Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
Date 2015-11-25 11:00 +0100
Message-ID <qyF1M-4qs-7@gated-at.bofh.it> (permalink)
References <qwQum-5rq-19@gated-at.bofh.it> <qwR75-5WC-41@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, Nov 20, 2015 at 06:36:48PM +0900, Hidehiro Kawai wrote:
> 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.
> 
> V5:
> - Add missing dummy __crash_kexec() for !CONFIG_KEXEC_CORE case
> - Replace atomic_xchg() with atomic_set() in crash_kexec() because
>   it is used as a release operation and there is no need of memory
>   barrier effect.  This change also removes an unused value warning
> 
> 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 |    2 ++
>  kernel/kexec_core.c   |   26 +++++++++++++++++++++++++-
>  kernel/panic.c        |    4 ++--
>  3 files changed, 29 insertions(+), 3 deletions(-)

...

> +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.

So can we make __crash_kexec() return error values?

* failed to grab kexec_mutex -> reset panic_cpu

* no kexec_crash_image -> no need to reset it, all future crash_kexec()
calls won't work so no need to run into that path anymore. However, this could
be problematic if we want the other CPUs to panic. Do we care?

* machine_kexec successful -> doesn't matter

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[V5 PATCH 0/4] Fix race issues among panic, NMI and crash_kexec Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-11-20 11:30 +0100
  [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec()  called directly Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-11-20 11:30 +0100
    Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec()  called directly Michal Hocko <mhocko@kernel.org> - 2015-11-24 14:10 +0100
    Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec()  called directly Steven Rostedt <rostedt@goodmis.org> - 2015-11-24 21:40 +0100
      RE: [V5 PATCH 3/4] kexec: Fix race between panic() and  crash_kexec() called directly 河合英宏 / KAWAI,HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2015-11-25 07:30 +0100
    Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec()  called directly Borislav Petkov <bp@alien8.de> - 2015-11-25 11:00 +0100

csiph-web