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

[Vadim Marchenko]

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 <jakosvadim at gmail.com>
---
 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