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

Damien Le Moal
2022-06-01T06:25:56Z

Make sure to always free 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).

To do this, rename sd_zbc_clear_zone_info() to sd_zbc_free_zone_info()
and remove sd_zbc_release_disk(). A call to sd_zbc_free_zone_info() is
added to sd_zbc_read_zones() for drives for which sd_is_zoned() returns
false. Furthermore, sd_zbc_free_zone_info() code make s sure that the
sdkp rev_mutex is never used while not being initialized by gating the
cleanup code with a a check on the zone_wp_update_buf field as it is
never NULL when rev_mutex has been initialized.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/sd.c | 2 +-
drivers/scsi/sd.h | 4 ++--
drivers/scsi/sd_zbc.c | 26 +++++++++++++-------------
3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index dabdc0eeb3dc..be812987aa3f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3578,7 +3578,7 @@ static void scsi_disk_release(struct device *dev)
struct scsi_disk *sdkp = to_scsi_disk(dev);

ida_free(&sd_index_ida, sdkp->index);
- sd_zbc_release_disk(sdkp);
+ sd_zbc_free_zone_info(sdkp);
put_device(&sdkp->device->sdev_gendev);
free_opal_dev(sdkp->opal_dev);

diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 2abad54fd23f..5eea762f84d1 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -241,7 +241,7 @@ static inline int sd_is_zoned(struct scsi_disk *sdkp)

#ifdef CONFIG_BLK_DEV_ZONED

-void sd_zbc_release_disk(struct scsi_disk *sdkp);
+void sd_zbc_free_zone_info(struct scsi_disk *sdkp);
int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE]);
int sd_zbc_revalidate_zones(struct scsi_disk *sdkp);
blk_status_t sd_zbc_setup_zone_mgmt_cmnd(struct scsi_cmnd *cmd,
@@ -256,7 +256,7 @@ blk_status_t sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd, sector_t *lba,

#else /* CONFIG_BLK_DEV_ZONED */

-static inline void sd_zbc_release_disk(struct scsi_disk *sdkp) {}
+static inline void sd_zbc_free_zone_info(struct scsi_disk *sdkp) {}

static inline int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
{
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 5b9fad70aa88..6acc4f406eb8 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -786,8 +786,11 @@ static int sd_zbc_init_disk(struct scsi_disk *sdkp)
return 0;
}

-static void sd_zbc_clear_zone_info(struct scsi_disk *sdkp)
+void sd_zbc_free_zone_info(struct scsi_disk *sdkp)
{
+ if (!sdkp->zone_wp_update_buf)
+ return;
+
/* Serialize against revalidate zones */
mutex_lock(&sdkp->rev_mutex);

@@ -802,12 +805,6 @@ static void sd_zbc_clear_zone_info(struct scsi_disk *sdkp)
mutex_unlock(&sdkp->rev_mutex);
}

-void sd_zbc_release_disk(struct scsi_disk *sdkp)
-{
- if (sd_is_zoned(sdkp))
- sd_zbc_clear_zone_info(sdkp);
-}
-
static void sd_zbc_revalidate_zones_cb(struct gendisk *disk)
{
struct scsi_disk *sdkp = scsi_disk(disk);
@@ -914,12 +911,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, free the disk zone information in
+ * case the device type changed.
*/
+ sd_zbc_free_zone_info(sdkp);
return 0;
+ }

/* READ16/WRITE16 is mandatory for ZBC disks */
sdkp->device->use_16_for_rw = 1;
@@ -928,11 +928,11 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
if (!blk_queue_is_zoned(q)) {
/*
* This can happen for a host aware disk with partitions.
- * The block device zone information was already cleared
- * by blk_queue_set_zoned(). Only clear the scsi disk zone
+ * The block device zone model was already cleared by
+ * blk_queue_set_zoned(). Only free the scsi disk zone
* information and exit early.
*/
- sd_zbc_clear_zone_info(sdkp);
+ sd_zbc_free_zone_info(sdkp);
return 0;
}

--
2.36.1