Re: [PATCH] mark pstore-blk as broken

From: Al Viro <viro@zeniv.linux.org.uk>
To: Christoph Hellwig <hch@lst.de>
Cc: Kees Cook <keescook@chromium.org>,
	axboe@kernel.dk, anton@enomsg.org, ccross@android.com,
	tony.luck@intel.com, gmpy.liaowx@gmail.com,
	linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mark pstore-blk as broken
Date: Mon, 14 Jun 2021 17:33:35 +0000	[thread overview]
Message-ID: <YMeS780YarlA30E0@zeniv-ca.linux.org.uk> (raw)
In-Reply-To: <20210614070712.GA29881@lst.de>

On Mon, Jun 14, 2021 at 09:07:12AM +0200, Christoph Hellwig wrote:
> On Tue, Jun 08, 2021 at 10:34:29AM -0700, Kees Cook wrote:
> > NAK, please answer my concerns about your patches instead:
> > https://lore.kernel.org/lkml/202012011149.5650B9796@keescook/
> 
> No.  This code pokes into block layer internals with all kinds of issues
> and without any signoff from the relevant parties.  We just can't keep it
> around.

There's a much more interesting question about that code: seeing that
psblk_generic_blk_write() contains this
        /* Console/Ftrace backend may handle buffer until flush dirty zones */
	if (in_interrupt() || irqs_disabled())
		return -EBUSY;
just what are the locking conditions guaranteed to that thing?
Because if it's ever called with one of the destination pages
held locked by the caller, we are fucked.  It won't get caught
by that test.

That really should've been discussed back when the entire thing
got merged; at the absolute least we need the locking environment
documented.