ZBC/ZAC report zones command may return less bytes than requested if the number of matching zones for the report request is small. However, unlike read or write commands, the remainder of incomplete report zones commands cannot be automatically requested by the block layer: the start sector of the next report cannot be known, and the report reply may not be 512B aligned for SAS drives (a report zone reply size is always a multiple of 64B). The regular request completion code executing bio_advance() and restart of the command remainder part currently causes invalid zone descriptor data to be reported to the caller if the report zone size is smaller than 512B (a case that can happen easily for a report of the last zones of a SAS drive for example).
Since blkdev_report_zones() handles report zone command processing in a loop until completion (no more zones are being reported), we can safely avoid that the block layer performs an incorrect bio_advance() call and restart of the remainder of incomplete report zone BIOs. To do so, always indicate a full completion of REQ_OP_ZONE_REPORT by setting good_bytes to the request buffer size and by setting the command resid to 0. This does not affect the post processing of the report zone reply done by sd_zbc_complete() since the reply header indicates the number of zones reported.
Fixes: 89d947561077 ("sd: Implement support for ZBC devices") Cc: stable@vger.kernel.org # 4.19 Cc: stable@vger.kernel.org # 4.14 Signed-off-by: Masato Suzuki masato.suzuki@wdc.com --- drivers/scsi/sd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 2955b856e9ec..e8c2afbb82e9 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1981,9 +1981,13 @@ static int sd_done(struct scsi_cmnd *SCpnt) } break; case REQ_OP_ZONE_REPORT: + /* To avoid that the block layer performs an incorrect + * bio_advance() call and restart of the remainder of + * incomplete report zone BIOs, always indicate a full + * completion of REQ_OP_ZONE_REPORT. + */ if (!result) { - good_bytes = scsi_bufflen(SCpnt) - - scsi_get_resid(SCpnt); + good_bytes = scsi_bufflen(SCpnt); scsi_set_resid(SCpnt, 0); } else { good_bytes = 0;
On 2020/01/27 14:07, Masato Suzuki wrote:
ZBC/ZAC report zones command may return less bytes than requested if the number of matching zones for the report request is small. However, unlike read or write commands, the remainder of incomplete report zones commands cannot be automatically requested by the block layer: the start sector of the next report cannot be known, and the report reply may not be 512B aligned for SAS drives (a report zone reply size is always a multiple of 64B). The regular request completion code executing bio_advance() and restart of the command remainder part currently causes invalid zone descriptor data to be reported to the caller if the report zone size is smaller than 512B (a case that can happen easily for a report of the last zones of a SAS drive for example).
Since blkdev_report_zones() handles report zone command processing in a loop until completion (no more zones are being reported), we can safely avoid that the block layer performs an incorrect bio_advance() call and restart of the remainder of incomplete report zone BIOs. To do so, always indicate a full completion of REQ_OP_ZONE_REPORT by setting good_bytes to the request buffer size and by setting the command resid to 0. This does not affect the post processing of the report zone reply done by sd_zbc_complete() since the reply header indicates the number of zones reported.
Fixes: 89d947561077 ("sd: Implement support for ZBC devices") Cc: stable@vger.kernel.org # 4.19 Cc: stable@vger.kernel.org # 4.14 Signed-off-by: Masato Suzuki masato.suzuki@wdc.com
This bug exists since the beginning of SMR support in 4.10, until report zones was changed to a device file method in kernel 4.20. It fell through the cracks the entire time and was caught only recently with improvements to our test suite.
Reviewed-by: Damien Le Moal damien.lemoal@wdc.com
drivers/scsi/sd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 2955b856e9ec..e8c2afbb82e9 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1981,9 +1981,13 @@ static int sd_done(struct scsi_cmnd *SCpnt) } break; case REQ_OP_ZONE_REPORT:
/* To avoid that the block layer performs an incorrect* bio_advance() call and restart of the remainder of* incomplete report zone BIOs, always indicate a full* completion of REQ_OP_ZONE_REPORT.*/
good_bytes = scsi_bufflen(SCpnt)- scsi_get_resid(SCpnt);
good_bytes = scsi_bufflen(SCpnt); scsi_set_resid(SCpnt, 0);
Damien,
Fixes: 89d947561077 ("sd: Implement support for ZBC devices") Cc: stable@vger.kernel.org # 4.19 Cc: stable@vger.kernel.org # 4.14 Signed-off-by: Masato Suzuki masato.suzuki@wdc.com
This bug exists since the beginning of SMR support in 4.10, until report zones was changed to a device file method in kernel 4.20. It fell through the cracks the entire time and was caught only recently with improvements to our test suite.
Looks good to me. Obviously only applies to stable since, as you point out, this code has been substantially reworked.
Acked-by: Martin K. Petersen martin.petersen@oracle.com
On Mon, Jan 27, 2020 at 11:01:50PM -0500, Martin K. Petersen wrote:
Damien,
Fixes: 89d947561077 ("sd: Implement support for ZBC devices") Cc: stable@vger.kernel.org # 4.19 Cc: stable@vger.kernel.org # 4.14 Signed-off-by: Masato Suzuki masato.suzuki@wdc.com
This bug exists since the beginning of SMR support in 4.10, until report zones was changed to a device file method in kernel 4.20. It fell through the cracks the entire time and was caught only recently with improvements to our test suite.
Looks good to me. Obviously only applies to stable since, as you point out, this code has been substantially reworked.
Acked-by: Martin K. Petersen martin.petersen@oracle.com
Thanks for the review, now queued up.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Damien Le Moal -
Greg Kroah-Hartman -
Martin K. Petersen -
Masato Suzuki