diff mbox series

[2/2] scsi: sd_zbc: prevent zone information memory leak

Message ID 20220530014341.115427-3-damien.lemoal@opensource.wdc.com
State New
Headers show
Series sd_zbc fixes | expand

Commit Message

Damien Le Moal May 30, 2022, 1:43 a.m. UTC
Make sure to always clear a scsi disk zone information, even for regular
disks. This ensures that there is no memory leak, even in the case of a
zoned disk changing type to a regular disk (e.g. with a reformat using
the FORMAT WITH PRESET command or other vendor proprietary command).

This change also makes sure that the sdkp rev_mutex is never used while
not being initialized by gating sd_zbc_clear_zone_info() cleanup code
with a check on the zone_wp_update_buf field which is never NULL when
rev_mutex has been initialized.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/scsi/sd_zbc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Johannes Thumshirn May 30, 2022, 7:48 a.m. UTC | #1
On 30/05/2022 03:43, Damien Le Moal wrote:
> Make sure to always clear a scsi disk zone information, even for regular
> disks. This ensures that there is no memory leak, even in the case of a
> zoned disk changing type to a regular disk (e.g. with a reformat using
> the FORMAT WITH PRESET command or other vendor proprietary command).
> 
> This change also makes sure that the sdkp rev_mutex is never used while
> not being initialized by gating sd_zbc_clear_zone_info() cleanup code
> with a check on the zone_wp_update_buf field which is never NULL when
> rev_mutex has been initialized.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  drivers/scsi/sd_zbc.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index 5b9fad70aa88..6245205b1159 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -788,6 +788,9 @@ static int sd_zbc_init_disk(struct scsi_disk *sdkp)
>  
>  static void sd_zbc_clear_zone_info(struct scsi_disk *sdkp)
>  {
> +	if (!sdkp->zone_wp_update_buf)
> +		return;
> +
>  	/* Serialize against revalidate zones */
>  	mutex_lock(&sdkp->rev_mutex);
>  
> @@ -804,8 +807,7 @@ static void sd_zbc_clear_zone_info(struct scsi_disk *sdkp)
>  
>  void sd_zbc_release_disk(struct scsi_disk *sdkp)
>  {
> -	if (sd_is_zoned(sdkp))
> -		sd_zbc_clear_zone_info(sdkp);
> +	sd_zbc_clear_zone_info(sdkp);
>  }

Now sd_zbc_release_disk() has become a simple rename of sd_zbc_clear_zone_info().
I think it can go and we can use sd_zbc_clear_zone_info() in the callers instead.
diff mbox series

Patch

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 5b9fad70aa88..6245205b1159 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -788,6 +788,9 @@  static int sd_zbc_init_disk(struct scsi_disk *sdkp)
 
 static void sd_zbc_clear_zone_info(struct scsi_disk *sdkp)
 {
+	if (!sdkp->zone_wp_update_buf)
+		return;
+
 	/* Serialize against revalidate zones */
 	mutex_lock(&sdkp->rev_mutex);
 
@@ -804,8 +807,7 @@  static void sd_zbc_clear_zone_info(struct scsi_disk *sdkp)
 
 void sd_zbc_release_disk(struct scsi_disk *sdkp)
 {
-	if (sd_is_zoned(sdkp))
-		sd_zbc_clear_zone_info(sdkp);
+	sd_zbc_clear_zone_info(sdkp);
 }
 
 static void sd_zbc_revalidate_zones_cb(struct gendisk *disk)
@@ -914,12 +916,15 @@  int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
 	u32 zone_blocks = 0;
 	int ret;
 
-	if (!sd_is_zoned(sdkp))
+	if (!sd_is_zoned(sdkp)) {
 		/*
-		 * Device managed or normal SCSI disk,
-		 * no special handling required
+		 * Device managed or normal SCSI disk, no special handling
+		 * required. Nevertheless, clear the disk zone information in
+		 * case the device type changed.
 		 */
+		sd_zbc_clear_zone_info(sdkp);
 		return 0;
+	}
 
 	/* READ16/WRITE16 is mandatory for ZBC disks */
 	sdkp->device->use_16_for_rw = 1;