From: Daejun Park <[email protected]>
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 MISMATCH DATA BUFFER SIZE before it reaches scsi-hd:
the PRDT describes less data than the command moves. 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]>
Reviewed-by: Jeuk Kim <[email protected]>
Signed-off-by: Jeuk Kim <[email protected]>
---
hw/ufs/lu.c | 24 ++++++++++++++++++----
hw/ufs/ufs.c | 10 ++++++++--
hw/ufs/ufs.h | 2 ++
tests/qtest/ufs-test.c | 45 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 75 insertions(+), 6 deletions(-)
diff --git a/hw/ufs/lu.c b/hw/ufs/lu.c
index ee4bb75361c..1101e368734 100644
--- a/hw/ufs/lu.c
+++ b/hw/ufs/lu.c
@@ -406,6 +406,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
@@ -417,12 +419,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 with OCS MISMATCH DATA BUFFER
+ * SIZE, as virtio-scsi fails a buffer that 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_MISMATCH_DATA_BUF;
+ }
+
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/hw/ufs/ufs.c b/hw/ufs/ufs.c
index f008af7467e..dda615da7ce 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -2451,10 +2451,16 @@ void ufs_complete_req(UfsRequest *req, UfsReqResult
req_result)
UfsHc *u = req->hc;
assert(req->state == UFS_REQUEST_RUNNING);
- if (req_result == UFS_REQUEST_SUCCESS) {
+ switch (req_result) {
+ case UFS_REQUEST_SUCCESS:
req->utrd.header.dword_2 = cpu_to_le32(UFS_OCS_SUCCESS);
- } else {
+ break;
+ case UFS_REQUEST_MISMATCH_DATA_BUF:
+ req->utrd.header.dword_2 = cpu_to_le32(UFS_OCS_MISMATCH_DATA_BUF_SIZE);
+ break;
+ default:
req->utrd.header.dword_2 = cpu_to_le32(UFS_OCS_INVALID_CMD_TABLE_ATTR);
+ break;
}
req->state = UFS_REQUEST_COMPLETE;
diff --git a/hw/ufs/ufs.h b/hw/ufs/ufs.h
index 265a43faaa6..adf74b6833d 100644
--- a/hw/ufs/ufs.h
+++ b/hw/ufs/ufs.h
@@ -47,6 +47,8 @@ typedef enum UfsReqResult {
UFS_REQUEST_SUCCESS = 0,
UFS_REQUEST_FAIL = 1,
UFS_REQUEST_NO_COMPLETE = 2,
+ /* The PRDT describes less data than the command moves */
+ UFS_REQUEST_MISMATCH_DATA_BUF = 3,
} UfsReqResult;
#define UFS_INVALID_SLOT (-1)
diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
index 9a9c5ca8346..ea7b9aa62a7 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_MISMATCH_DATA_BUF_SIZE);
+
+ ocs = ufs_send_scsi_command(ufs, test_lun, write_cdb, NULL, 0, NULL, 0,
+ &rsp_upiu);
+ g_assert_cmpuint(ocs, ==, UFS_OCS_MISMATCH_DATA_BUF_SIZE);
+
+ /* 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
