From: "Martin K. Petersen" <martin.petersen@xxxxxxxxxx>
Commit 20bd1d026aac ("scsi: sd: Keep disk read-only when re-reading
partition") addressed a long-standing problem with user read-only
policy being overridden as a result of a device-initiated revalidate.
The commit has since been reverted due to a regression that left some
USB devices read-only indefinitely.
To fix the underlying problems with revalidate we need to keep track
of hardware state and user policy separately. Every time the state is
changed, either via a hardware event or the BLKROSET ioctl, the
per-partition read-only state is updated based on the combination of
device state and policy. The resulting active state is stored in a
separate hd_struct flag to avoid introducing additional lookups in the
I/O hot path.
The gendisk has been updated to reflect the current hardware state set
by the device driver. This is done to allow returning the device to
the hardware state once the user clears the BLKROSET flag.
For partitions, the existing hd_struct 'policy' flag is replaced with a
new HD_RO_POLICY flag in a new state variable and changed to only
indicate whether the user has administratively set partition read-only
via the BLKROSET ioctl.
The bdev_read_only helper and its open coded variants also check the
whole device administratively policy as well as the hardware state.
The resulting semantics are as follows:
- If BLKROSET is used to set a whole-disk device read-only, any
partitions will end up in a read-only state until the user
explicitly clears the flag.
- If BLKROSET sets a given partition read-only, that partition will
remain read-only even if the underlying storage stack initiates a
revalidate. However, the BLKRRPART ioctl will cause the partition
table to be dropped and any user policy on partitions will be lost.
- If BLKROSET has not been set, both the whole disk device and any
partitions will reflect the current write-protect state of the
underlying device.
Reported-by: Oleksii Kurochko <olkuroch@xxxxxxxxx>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201221
Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
[hch: rebased. don't mirror the compound read-only flag into a field,
use flag instead of bools]
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
---
block/blk-core.c | 4 ++-
block/blk.h | 3 +++
block/genhd.c | 59 ++++++++++++++++++++++++++++++++---------
block/ioctl.c | 5 +++-
block/partitions/core.c | 7 +++--
include/linux/genhd.h | 10 ++++---
6 files changed, 69 insertions(+), 19 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 2db8bda43b6e6d..b6dea07e08c9c7 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -695,7 +695,9 @@ static inline bool bio_check_ro(struct bio *bio, struct hd_struct *part)
{
const int op = bio_op(bio);
- if (part->policy && op_is_write(op)) {
+ if (op_is_write(op) &&
+ (test_bit(GD_READ_ONLY, &bio->bi_disk->state) ||
+ test_bit(HD_RO_POLICY, &part->state))) {
char b[BDEVNAME_SIZE];
if (op_is_flush(bio->bi_opf) && !bio_sectors(bio))
diff --git a/block/blk.h b/block/blk.h
index dfab98465db9a5..446776853cedd9 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -444,4 +444,7 @@ int bio_add_hw_page(struct request_queue *q, struct bio *bio,
struct page *page, unsigned int len, unsigned int offset,
unsigned int max_sectors, bool *same_page);
+void update_part_ro_state(struct hd_struct *part, bool read_only);
+void update_all_part_ro_state(struct gendisk *disk, bool read_only);
+
#endif /* BLK_INTERNAL_H */
diff --git a/block/genhd.c b/block/genhd.c
index 15b90e56a1a6ea..65f6b744f4ebd5 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1835,31 +1835,66 @@ static void set_disk_ro_uevent(struct gendisk *gd, int ro)
kobject_uevent_env(&disk_to_dev(gd)->kobj, KOBJ_CHANGE, envp);
}
-void set_disk_ro(struct gendisk *disk, int flag)
+void update_part_ro_state(struct hd_struct *part, bool read_only)
{
- struct disk_part_iter piter;
- struct hd_struct *part;
+ if (read_only)
+ set_bit(HD_RO_POLICY, &part->state);
+ else
+ clear_bit(HD_RO_POLICY, &part->state);
+}
- if (disk->part0.policy != flag) {
- set_disk_ro_uevent(disk, flag);
- disk->part0.policy = flag;
- }
+/**
+ * update_all_part_ro_state - iterate over partitions to update read-only state
+ * @disk: The disk device
+ *
+ * This function updates the read-only state for all partitions on a
+ * given disk device. This is required every time a hardware event
+ * signals that the device write-protect state has changed. It is also
+ * necessary when the user sets or clears the read-only flag on the
+ * whole-disk device.
+ */
+void update_all_part_ro_state(struct gendisk *disk, bool read_only)
+{
+ struct disk_part_iter piter;
+ struct hd_struct *part;
- disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY);
+ disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY_PART0);
while ((part = disk_part_iter_next(&piter)))
- part->policy = flag;
- disk_part_iter_exit(&piter);
+ update_part_ro_state(part, read_only);
+ disk_part_iter_exit(&piter);
}
+/**
+ * set_disk_ro - set a gendisk read-only
+ * @disk: The disk device
+ * @state: true or false
+ *
+ * This function is used to indicate whether a given disk device
+ * should have its read-only flag set. set_disk_ro() is typically used
+ * by device drivers to indicate whether the underlying physical
+ * device is write-protected.
+ */
+void set_disk_ro(struct gendisk *disk, bool read_only)
+{
+ if (read_only) {
+ if (test_and_set_bit(GD_READ_ONLY, &disk->state))
+ return;
+ } else {
+ if (!test_and_clear_bit(GD_READ_ONLY, &disk->state))
+ return;
+ }
+ update_all_part_ro_state(disk, read_only);
+ set_disk_ro_uevent(disk, read_only);
+}
EXPORT_SYMBOL(set_disk_ro);
int bdev_read_only(struct block_device *bdev)
{
if (!bdev)
return 0;
- return bdev->bd_part->policy;
+ return test_bit(GD_READ_ONLY, &bdev->bd_disk->state) ||
+ test_bit(HD_RO_POLICY, &bdev->bd_part->state);
}
-
EXPORT_SYMBOL(bdev_read_only);
/*
diff --git a/block/ioctl.c b/block/ioctl.c
index 6b785181344fe1..ce3c9a4dd66d34 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -354,7 +354,10 @@ static int blkdev_roset(struct block_device *bdev, fmode_t mode,
if (ret)
return ret;
}
- bdev->bd_part->policy = n;
+ if (bdev_is_partition(bdev))
+ update_part_ro_state(bdev->bd_part, n);
+ else
+ update_all_part_ro_state(bdev->bd_disk, n);
return 0;
}
diff --git a/block/partitions/core.c b/block/partitions/core.c
index a02e224115943d..aabf9d6d39e59e 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -192,7 +192,9 @@ static ssize_t part_ro_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct hd_struct *p = dev_to_part(dev);
- return sprintf(buf, "%d\n", p->policy ? 1 : 0);
+ return sprintf(buf, "%d\n",
+ test_bit(GD_READ_ONLY, &part_to_disk(p)->state) ||
+ test_bit(HD_RO_POLICY, &p->state));
}
static ssize_t part_alignment_offset_show(struct device *dev,
@@ -414,7 +416,8 @@ static struct hd_struct *add_partition(struct gendisk *disk, int partno,
p->start_sect = start;
p->nr_sects = len;
p->partno = partno;
- p->policy = get_disk_ro(disk);
+ if (get_disk_ro(disk))
+ set_bit(HD_RO_POLICY, &p->state);
if (info) {
struct partition_meta_info *pinfo;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 79e334159778a5..2609bc78ff131b 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -67,7 +67,9 @@ struct hd_struct {
struct device __dev;
struct kobject *holder_dir;
- int policy, partno;
+ int partno;
+ unsigned long state;
+#define HD_RO_POLICY 0
struct partition_meta_info *info;
#ifdef CONFIG_FAIL_MAKE_REQUEST
int make_it_fail;
@@ -193,6 +195,7 @@ struct gendisk {
int flags;
unsigned long state;
#define GD_NEED_PART_SCAN 0
+#define GD_READ_ONLY 1
struct rw_semaphore lookup_sem;
struct kobject *slave_dir;
@@ -304,11 +307,12 @@ extern void del_gendisk(struct gendisk *gp);
extern struct gendisk *get_gendisk(dev_t dev, int *partno);
extern struct block_device *bdget_disk(struct gendisk *disk, int partno);
-extern void set_disk_ro(struct gendisk *disk, int flag);
+void set_disk_ro(struct gendisk *disk, bool read_only);
static inline int get_disk_ro(struct gendisk *disk)
{
- return disk->part0.policy;
+ return test_bit(GD_READ_ONLY, &disk->state) ||
+ test_bit(HD_RO_POLICY, &disk->part0.state);
}
extern void disk_block_events(struct gendisk *disk);
--
2.28.0
