[Qemu-devel] [PATCH v2 5/5] hw/scsi: Check sense key before READ CAPACITY output snoop

[Dmitry Fomichev <dmitry.fomichev@wdc.com>]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

When READ CAPACITY command completes, scsi_read_complete() function
snoops the command result and updates SCSIDevice members blocksize and
max_lba . However, this update is executed even when READ CAPACITY
command indicates an error in sense data. This causes unexpected
blocksize update with zero value for SCSI devices without
READ CAPACITY(10) command support and eventually results in a divide
by zero. An emulated device by TCMU-runner is an example of a device
that doesn't support READ CAPACITY(10) command.

To avoid the unexpected update, add sense key check in
scsi_read_complete() function. The function already checks the sense
key for VPD Block Limits emulation. Make scsi_parse_sense_buf() call
common for VPD Block Limits emulation and READ CAPACITY snoop. Update
blocksize and max_lba only if READ CAPACITY returns zero sense key.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 hw/scsi/scsi-generic.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index a43efe39ec..e38d3160fa 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -238,6 +238,7 @@ static void scsi_read_complete(void * opaque, int ret)
     SCSIGenericReq *r = (SCSIGenericReq *)opaque;
     SCSIDevice *s = r->req.dev;
     int len;
+    uint8_t sense_key = NO_SENSE;
 
     assert(r->req.aiocb != NULL);
     r->req.aiocb = NULL;
@@ -254,6 +255,12 @@ static void scsi_read_complete(void * opaque, int ret)
 
     r->len = -1;
 
+    if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) {
+        SCSISense sense =
+            scsi_parse_sense_buf(r->req.sense, r->io_header.sb_len_wr);
+        sense_key = sense.key;
+    }
+
     /*
      * Check if this is a VPD Block Limits request that
      * resulted in sense error but would need emulation.
@@ -264,9 +271,7 @@ static void scsi_read_complete(void * opaque, int ret)
         r->req.cmd.buf[0] == INQUIRY &&
         (r->req.cmd.buf[1] & 0x01) &&
         r->req.cmd.buf[2] == 0xb0) {
-        SCSISense sense =
-            scsi_parse_sense_buf(r->req.sense, r->io_header.sb_len_wr);
-        if (sense.key == ILLEGAL_REQUEST) {
+        if (sense_key == ILLEGAL_REQUEST) {
             len = scsi_generic_emulate_block_limits(r, s);
             /*
              * No need to let scsi_read_complete go on and handle an
@@ -281,15 +286,17 @@ static void scsi_read_complete(void * opaque, int ret)
         goto done;
     }
 
-    /* Snoop READ CAPACITY output to set the blocksize.  */
-    if (r->req.cmd.buf[0] == READ_CAPACITY_10 &&
-        (ldl_be_p(&r->buf[0]) != 0xffffffffU || s->max_lba == 0)) {
-        s->blocksize = ldl_be_p(&r->buf[4]);
-        s->max_lba = ldl_be_p(&r->buf[0]) & 0xffffffffULL;
-    } else if (r->req.cmd.buf[0] == SERVICE_ACTION_IN_16 &&
-               (r->req.cmd.buf[1] & 31) == SAI_READ_CAPACITY_16) {
-        s->blocksize = ldl_be_p(&r->buf[8]);
-        s->max_lba = ldq_be_p(&r->buf[0]);
+    /* Snoop READ CAPACITY output to set the blocksize. */
+    if (sense_key == NO_SENSE) {
+        if (r->req.cmd.buf[0] == READ_CAPACITY_10 &&
+            (ldl_be_p(&r->buf[0]) != 0xffffffffU || s->max_lba == 0)) {
+            s->blocksize = ldl_be_p(&r->buf[4]);
+            s->max_lba = ldl_be_p(&r->buf[0]) & 0xffffffffULL;
+        } else if (r->req.cmd.buf[0] == SERVICE_ACTION_IN_16 &&
+                   (r->req.cmd.buf[1] & 31) == SAI_READ_CAPACITY_16) {
+            s->blocksize = ldl_be_p(&r->buf[8]);
+            s->max_lba = ldq_be_p(&r->buf[0]);
+        }
     }
     blk_set_guest_block_size(s->conf.blk, s->blocksize);
 
-- 
2.21.0