Re: [PATCH] block: Add config option to not allow writing to mounted devices

From: Christian Brauner <brauner@kernel.org>
To: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@infradead.org>,
	Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org, Dmitry Vyukov <dvyukov@google.com>,
	Ted Tso <tytso@mit.edu>, yebin <yebin@huaweicloud.com>,
	linux-fsdevel@vger.kernel.org, Kees Cook <keescook@google.com>,
	Alexander Popov <alex.popov@linux.com>,
	Eric Biggers <ebiggers@google.com>
Subject: Re: [PATCH] block: Add config option to not allow writing to mounted devices
Date: Wed, 14 Jun 2023 09:35:29 +0200	[thread overview]
Message-ID: <20230614-galopp-aussuchen-1c95be0709f0@brauner> (raw)
In-Reply-To: <20230613205614.atlrwst55bpqjzxf@quack3>

On Tue, Jun 13, 2023 at 10:56:14PM +0200, Jan Kara wrote:
> On Mon 12-06-23 22:10:30, Christoph Hellwig wrote:
> > > +config BLK_DEV_WRITE_HARDENING
> > > +	bool "Do not allow writing to mounted devices"
> > > +	help
> > > +	When a block device is mounted, writing to its buffer cache very likely
> > > +	going to cause filesystem corruption. It is also rather easy to crash
> > > +	the kernel in this way since the filesystem has no practical way of
> > > +	detecting these writes to buffer cache and verifying its metadata
> > > +	integrity. Select this option to disallow writing to mounted devices.
> > > +	This should be mostly fine but some filesystems (e.g. ext4) rely on
> > > +	the ability of filesystem tools to write to mounted filesystems to
> > > +	set e.g. UUID or run fsck on the root filesystem in some setups.
> > 
> > I'm not sure a config option is really the right thing.
> > 
> > I'd much prefer a BLK_OPEN_ flag to prohibit any other writer.
> > Except for etN and maybe fat all file systems can set that
> > unconditionally.  And for those file systems that have historically
> > allowed writes to mounted file systems they can find a local way
> > to decide on when and when not to set it.
> 
> Well, as I've mentioned in the changelog there are old setups (without

Before going into the details: Let's please take syzbot out of the
picture as a justification or required reviewer for this patch. This is
a complete distraction imho. If the patch has the side effect that it
somehow makes for less noisy syzbot reports then so be it but it's
really not why this patch is a good idea.

For userspace this patch is immediately useful and a security mechanism
that everyone familiar with block device/filesystem attack surfaces will
want to make use of. And we should encourage this be the default
whenever possible imho. That's all the justification that we need.

> initrd) that run fsck on root filesystem mounted read-only and fsck
> programs tend to open the device with O_RDWR. These would be broken by this
> change (for the filesystems that would use BLK_OPEN_ flag). Similarly some
> boot loaders can write to first sectors of the root partition while the
> filesystem is mounted. So I don't think controlling the behavior by the
> in-kernel user that is having the bdev exclusively open really works. It
> seems to be more a property of the system setup than a property of the
> in-kernel bdev user. Am I mistaken?
> 
> So I think kconfig option or sysfs tunable (maybe a per-device one?) will
> be more appropriate choice? With default behavior configurable by kernel
> parameter? And once set to write-protect on mount, do we allow flipping it
> back? Both have advantages and disadvantages so the tunable might be
> tri-state in the end (no protection, write-protect but can be turned off,
> write-protect that cannot be turned off)? But maybe I'm overcomplicating
> this so please share your thoughts :)

A simple bool Kconfig overridable by kernel command line option is what
we want. Fundamental security relevant properties such as this should
never be runtime configurable. They should be boot and build time
configurable and then they should be off limits.