diff mbox series

[v2] scsi: sd: block: Fix regressions in read-only block device handling

Message ID 20190213025717.20057-1-martin.petersen@oracle.com (mailing list archive)
State New, archived
Headers show
Series [v2] scsi: sd: block: Fix regressions in read-only block device handling | expand

Commit Message

Martin K. Petersen Feb. 13, 2019, 2:57 a.m. UTC
Some devices come online in write protected state and switch to
read-write once they are ready to process I/O requests. These devices
broke with commit 20bd1d026aac ("scsi: sd: Keep disk read-only when
re-reading partition") because we had no way to distinguish between a
user decision to set a block_device read-only and the actual hardware
device being write-protected.

Because partitions are dropped and recreated on revalidate we are
unable to persist any user-provided policy in hd_struct. Introduce a
bitmap in struct gendisk to track the user configuration. This bitmap
is updated when BLKROSET is called on a given disk or partition.

A helper function, get_user_ro(), is provided to determine whether the
ioctl has forced read-only state for a given block device. This helper
is used by set_disk_ro() and add_partition() to ensure that both
existing and newly created partitions will get the correct state.

 - If BLKROSET sets a whole disk device read-only, all partitions will
   now end up in a read-only state.

 - If BLKROSET sets a given partition read-only, that partition will
   remain read-only post revalidate.

 - Otherwise both the whole disk device and any partitions will
   reflect the write protect state of the underlying device.

Cc: Jeremy Cline <jeremy@jcline.org>
Cc: Oleksii Kurochko <olkuroch@cisco.com>
Cc: stable@vger.kernel.org # v4.16+
Reported-by: Oleksii Kurochko <olkuroch@cisco.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201221
Fixes: 20bd1d026aac ("scsi: sd: Keep disk read-only when re-reading partition")
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

---

v2:
	- Track user read-only state in a bitmap

	- Work around the regression that caused us to drop user
	  preferences on revalidate
---
 block/genhd.c             | 22 +++++++++++++++++-----
 block/ioctl.c             |  4 ++++
 block/partition-generic.c |  2 +-
 drivers/scsi/sd.c         |  4 +---
 include/linux/genhd.h     |  2 ++
 5 files changed, 25 insertions(+), 9 deletions(-)

Comments

Hannes Reinecke Feb. 13, 2019, 7:13 a.m. UTC | #1
On 2/13/19 3:57 AM, Martin K. Petersen wrote:
> Some devices come online in write protected state and switch to
> read-write once they are ready to process I/O requests. These devices
> broke with commit 20bd1d026aac ("scsi: sd: Keep disk read-only when
> re-reading partition") because we had no way to distinguish between a
> user decision to set a block_device read-only and the actual hardware
> device being write-protected.
> 
> Because partitions are dropped and recreated on revalidate we are
> unable to persist any user-provided policy in hd_struct. Introduce a
> bitmap in struct gendisk to track the user configuration. This bitmap
> is updated when BLKROSET is called on a given disk or partition.
> 
> A helper function, get_user_ro(), is provided to determine whether the
> ioctl has forced read-only state for a given block device. This helper
> is used by set_disk_ro() and add_partition() to ensure that both
> existing and newly created partitions will get the correct state.
> 
>   - If BLKROSET sets a whole disk device read-only, all partitions will
>     now end up in a read-only state.
> 
>   - If BLKROSET sets a given partition read-only, that partition will
>     remain read-only post revalidate.
> 
>   - Otherwise both the whole disk device and any partitions will
>     reflect the write protect state of the underlying device.
> 
> Cc: Jeremy Cline <jeremy@jcline.org>
> Cc: Oleksii Kurochko <olkuroch@cisco.com>
> Cc: stable@vger.kernel.org # v4.16+
> Reported-by: Oleksii Kurochko <olkuroch@cisco.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201221
> Fixes: 20bd1d026aac ("scsi: sd: Keep disk read-only when re-reading partition")
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> ---
> 
> v2:
> 	- Track user read-only state in a bitmap
> 
> 	- Work around the regression that caused us to drop user
> 	  preferences on revalidate
> ---
>   block/genhd.c             | 22 +++++++++++++++++-----
>   block/ioctl.c             |  4 ++++
>   block/partition-generic.c |  2 +-
>   drivers/scsi/sd.c         |  4 +---
>   include/linux/genhd.h     |  2 ++
>   5 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 1dd8fd6613b8..34667eb1d3cc 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -1544,19 +1544,31 @@ void set_device_ro(struct block_device *bdev, int flag)
>   
>   EXPORT_SYMBOL(set_device_ro);
>   
> +bool get_user_ro(struct gendisk *disk, unsigned int partno)
> +{
> +	/* Is the user read-only bit set for the whole disk device? */
> +	if (test_bit(0, disk->user_ro_bitmap))
> +		return true;
> +
> +	/* Is the user read-only bit set for this particular partition? */
> +	if (test_bit(partno, disk->user_ro_bitmap))
> +		return true;
> +
> +	return false;
> +}
> +EXPORT_SYMBOL(get_user_ro);
> +
>   void set_disk_ro(struct gendisk *disk, int flag)
>   {
>   	struct disk_part_iter piter;
>   	struct hd_struct *part;
>   
> -	if (disk->part0.policy != flag) {
> +	if (disk->part0.policy != flag)
>   		set_disk_ro_uevent(disk, flag);
> -		disk->part0.policy = flag;
> -	}
>   
> -	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;
> +		part->policy = get_user_ro(disk, part->partno) ?: flag;
>   	disk_part_iter_exit(&piter);
>   }
>   
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 4825c78a6baa..41206df89485 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -451,6 +451,10 @@ static int blkdev_roset(struct block_device *bdev, fmode_t mode,
>   		return ret;
>   	if (get_user(n, (int __user *)arg))
>   		return -EFAULT;
> +	if (n)
> +		set_bit(bdev->bd_partno, bdev->bd_disk->user_ro_bitmap);
> +	else
> +		clear_bit(bdev->bd_partno, bdev->bd_disk->user_ro_bitmap);
>   	set_device_ro(bdev, n);
>   	return 0;
>   }
> diff --git a/block/partition-generic.c b/block/partition-generic.c
> index 8e596a8dff32..c6a3c21c2496 100644
> --- a/block/partition-generic.c
> +++ b/block/partition-generic.c
> @@ -338,7 +338,7 @@ struct hd_struct *add_partition(struct gendisk *disk, int partno,
>   		queue_limit_discard_alignment(&disk->queue->limits, start);
>   	p->nr_sects = len;
>   	p->partno = partno;
> -	p->policy = get_disk_ro(disk);
> +	p->policy = get_user_ro(disk, partno) ?: get_disk_ro(disk);
>   
>   	if (info) {
>   		struct partition_meta_info *pinfo = alloc_part_info(disk);
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 67cc439b86e4..5dfe37b08d3b 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2591,10 +2591,8 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
>   	int res;
>   	struct scsi_device *sdp = sdkp->device;
>   	struct scsi_mode_data data;
> -	int disk_ro = get_disk_ro(sdkp->disk);
>   	int old_wp = sdkp->write_prot;
>   
> -	set_disk_ro(sdkp->disk, 0);
>   	if (sdp->skip_ms_page_3f) {
>   		sd_first_printk(KERN_NOTICE, sdkp, "Assuming Write Enabled\n");
>   		return;
> @@ -2632,7 +2630,7 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
>   			  "Test WP failed, assume Write Enabled\n");
>   	} else {
>   		sdkp->write_prot = ((data.device_specific & 0x80) != 0);
> -		set_disk_ro(sdkp->disk, sdkp->write_prot || disk_ro);
> +		set_disk_ro(sdkp->disk, sdkp->write_prot);
>   		if (sdkp->first_scan || old_wp != sdkp->write_prot) {
>   			sd_printk(KERN_NOTICE, sdkp, "Write Protect is %s\n",
>   				  sdkp->write_prot ? "on" : "off");
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index 06c0fd594097..9645c2604465 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -194,6 +194,7 @@ struct gendisk {
>   	 */
>   	struct disk_part_tbl __rcu *part_tbl;
>   	struct hd_struct part0;
> +	DECLARE_BITMAP(user_ro_bitmap, DISK_MAX_PARTS);
>   
>   	const struct block_device_operations *fops;
>   	struct request_queue *queue;
> @@ -433,6 +434,7 @@ extern struct block_device *bdget_disk(struct gendisk *disk, int partno);
>   
>   extern void set_device_ro(struct block_device *bdev, int flag);
>   extern void set_disk_ro(struct gendisk *disk, int flag);
> +extern bool get_user_ro(struct gendisk *disk, unsigned int partno);
>   
>   static inline int get_disk_ro(struct gendisk *disk)
>   {
> 
Hmm. Can't say I like it. But as we're dropping partitions on revalidate 
I guess we'll have to do it that way. Oh well.

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
Martin K. Petersen Feb. 16, 2019, 3:02 a.m. UTC | #2
Christoph? Jens?

> Some devices come online in write protected state and switch to
> read-write once they are ready to process I/O requests. These devices
> broke with commit 20bd1d026aac ("scsi: sd: Keep disk read-only when
> re-reading partition") because we had no way to distinguish between a
> user decision to set a block_device read-only and the actual hardware
> device being write-protected.
>
> Because partitions are dropped and recreated on revalidate we are
> unable to persist any user-provided policy in hd_struct. Introduce a
> bitmap in struct gendisk to track the user configuration. This bitmap
> is updated when BLKROSET is called on a given disk or partition.
>
> A helper function, get_user_ro(), is provided to determine whether the
> ioctl has forced read-only state for a given block device. This helper
> is used by set_disk_ro() and add_partition() to ensure that both
> existing and newly created partitions will get the correct state.
>
>  - If BLKROSET sets a whole disk device read-only, all partitions will
>    now end up in a read-only state.
>
>  - If BLKROSET sets a given partition read-only, that partition will
>    remain read-only post revalidate.
>
>  - Otherwise both the whole disk device and any partitions will
>    reflect the write protect state of the underlying device.
>
> Cc: Jeremy Cline <jeremy@jcline.org>
> Cc: Oleksii Kurochko <olkuroch@cisco.com>
> Cc: stable@vger.kernel.org # v4.16+
> Reported-by: Oleksii Kurochko <olkuroch@cisco.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201221
> Fixes: 20bd1d026aac ("scsi: sd: Keep disk read-only when re-reading partition")
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>
> ---
>
> v2:
> 	- Track user read-only state in a bitmap
>
> 	- Work around the regression that caused us to drop user
> 	  preferences on revalidate
> ---
>  block/genhd.c             | 22 +++++++++++++++++-----
>  block/ioctl.c             |  4 ++++
>  block/partition-generic.c |  2 +-
>  drivers/scsi/sd.c         |  4 +---
>  include/linux/genhd.h     |  2 ++
>  5 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/block/genhd.c b/block/genhd.c
> index 1dd8fd6613b8..34667eb1d3cc 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -1544,19 +1544,31 @@ void set_device_ro(struct block_device *bdev, int flag)
>  
>  EXPORT_SYMBOL(set_device_ro);
>  
> +bool get_user_ro(struct gendisk *disk, unsigned int partno)
> +{
> +	/* Is the user read-only bit set for the whole disk device? */
> +	if (test_bit(0, disk->user_ro_bitmap))
> +		return true;
> +
> +	/* Is the user read-only bit set for this particular partition? */
> +	if (test_bit(partno, disk->user_ro_bitmap))
> +		return true;
> +
> +	return false;
> +}
> +EXPORT_SYMBOL(get_user_ro);
> +
>  void set_disk_ro(struct gendisk *disk, int flag)
>  {
>  	struct disk_part_iter piter;
>  	struct hd_struct *part;
>  
> -	if (disk->part0.policy != flag) {
> +	if (disk->part0.policy != flag)
>  		set_disk_ro_uevent(disk, flag);
> -		disk->part0.policy = flag;
> -	}
>  
> -	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;
> +		part->policy = get_user_ro(disk, part->partno) ?: flag;
>  	disk_part_iter_exit(&piter);
>  }
>  
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 4825c78a6baa..41206df89485 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -451,6 +451,10 @@ static int blkdev_roset(struct block_device *bdev, fmode_t mode,
>  		return ret;
>  	if (get_user(n, (int __user *)arg))
>  		return -EFAULT;
> +	if (n)
> +		set_bit(bdev->bd_partno, bdev->bd_disk->user_ro_bitmap);
> +	else
> +		clear_bit(bdev->bd_partno, bdev->bd_disk->user_ro_bitmap);
>  	set_device_ro(bdev, n);
>  	return 0;
>  }
> diff --git a/block/partition-generic.c b/block/partition-generic.c
> index 8e596a8dff32..c6a3c21c2496 100644
> --- a/block/partition-generic.c
> +++ b/block/partition-generic.c
> @@ -338,7 +338,7 @@ struct hd_struct *add_partition(struct gendisk *disk, int partno,
>  		queue_limit_discard_alignment(&disk->queue->limits, start);
>  	p->nr_sects = len;
>  	p->partno = partno;
> -	p->policy = get_disk_ro(disk);
> +	p->policy = get_user_ro(disk, partno) ?: get_disk_ro(disk);
>  
>  	if (info) {
>  		struct partition_meta_info *pinfo = alloc_part_info(disk);
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 67cc439b86e4..5dfe37b08d3b 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2591,10 +2591,8 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
>  	int res;
>  	struct scsi_device *sdp = sdkp->device;
>  	struct scsi_mode_data data;
> -	int disk_ro = get_disk_ro(sdkp->disk);
>  	int old_wp = sdkp->write_prot;
>  
> -	set_disk_ro(sdkp->disk, 0);
>  	if (sdp->skip_ms_page_3f) {
>  		sd_first_printk(KERN_NOTICE, sdkp, "Assuming Write Enabled\n");
>  		return;
> @@ -2632,7 +2630,7 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
>  			  "Test WP failed, assume Write Enabled\n");
>  	} else {
>  		sdkp->write_prot = ((data.device_specific & 0x80) != 0);
> -		set_disk_ro(sdkp->disk, sdkp->write_prot || disk_ro);
> +		set_disk_ro(sdkp->disk, sdkp->write_prot);
>  		if (sdkp->first_scan || old_wp != sdkp->write_prot) {
>  			sd_printk(KERN_NOTICE, sdkp, "Write Protect is %s\n",
>  				  sdkp->write_prot ? "on" : "off");
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index 06c0fd594097..9645c2604465 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -194,6 +194,7 @@ struct gendisk {
>  	 */
>  	struct disk_part_tbl __rcu *part_tbl;
>  	struct hd_struct part0;
> +	DECLARE_BITMAP(user_ro_bitmap, DISK_MAX_PARTS);
>  
>  	const struct block_device_operations *fops;
>  	struct request_queue *queue;
> @@ -433,6 +434,7 @@ extern struct block_device *bdget_disk(struct gendisk *disk, int partno);
>  
>  extern void set_device_ro(struct block_device *bdev, int flag);
>  extern void set_disk_ro(struct gendisk *disk, int flag);
> +extern bool get_user_ro(struct gendisk *disk, unsigned int partno);
>  
>  static inline int get_disk_ro(struct gendisk *disk)
>  {
Ming Lei Feb. 19, 2019, 1:36 a.m. UTC | #3
On Wed, Feb 13, 2019 at 5:01 PM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
> Some devices come online in write protected state and switch to
> read-write once they are ready to process I/O requests. These devices
> broke with commit 20bd1d026aac ("scsi: sd: Keep disk read-only when
> re-reading partition") because we had no way to distinguish between a
> user decision to set a block_device read-only and the actual hardware
> device being write-protected.
>
> Because partitions are dropped and recreated on revalidate we are
> unable to persist any user-provided policy in hd_struct. Introduce a
> bitmap in struct gendisk to track the user configuration. This bitmap
> is updated when BLKROSET is called on a given disk or partition.
>
> A helper function, get_user_ro(), is provided to determine whether the
> ioctl has forced read-only state for a given block device. This helper
> is used by set_disk_ro() and add_partition() to ensure that both
> existing and newly created partitions will get the correct state.

Hi Martin & Oleksii,

From the Bugzilla, looks it is only reported on the "Kingston DT Ultimate G3"
USB flash drive.

If it is true, this particular issue might be addressed simply by applying one
quirk on this drive, such as, by adding one delay before calling
sd_spinup_disk()
in the 1st sd_revalidate_disk() to wait until it is ready to process
I/O requests.

Otherwise if there are many such devices, I think your approach is good.

Thanks,
Ming Lei
Martin K. Petersen Feb. 19, 2019, 11:26 p.m. UTC | #4
Hi Ming,

> From the Bugzilla, looks it is only reported on the "Kingston DT
> Ultimate G3" USB flash drive.

There are several bug reports as a result of the change. It's not at all
uncommon for a device to switch at runtime. Before Jeremy's change we
just didn't see it because the device setting always took precedence and
voided any user setting.
Christoph Hellwig Feb. 22, 2019, 2:29 p.m. UTC | #5
On Tue, Feb 12, 2019 at 09:57:17PM -0500, Martin K. Petersen wrote:
> Some devices come online in write protected state and switch to
> read-write once they are ready to process I/O requests. These devices
> broke with commit 20bd1d026aac ("scsi: sd: Keep disk read-only when
> re-reading partition") because we had no way to distinguish between a
> user decision to set a block_device read-only and the actual hardware
> device being write-protected.
> 
> Because partitions are dropped and recreated on revalidate we are
> unable to persist any user-provided policy in hd_struct. Introduce a
> bitmap in struct gendisk to track the user configuration. This bitmap
> is updated when BLKROSET is called on a given disk or partition.
> 
> A helper function, get_user_ro(), is provided to determine whether the
> ioctl has forced read-only state for a given block device. This helper
> is used by set_disk_ro() and add_partition() to ensure that both
> existing and newly created partitions will get the correct state.
> 
>  - If BLKROSET sets a whole disk device read-only, all partitions will
>    now end up in a read-only state.
> 
>  - If BLKROSET sets a given partition read-only, that partition will
>    remain read-only post revalidate.
> 
>  - Otherwise both the whole disk device and any partitions will
>    reflect the write protect state of the underlying device.
> 
> Cc: Jeremy Cline <jeremy@jcline.org>
> Cc: Oleksii Kurochko <olkuroch@cisco.com>
> Cc: stable@vger.kernel.org # v4.16+
> Reported-by: Oleksii Kurochko <olkuroch@cisco.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201221
> Fixes: 20bd1d026aac ("scsi: sd: Keep disk read-only when re-reading partition")
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> ---
> 
> v2:
> 	- Track user read-only state in a bitmap
> 
> 	- Work around the regression that caused us to drop user
> 	  preferences on revalidate
> ---
>  block/genhd.c             | 22 +++++++++++++++++-----
>  block/ioctl.c             |  4 ++++
>  block/partition-generic.c |  2 +-
>  drivers/scsi/sd.c         |  4 +---
>  include/linux/genhd.h     |  2 ++
>  5 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 1dd8fd6613b8..34667eb1d3cc 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -1544,19 +1544,31 @@ void set_device_ro(struct block_device *bdev, int flag)
>  
>  EXPORT_SYMBOL(set_device_ro);
>  
> +bool get_user_ro(struct gendisk *disk, unsigned int partno)
> +{
> +	/* Is the user read-only bit set for the whole disk device? */
> +	if (test_bit(0, disk->user_ro_bitmap))
> +		return true;
> +
> +	/* Is the user read-only bit set for this particular partition? */
> +	if (test_bit(partno, disk->user_ro_bitmap))
> +		return true;
> +
> +	return false;
> +}
> +EXPORT_SYMBOL(get_user_ro);

No need to export this function.

> +	p->policy = get_user_ro(disk, partno) ?: get_disk_ro(disk);

Can we avoid the obsfucating non-standard (GNU extension) use of ?: here?
Just use a local variable and a good old if.
diff mbox series

Patch

diff --git a/block/genhd.c b/block/genhd.c
index 1dd8fd6613b8..34667eb1d3cc 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1544,19 +1544,31 @@  void set_device_ro(struct block_device *bdev, int flag)
 
 EXPORT_SYMBOL(set_device_ro);
 
+bool get_user_ro(struct gendisk *disk, unsigned int partno)
+{
+	/* Is the user read-only bit set for the whole disk device? */
+	if (test_bit(0, disk->user_ro_bitmap))
+		return true;
+
+	/* Is the user read-only bit set for this particular partition? */
+	if (test_bit(partno, disk->user_ro_bitmap))
+		return true;
+
+	return false;
+}
+EXPORT_SYMBOL(get_user_ro);
+
 void set_disk_ro(struct gendisk *disk, int flag)
 {
 	struct disk_part_iter piter;
 	struct hd_struct *part;
 
-	if (disk->part0.policy != flag) {
+	if (disk->part0.policy != flag)
 		set_disk_ro_uevent(disk, flag);
-		disk->part0.policy = flag;
-	}
 
-	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;
+		part->policy = get_user_ro(disk, part->partno) ?: flag;
 	disk_part_iter_exit(&piter);
 }
 
diff --git a/block/ioctl.c b/block/ioctl.c
index 4825c78a6baa..41206df89485 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -451,6 +451,10 @@  static int blkdev_roset(struct block_device *bdev, fmode_t mode,
 		return ret;
 	if (get_user(n, (int __user *)arg))
 		return -EFAULT;
+	if (n)
+		set_bit(bdev->bd_partno, bdev->bd_disk->user_ro_bitmap);
+	else
+		clear_bit(bdev->bd_partno, bdev->bd_disk->user_ro_bitmap);
 	set_device_ro(bdev, n);
 	return 0;
 }
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 8e596a8dff32..c6a3c21c2496 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -338,7 +338,7 @@  struct hd_struct *add_partition(struct gendisk *disk, int partno,
 		queue_limit_discard_alignment(&disk->queue->limits, start);
 	p->nr_sects = len;
 	p->partno = partno;
-	p->policy = get_disk_ro(disk);
+	p->policy = get_user_ro(disk, partno) ?: get_disk_ro(disk);
 
 	if (info) {
 		struct partition_meta_info *pinfo = alloc_part_info(disk);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 67cc439b86e4..5dfe37b08d3b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2591,10 +2591,8 @@  sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
 	int res;
 	struct scsi_device *sdp = sdkp->device;
 	struct scsi_mode_data data;
-	int disk_ro = get_disk_ro(sdkp->disk);
 	int old_wp = sdkp->write_prot;
 
-	set_disk_ro(sdkp->disk, 0);
 	if (sdp->skip_ms_page_3f) {
 		sd_first_printk(KERN_NOTICE, sdkp, "Assuming Write Enabled\n");
 		return;
@@ -2632,7 +2630,7 @@  sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
 			  "Test WP failed, assume Write Enabled\n");
 	} else {
 		sdkp->write_prot = ((data.device_specific & 0x80) != 0);
-		set_disk_ro(sdkp->disk, sdkp->write_prot || disk_ro);
+		set_disk_ro(sdkp->disk, sdkp->write_prot);
 		if (sdkp->first_scan || old_wp != sdkp->write_prot) {
 			sd_printk(KERN_NOTICE, sdkp, "Write Protect is %s\n",
 				  sdkp->write_prot ? "on" : "off");
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 06c0fd594097..9645c2604465 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -194,6 +194,7 @@  struct gendisk {
 	 */
 	struct disk_part_tbl __rcu *part_tbl;
 	struct hd_struct part0;
+	DECLARE_BITMAP(user_ro_bitmap, DISK_MAX_PARTS);
 
 	const struct block_device_operations *fops;
 	struct request_queue *queue;
@@ -433,6 +434,7 @@  extern struct block_device *bdget_disk(struct gendisk *disk, int partno);
 
 extern void set_device_ro(struct block_device *bdev, int flag);
 extern void set_disk_ro(struct gendisk *disk, int flag);
+extern bool get_user_ro(struct gendisk *disk, unsigned int partno);
 
 static inline int get_disk_ro(struct gendisk *disk)
 {