A command whose CDB transfers data, sent with a PRDT length of 0,
reaches scsi-hd without an SG list. The SCSI layer then calls the
transfer_data callback of the bus, which ufs_scsi_info does not set, and
QEMU dies with SIGSEGV. A guest triggers this with, for example, an
INQUIRY whose PRDT is empty; the new no-prdt qtest crashed QEMU before
this change.
Check the transfer direction that scsi_req_new() parsed, and fail such a
command with OCS INVALID COMMAND TABLE ATTRIBUTES before it reaches
scsi-hd. virtio-scsi likewise fails a command that has no matching
buffer (VIRTIO_SCSI_S_OVERRUN). A request that only reports a pending
unit attention moves no data and goes through, so that the guest still
sees the unit attention.
Handing scsi-hd an empty SG list instead is not enough: READ and WRITE
then complete synchronously in dma_blk_io() and trip the assertion in
scsi_dma_complete().
Fixes: 2a8b36a4967d ("hw/ufs: Support for UFS logical unit")
Cc: [email protected]
Signed-off-by: Daejun Park <[email protected]>
---
hw/ufs/lu.c | 24 ++++++++++++++++++++----
tests/qtest/ufs-test.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/hw/ufs/lu.c b/hw/ufs/lu.c
index 5905c49fac..1853284acf 100644
--- a/hw/ufs/lu.c
+++ b/hw/ufs/lu.c
@@ -382,6 +382,8 @@ static UfsReqResult ufs_emulate_scsi_cmd(UfsLu *lu,
UfsRequest *req)
static UfsReqResult ufs_process_scsi_cmd(UfsLu *lu, UfsRequest *req)
{
uint8_t task_tag = req->req_upiu.header.task_tag;
+ SCSIRequest *scsi_req;
+ uint32_t len;
/*
* Each ufs-lu has its own independent virtual SCSI bus. Therefore, we
can't
@@ -393,12 +395,26 @@ static UfsReqResult ufs_process_scsi_cmd(UfsLu *lu,
UfsRequest *req)
return ufs_emulate_scsi_cmd(lu, req);
}
- SCSIRequest *scsi_req =
- scsi_req_new(lu->scsi_dev, task_tag, lu->lun, req->req_upiu.sc.cdb,
- UFS_CDB_SIZE, req);
+ scsi_req = scsi_req_new(lu->scsi_dev, task_tag, lu->lun,
+ req->req_upiu.sc.cdb, UFS_CDB_SIZE, req);
+
+ /*
+ * A command that moves data needs a PRDT. Without one there is no SG
+ * list, and the SCSI layer would call the transfer_data callback, which
+ * ufs-lu does not set. Fail the request, as virtio-scsi does when the
+ * buffer does not match the command. A request that reports a pending
+ * unit attention instead moves no data; its sense is already set, so let
+ * it through and the guest sees the UA.
+ */
+ if (scsi_req->cmd.mode != SCSI_XFER_NONE && !req->sg &&
+ !scsi_req->sense_len) {
+ scsi_req_unref(scsi_req);
+ return UFS_REQUEST_FAIL;
+ }
+
req->sreq = scsi_req;
- uint32_t len = scsi_req_enqueue(scsi_req);
+ len = scsi_req_enqueue(scsi_req);
if (len) {
scsi_req_continue(scsi_req);
}
diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
index 9a9c5ca834..63f805419e 100644
--- a/tests/qtest/ufs-test.c
+++ b/tests/qtest/ufs-test.c
@@ -770,6 +770,50 @@ static void ufstest_read_write(void *obj, void *data,
QGuestAllocator *alloc)
ufs_exit(ufs, alloc);
}
+static void ufstest_no_prdt(void *obj, void *data, QGuestAllocator *alloc)
+{
+ QUfs *ufs = obj;
+ uint8_t buf[4096];
+ const uint8_t inquiry_cdb[UFS_CDB_SIZE] = {
+ /* INQUIRY, allocation length 36 */
+ INQUIRY, 0x00, 0x00, 0x00, 0x24
+ };
+ const uint8_t write_cdb[UFS_CDB_SIZE] = {
+ /* WRITE(10) to LBA 0, transfer length 1 */
+ WRITE_10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00
+ };
+ enum UtpOcsCodes ocs;
+ UtpUpiuRsp rsp_upiu;
+ const int test_lun = 1;
+
+ ufs_init(ufs, alloc);
+
+ /* A pending unit attention is reported even without a PRDT */
+ ocs = ufs_send_scsi_command(ufs, test_lun, write_cdb, NULL, 0, NULL, 0,
+ &rsp_upiu);
+ g_assert_cmpuint(ocs, ==, UFS_OCS_SUCCESS);
+ g_assert_cmpuint(rsp_upiu.header.scsi_status, ==, CHECK_CONDITION);
+ g_assert_cmpuint(rsp_upiu.sr.sense_data[2] & 0xf, ==, UNIT_ATTENTION);
+
+ /* Commands that move data but come without a PRDT fail */
+ ocs = ufs_send_scsi_command(ufs, test_lun, inquiry_cdb, NULL, 0, NULL, 0,
+ &rsp_upiu);
+ g_assert_cmpuint(ocs, ==, UFS_OCS_INVALID_CMD_TABLE_ATTR);
+
+ ocs = ufs_send_scsi_command(ufs, test_lun, write_cdb, NULL, 0, NULL, 0,
+ &rsp_upiu);
+ g_assert_cmpuint(ocs, ==, UFS_OCS_INVALID_CMD_TABLE_ATTR);
+
+ /* The logical unit keeps working */
+ ocs = ufs_send_scsi_command(ufs, test_lun, inquiry_cdb, NULL, 0, buf, 0x24,
+ &rsp_upiu);
+ g_assert_cmpuint(ocs, ==, UFS_OCS_SUCCESS);
+ g_assert_cmpuint(rsp_upiu.header.scsi_status, ==, GOOD);
+ g_assert_cmpuint(buf[0] & 0x1f, ==, TYPE_DISK);
+
+ ufs_exit(ufs, alloc);
+}
+
static void ufstest_mcq_cq_wraparound(void *obj, void *data,
QGuestAllocator *alloc)
{
@@ -1721,6 +1765,7 @@ static void ufs_register_nodes(void)
qos_add_test("init", "ufs", ufstest_init, NULL);
qos_add_test("legacy-read-write", "ufs", ufstest_read_write,
&io_test_opts);
qos_add_test("mcq-read-write", "ufs", ufstest_read_write, &mcq_test_opts);
+ qos_add_test("no-prdt", "ufs", ufstest_no_prdt, &io_test_opts);
qos_add_test("mcq-cq-wraparound", "ufs", ufstest_mcq_cq_wraparound,
&mcq_test_opts);
qos_add_test("query-flag", "ufs", ufstest_query_flag_request,
--
2.43.0
