From 12b69878fc7b4b92b1bbd3959f2c3d4c717881fb Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 30 Aug 2022 08:38:27 +0200
Subject: [PATCH] usb/msd: add usb_msd_fatal_error() and fix guest-triggerable
 assert

Add handler for fatal errors.  Moves device into error state where it
stops responding until the guest resets it.

Guest can send illegal requests where scsi command and usb packet
transfer directions are inconsistent.  Use the new usb_msd_fatal_error()
function instead of assert() in that case.

Reported-by: Qiang Liu <cyruscyliu@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Tested-by: Qiang Liu <cyruscyliu@gmail.com>
Message-Id: <20220830063827.813053-3-kraxel@redhat.com>
---
 hw/usb/dev-storage.c | 30 +++++++++++++++++++++++++++++-
 hw/usb/trace-events  |  1 +
 include/hw/usb/msd.h |  1 +
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 140ef2aeaa..e3bcffb3e0 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -191,6 +191,23 @@ static void usb_msd_packet_complete(MSDState *s)
     usb_packet_complete(&s->dev, p);
 }
 
+static void usb_msd_fatal_error(MSDState *s)
+{
+    trace_usb_msd_fatal_error();
+
+    if (s->packet) {
+        s->packet->status = USB_RET_STALL;
+        usb_msd_packet_complete(s);
+    }
+
+    /*
+     * Guest messed up up device state with illegal requests.  Go
+     * ignore any requests until the guests resets the device (and
+     * brings it into a known state that way).
+     */
+    s->needs_reset = true;
+}
+
 static void usb_msd_copy_data(MSDState *s, USBPacket *p)
 {
     uint32_t len;
@@ -227,7 +244,11 @@ void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->packet;
 
-    assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
+    if ((s->mode == USB_MSDM_DATAOUT) != (req->cmd.mode == SCSI_XFER_TO_DEV)) {
+        usb_msd_fatal_error(s);
+        return;
+    }
+
     s->scsi_len = len;
     s->scsi_off = 0;
     if (p) {
@@ -317,6 +338,8 @@ void usb_msd_handle_reset(USBDevice *dev)
 
     memset(&s->csw, 0, sizeof(s->csw));
     s->mode = USB_MSDM_CBW;
+
+    s->needs_reset = false;
 }
 
 static void usb_msd_handle_control(USBDevice *dev, USBPacket *p,
@@ -382,6 +405,11 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
     SCSIDevice *scsi_dev;
     uint32_t len;
 
+    if (s->needs_reset) {
+        p->status = USB_RET_STALL;
+        return;
+    }
+
     switch (p->pid) {
     case USB_TOKEN_OUT:
         if (devep != 2)
diff --git a/hw/usb/trace-events b/hw/usb/trace-events
index 914ca71668..b65269892c 100644
--- a/hw/usb/trace-events
+++ b/hw/usb/trace-events
@@ -263,6 +263,7 @@ usb_msd_packet_complete(void) ""
 usb_msd_cmd_submit(unsigned lun, unsigned tag, unsigned flags, unsigned len, unsigned data_len) "lun %u, tag 0x%x, flags 0x%08x, len %d, data-len %d"
 usb_msd_cmd_complete(unsigned status, unsigned tag) "status %d, tag 0x%x"
 usb_msd_cmd_cancel(unsigned tag) "tag 0x%x"
+usb_msd_fatal_error(void) ""
 
 # dev-uas.c
 usb_uas_reset(int addr) "dev %d"
diff --git a/include/hw/usb/msd.h b/include/hw/usb/msd.h
index 54e9f38bda..f9fd862b52 100644
--- a/include/hw/usb/msd.h
+++ b/include/hw/usb/msd.h
@@ -40,6 +40,7 @@ struct MSDState {
     bool removable;
     bool commandlog;
     SCSIDevice *scsi_dev;
+    bool needs_reset;
 };
 
 typedef struct MSDState MSDState;