[dm-devel] [PATCH 3/3] block: fail writes to read-only devices

[Linus Torvalds]

On Thu, Jun 1, 2023 at 3:28 AM Christoph Hellwig <hch at lst.de> wrote:
>
> Note that the last attempt to do this got reverted by Linus in commit
> a32e236eb93e ("Partially revert "block: fail op_is_write() requests to
> read-only partitions") because device mapper relyied on not enforcing
> the read-only state when used together with older lvm-tools.

I'm certainly happy to try again, but I have my doubts whether this will work.

Note that the problem case is for a device that *used* to be writable,
but became read-only later, and there's an existing writer that needs
that writability.

The lvm fix was not to stop writing, but to simply not mark it
read-only too early.

So I do think that the problem here is purely in the block layer.

The logic wrt "bdev_read_only()" is not necessarily a "right now it's
read-only", but more of a thing that should be checked purely when the
device is opened. Which is pretty much exactly what we do.

So honestly, that whole test for

+       if (op_is_write(bio_op(bio)) && bio_sectors(bio) &&
+           bdev_read_only(bdev)) {

may look "obviously correct", but it's also equally valid to view it
as "obviously garbage", simply because the test is being done at the
wrong point.

The same way you can write to a file that was opened for writing, but
has then become read-only afterwards, writing to a device with a bdev
that was writable when you *started* writing is not at all necessarily
wrong.

So please at least consider the possibility that that test - while it
looks obvious - is simply buggy.

                Linus