On Wed, May 10, 2023 at 09:42:23AM +0200, Loic Poulain wrote:
> User should not be able to write block device if it is read-only at
> block level (e.g force_ro attribute). This is ensured in the regular
> fops write operation (blkdev_write_iter) but not when writing via
> user mapping (mmap), allowing user to actually write a read-only
> block device via a PROT_WRITE mapping.
>
> Example: This can lead to integrity issue of eMMC boot partition
> (e.g mmcblk0boot0) which is read-only by default.
>
> To fix this issue, simply deny shared writable mapping if the block
> is readonly.
>
> Note: Block remains writable if switch to read-only is performed
> after the initial mapping, but this is expected behavior according
> to commit a32e236eb93e ("Partially revert "block: fail op_is_write()
> requests to read-only partitions"")'.
We should not be able to every mmap something (shared-)writable if the
file descriptor.
And ... we don't failed writable opens for block devices ?!?
Something like this is what we would need, but I really need to look
into the history of this whole thing a bit more:
diff --git a/block/bdev.c b/block/bdev.c
index 1795c7d4b99efa..6dd6045672b8bf 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -724,6 +724,11 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder)
return ERR_PTR(-ENXIO);
disk = bdev->bd_disk;
+ if ((mode & FMODE_WRITE) && bdev_read_only(bdev)) {
+ ret = -EACCES;
+ goto put_blkdev;
+ }
+
if (mode & FMODE_EXCL) {
ret = bd_prepare_to_claim(bdev, holder);
if (ret)
@@ -798,7 +803,6 @@ EXPORT_SYMBOL(blkdev_get_by_dev);
struct block_device *blkdev_get_by_path(const char *path, fmode_t mode,
void *holder)
{
- struct block_device *bdev;
dev_t dev;
int error;
@@ -806,13 +810,7 @@ struct block_device *blkdev_get_by_path(const char *path, fmode_t mode,
if (error)
return ERR_PTR(error);
- bdev = blkdev_get_by_dev(dev, mode, holder);
- if (!IS_ERR(bdev) && (mode & FMODE_WRITE) && bdev_read_only(bdev)) {
- blkdev_put(bdev, mode);
- return ERR_PTR(-EACCES);
- }
-
- return bdev;
+ return blkdev_get_by_dev(dev, mode, holder);
}
EXPORT_SYMBOL(blkdev_get_by_path);
