[PATCH 0/2] scsi: sd: Fix WRITE SAME(10) data corruption

[PATCH 0/2] scsi: sd: Fix WRITE SAME(10) data corruption

[Chris Down]

With scsi_debug and 512-byte logical blocks, a 1 MiB discard at 2 TiB
succeeds, but it's the first MiB of the disk that gets discarded. This
happens because the starting LBA wraps to zero, because it doesn't fit
in WRITE SAME(10).

Write zeroes can also run out of bits for the block count. With a
0x20000-block WRITE SAME limit, a 64 MiB hole punch sends a count of
zero and leaves the whole range as it was.

Let's use plain WRITE SAME(16) for write zeroes when the LBA or count
won't fit in WRITE SAME(10). For discard, we can just refuse the request
and leave the data alone.

Chris Down (2):
  scsi: sd: Fix data corruption in WRITE SAME(10) write zeroes
  scsi: sd: Fix data corruption in WRITE SAME(10) discards

 drivers/scsi/sd.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)


base-commit: 04feb5fd2c23894ebc2cdaf4a4cc38b8667b17ac
-- 
2.54.0

[PATCH 1/2] scsi: sd: Fix data corruption in WRITE SAME(10) write zeroes

[Chris Down]

In writesame_10_unmap mode, we send write zeroes as WRITE SAME(10).
If the LBA or count doesn't fit, we truncate it and can return
success without zeroing the requested range. If the LBA wraps, we
can also corrupt live data near the start of the disk.

Cap the queue limit when sd selects this mode so large requests can
still unmap in chunks. If the LBA or count still won't fit, fall back
to WRITE SAME(16) with UNMAP clear. All the caller needs is for the
requested blocks to read back as zeroes.

Fixes: e4b878371b85 ("sd: implement unmapping Write Zeroes")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chris Down <chris@chrisdown.name>
---
 drivers/scsi/sd.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index dc45df819c42..a28d11886aca 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1164,7 +1164,10 @@ static blk_status_t sd_setup_write_zeroes_cmnd(struct scsi_cmnd *cmd)
 		case SD_ZERO_WS16_UNMAP:
 			return sd_setup_write_same16_cmnd(cmd, true);
 		case SD_ZERO_WS10_UNMAP:
-			return sd_setup_write_same10_cmnd(cmd, true);
+			if (lba <= 0xffffffff && nr_blocks <= 0xffff)
+				return sd_setup_write_same10_cmnd(cmd, true);
+			/* Unmapping is optional for write zeroes. */
+			break;
 		}
 	}
 
@@ -1221,6 +1224,11 @@ static void sd_config_write_same(struct scsi_disk *sdkp,
 	else
 		sdkp->zeroing_mode = SD_ZERO_WRITE;
 
+	/* Split large requests so they can still unmap. */
+	if (sdkp->zeroing_mode == SD_ZERO_WS10_UNMAP)
+		sdkp->max_ws_blocks = min(sdkp->max_ws_blocks,
+					  (u32)SD_MAX_WS10_BLOCKS);
+
 	if (sdkp->max_ws_blocks &&
 	    sdkp->physical_block_size > logical_block_size) {
 		/*
-- 
2.54.0

[PATCH 2/2] scsi: sd: Fix data corruption in WRITE SAME(10) discards

[Chris Down]

We already cap the discard length in writesame_10 and writesame_zero
mode, but we still truncate the starting LBA to 32 bits. A discard at
or above block 2^32 can corrupt live data near the start of the disk
and still report success.

Discard is advisory, so return BLK_STS_NOTSUPP if the request won't
fit in WRITE SAME(10). Discards that fit can still go through.

Fixes: c98a0eb0e90d ("[SCSI] sd: Logical Block Provisioning update")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chris Down <chris@chrisdown.name>
---
 drivers/scsi/sd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a28d11886aca..9b8530267b4a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1131,6 +1131,9 @@ static blk_status_t sd_setup_write_same10_cmnd(struct scsi_cmnd *cmd,
 	u32 data_len = sdp->sector_size;
 	blk_status_t ret;
 
+	if (lba > 0xffffffff || nr_blocks > 0xffff)
+		return BLK_STS_NOTSUPP;
+
 	if (!sd_set_special_bvec(cmd, data_len))
 		return BLK_STS_RESOURCE;
 
-- 
2.54.0