[PATCH] scsi: hptiop: Fix buffer overflow of hba->reqs[]

From: Vadim Marchenko
Date: Fri Jan 26 2024 - 13:44:18 EST


Buffer overflow when accessing an hba->reqs[tag]. Since the tag value is
read from the device with readl(), it can be greater than
HPTIOP_MAX_REQUESTS, which is the maximum size of reqs[].
struct hptiop_hba { ... struct hptiop_request reqs[HPTIOP_MAX_REQUESTS]; ... }

For example, if tag is 0x80000101, then in hptiop.c:79 we will pass tag equal
to (tag & ~IOPMU_QUEUE_ADDR_HOST_BIT) = (0x80000101 & 0x7fffffff) = 0x101 = 257
and get a buffer overflow in hptiop_host_request_callback_itl().

To fix it, we need to get the last 8 bits of the tag before accessing the
hba->reqs[tag]. We can do this by calculating bitwise and of tag with macros
IOPMU_QUEUE_REQUEST_INDEX_BITS which is equal to 0xff.
By the way, array access that prevents overflow was in commit 286aa031664b
("[SCSI] hptiop: Support HighPoint RR4520/RR4522 HBA") in function
hptiop_request_callback_mvfrey(), and this fix extends it to all other cases.

Found by Linux Verification Center (linuxtesting.org) with KLEVER.

Signed-off-by: Vadim Marchenko <v.marchenko@xxxxxxxxx>
---
drivers/scsi/hptiop.c | 22 +++++++++++++++-------
drivers/scsi/hptiop.h | 1 +
2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
index f5334ccbf2ca..174a350c4f58 100644
--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -176,6 +176,7 @@ static void hptiop_request_callback_mv(struct hptiop_hba *hba, u64 tag)
{
u32 req_type = (tag >> 5) & 0x7;
struct hpt_iop_request_scsi_command *req;
+ u32 req_idx;

dprintk("hptiop_request_callback_mv: tag=%llx\n", tag);

@@ -188,7 +189,8 @@ static void hptiop_request_callback_mv(struct hptiop_hba *hba, u64 tag)
break;

case IOP_REQUEST_TYPE_SCSI_COMMAND:
- req = hba->reqs[tag >> 8].req_virt;
+ req_idx = (tag >> 8) & IOPMU_QUEUE_REQUEST_INDEX_BITS;
+ req = hba->reqs[req_idx].req_virt;
if (likely(tag & MVIOP_MU_QUEUE_REQUEST_RESULT_BIT))
req->header.result = cpu_to_le32(IOP_RESULT_SUCCESS);

@@ -231,6 +233,7 @@ static void hptiop_request_callback_mvfrey(struct hptiop_hba *hba, u32 _tag)
{
u32 req_type = _tag & 0xf;
struct hpt_iop_request_scsi_command *req;
+ u32 req_idx;

switch (req_type) {
case IOP_REQUEST_TYPE_GET_CONFIG:
@@ -239,10 +242,11 @@ static void hptiop_request_callback_mvfrey(struct hptiop_hba *hba, u32 _tag)
break;

case IOP_REQUEST_TYPE_SCSI_COMMAND:
- req = hba->reqs[(_tag >> 4) & 0xff].req_virt;
+ req_idx = (_tag >> 4) & IOPMU_QUEUE_REQUEST_INDEX_BITS;
+ req = hba->reqs[req_idx].req_virt;
if (likely(_tag & IOPMU_QUEUE_REQUEST_RESULT_BIT))
req->header.result = IOP_RESULT_SUCCESS;
- hptiop_finish_scsi_req(hba, (_tag >> 4) & 0xff, req);
+ hptiop_finish_scsi_req(hba, req_idx, req);
break;

default:
@@ -717,6 +721,7 @@ static void hptiop_finish_scsi_req(struct hptiop_hba *hba, u32 tag,
struct hpt_iop_request_scsi_command *req)
{
struct scsi_cmnd *scp;
+ u32 req_idx = tag & IOPMU_QUEUE_REQUEST_INDEX_BITS;

dprintk("hptiop_finish_scsi_req: req=%p, type=%d, "
"result=%d, context=0x%x tag=%d\n",
@@ -726,7 +731,7 @@ static void hptiop_finish_scsi_req(struct hptiop_hba *hba, u32 tag,
BUG_ON(!req->header.result);
BUG_ON(req->header.type != cpu_to_le32(IOP_REQUEST_TYPE_SCSI_COMMAND));

- scp = hba->reqs[tag].scp;
+ scp = hba->reqs[req_idx].scp;

if (HPT_SCP(scp)->mapped)
scsi_dma_unmap(scp);
@@ -770,22 +775,25 @@ static void hptiop_finish_scsi_req(struct hptiop_hba *hba, u32 tag,
skip_resid:
dprintk("scsi_done(%p)\n", scp);
scsi_done(scp);
- free_req(hba, &hba->reqs[tag]);
+ free_req(hba, &hba->reqs[req_idx]);
}

static void hptiop_host_request_callback_itl(struct hptiop_hba *hba, u32 _tag)
{
struct hpt_iop_request_scsi_command *req;
u32 tag;
+ u32 req_idx;

if (hba->iopintf_v2) {
tag = _tag & ~IOPMU_QUEUE_REQUEST_RESULT_BIT;
- req = hba->reqs[tag].req_virt;
+ req_idx = tag & IOPMU_QUEUE_REQUEST_INDEX_BITS;
+ req = hba->reqs[req_idx].req_virt;
if (likely(_tag & IOPMU_QUEUE_REQUEST_RESULT_BIT))
req->header.result = cpu_to_le32(IOP_RESULT_SUCCESS);
} else {
tag = _tag;
- req = hba->reqs[tag].req_virt;
+ req_idx = tag & IOPMU_QUEUE_REQUEST_INDEX_BITS;
+ req = hba->reqs[req_idx].req_virt;
}

hptiop_finish_scsi_req(hba, tag, req);
diff --git a/drivers/scsi/hptiop.h b/drivers/scsi/hptiop.h
index 394ef6aa469e..742ce87ab56d 100644
--- a/drivers/scsi/hptiop.h
+++ b/drivers/scsi/hptiop.h
@@ -32,6 +32,7 @@ struct hpt_iopmu_itl {
#define IOPMU_QUEUE_ADDR_HOST_BIT 0x80000000
#define IOPMU_QUEUE_REQUEST_SIZE_BIT 0x40000000
#define IOPMU_QUEUE_REQUEST_RESULT_BIT 0x40000000
+#define IOPMU_QUEUE_REQUEST_INDEX_BITS 0xff

#define IOPMU_OUTBOUND_INT_MSG0 1
#define IOPMU_OUTBOUND_INT_MSG1 2
--
2.39.2