linux-kernel - Re: [PATCH v2 0/2] pstore,efi_pstore: Avoid deadlock in non-blocking paths

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121217214506.GF60946@redhat.com>
Date:	Mon, 17 Dec 2012 16:45:06 -0500
From:	Don Zickus <dzickus@...hat.com>
To:	Seiji Aguchi <seiji.aguchi@....com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Luck, Tony (tony.luck@...el.com)" <tony.luck@...el.com>,
	"ccross@...roid.com" <ccross@...roid.com>,
	"keescook@...omium.org" <keescook@...omium.org>,
	"cbouatmailru@...il.com" <cbouatmailru@...il.com>,
	Satoru Moriya <satoru.moriya@....com>,
	"dle-develop@...ts.sourceforge.net" 
	<dle-develop@...ts.sourceforge.net>
Subject: Re: [PATCH v2 0/2] pstore,efi_pstore: Avoid deadlock in non-blocking
 paths

On Mon, Dec 17, 2012 at 08:56:27PM +0000, Seiji Aguchi wrote:
> Changelog v1 -> v2
>  - Erase a logic checking the number of online cpus.
>  - Create a patchset to fix deadlocking issue in both pstore filesystem and
>    efi_pstore driver.
>    - Introduce a function, is_non_blocking_path(), to check if pstore 
>      is in panic and emergency-restart paths (PATCH 1/2)
>    - Avoid efi_pstore_driver is blocked in non-blocking paths
>      such as nmi, panic and emergency-restart paths (PATCH 2/2)

This patch set seems cleaner.  Though it seems most of patch2 should have
been in patch1 (except for the efi part).

The only nitpick I have is the name 'is_non_blocking_path'.  I don't know
why, but the name doesn't seem exactly right to me.  I was thinking
something like 'pstore_cannot_block_path' or something.  But it is just a
nitpick, so it doesn't matter either way to me.

Cheers,
Don

> 
> [Issue]
> 
> There are some paths which shouldn't be blocked in kernel, like NMI, panic case after stopping cpus, 
> emergency-restart.
> 
> On the other hand, current pstore avoids blocking in a NMI path but it may be blocked in other paths.
> Also, an efi_pstore driver may be blocked in all of those paths because it simply takes a spin lock 
> at writing time.
> 
> If they are blocked in those paths, the system will hang up and it has a big impact for users.
> 
> Here is an example scenario which pstore is blocked in panic path.
> 
>  - cpuA grabs psinfo->buf_lock
>  - cpuB panics and calls smp_send_stop
>  - smp_send_stop sends IRQ to cpuA
>    after 1 second, cpuB gives up on cpuA and sends an NMI instead
>  - cpuA is now in an NMI handler while still holding buf_lock.
>    And then, cpuB is deadlocked by taking efi_pstore->lock again.
> 
> This case may happen if a firmware has a bug and cpuA is stuck in it.
> 
> [Solution]
> 
> This patchset avoids that pstore and efi_pstore driver are blocked in the non-blocking paths 
> like NMI, panic, and emrgency-restart by introducing a function checking if they are in those paths.
> Please see each patch for detailed explanations.
> 
> Seiji Aguchi (2):
>   [PATCH v2 1/2] pstore: Avoid deadlock in panic and emergency-restart path
>   [PATCH v2 2/2] efi_pstore: Avoid deadlock in non-blocking paths
> 
>  drivers/firmware/efivars.c |   11 ++++++++++-
>  fs/pstore/platform.c       |   34 ++++++++++++++++++++++++++++------
>  include/linux/pstore.h     |    6 ++++++
>  3 files changed, 44 insertions(+), 7 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/