linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Dave Chinner <dchinner@redhat.com>,
	Ming Lei <ming.lei@redhat.com>, Theodore Ts'o <tytso@mit.edu>,
	linux-ext4@vger.kernel.org,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	linux-block@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	Eric Sandeen <sandeen@redhat.com>, Christoph Hellwig <hch@lst.de>,
	Zhang Yi <yi.zhang@redhat.com>
Subject: Re: [ext4 io hang] buffered write io hang in balance_dirty_pages
Date: Fri, 28 Apr 2023 15:24:22 +1000	[thread overview]
Message-ID: <20230428052422.GB1969623@dread.disaster.area> (raw)
In-Reply-To: <ZEs1za7Q0U4bY08w@casper.infradead.org>

On Fri, Apr 28, 2023 at 03:56:13AM +0100, Matthew Wilcox wrote:
> On Fri, Apr 28, 2023 at 09:33:20AM +1000, Dave Chinner wrote:
> > The block device needs to be shutting down the filesystem when it
> > has some sort of fatal, unrecoverable error like this (e.g. hot
> > unplug). We have the XFS_IOC_GOINGDOWN ioctl for telling the
> > filesystem it can't function anymore. This ioctl
> > (_IOR('X',125,__u32)) has also been replicated into ext4, f2fs and
> > CIFS and it gets exercised heavily by fstests. Hence this isn't XFS
> > specific functionality, nor is it untested functionality.
> > 
> > The ioctl should be lifted to the VFS as FS_IOC_SHUTDOWN and a
> > super_operations method added to trigger a filesystem shutdown.
> > That way the block device removal code could simply call
> > sb->s_ops->shutdown(sb, REASON) if it exists rather than
> > sync_filesystem(sb) if there's a superblock associated with the
> > block device. Then all these 
> 
> I think this is the wrong approach.  Not that I've had any time to
> work on my alternative approach:
> 
> https://www.infradead.org/~willy/banbury.html

While that looks interesting, I'm going to say straight out that
re-attaching a storage device that was hot-unplugged from under a
running filesystem and then resuming service as if nothing happened
is going to be both a filesystem corruption vector and a major
security hole.

The moment a mounted device is unexpectedly unplugged, it becomes an
untrusted device.  We cannot trust that it's contents when plugged
back in are identical to when it was unplugged.  I can't wait for
syzbot to learn that it can mount a filesystem, hot-unplug the
device, fuzz the image on the device and then plug it back in and
expect the filesystem to handle the in-memory vs on-disk
inconsistencies that result from the fuzzing. it's basically no
different to allowing someone to write to the block device while the
filesystem is mounted. You do that, you get to keep all the broken
bits to yourself.

Even without image tampering considerations, there's no guarantee
that the device caches are flushed properly when someone just pulls
the plug on a storage device. Hence even without tampering, we
cannot trust the image on the device to match what the in-memory
state of the filesystem expects it to be.

So, yeah, I just don't see this ever being something we'd support
with filesystems like XFS - there's just too much risk associated
with untrusted devices...

-Dave.
-- 
Dave Chinner
david@fromorbit.com


  reply	other threads:[~2023-04-28  5:24 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-27  2:20 Ming Lei
2023-04-27  3:58 ` Matthew Wilcox
2023-04-27  4:50   ` Ming Lei
2023-04-27  6:36     ` Baokun Li
2023-04-27  7:33       ` Baokun Li
2023-04-27 10:01       ` Ming Lei
2023-04-27 11:19         ` Baokun Li
2023-04-27 11:27           ` Ming Lei
2023-04-28  1:41             ` Ming Lei
2023-04-28  3:47               ` Baokun Li
2023-04-28  5:47                 ` Theodore Ts'o
2023-04-29  3:16                   ` Ming Lei
2023-04-29  4:40                     ` Christoph Hellwig
2023-04-29  5:10                       ` Ming Lei
2023-05-01  4:47                         ` Christoph Hellwig
2023-05-02  0:57                           ` Ming Lei
2023-05-02  1:35                             ` Dave Chinner
2023-05-02 15:35                               ` Darrick J. Wong
2023-05-02 22:33                                 ` Dave Chinner
2023-05-02 23:27                                   ` Darrick J. Wong
2023-04-29  4:56                     ` Theodore Ts'o
2023-05-01  2:06                       ` Dave Chinner
2023-05-04  3:09                   ` Baokun Li
2023-04-27 23:33 ` Dave Chinner
2023-04-28  2:56   ` Matthew Wilcox
2023-04-28  5:24     ` Dave Chinner [this message]
2023-05-04 15:59 ` Keith Busch
2023-05-04 16:21   ` Matthew Wilcox
2023-05-05  2:06   ` Ming Lei

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=20230428052422.GB1969623@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=akpm@linux-foundation.org \
    --cc=dchinner@redhat.com \
    --cc=hch@lst.de \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ming.lei@redhat.com \
    --cc=sandeen@redhat.com \
    --cc=tytso@mit.edu \
    --cc=willy@infradead.org \
    --cc=yi.zhang@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox