All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vadim Marchenko <jakosvadim@gmail.com>
To: HighPoint Linux Team <linux@highpoint-tech.com>
Cc: Vadim Marchenko <jakosvadim@gmail.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	lvc-project@linuxtesting.org
Subject: [PATCH] scsi: hptiop: Fix buffer overflow of hba->reqs[]
Date: Fri,  6 Oct 2023 23:46:53 +0300	[thread overview]
Message-ID: <20231006204653.4677-1-jakosvadim@gmail.com> (raw)

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@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


             reply	other threads:[~2023-10-06 20:47 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-06 20:46 Vadim Marchenko [this message]
2024-01-26 16:50 ` [PATCH] scsi: hptiop: Fix buffer overflow of hba->reqs[] Vadim Marchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231006204653.4677-1-jakosvadim@gmail.com \
    --to=jakosvadim@gmail.com \
    --cc=jejb@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux@highpoint-tech.com \
    --cc=lvc-project@linuxtesting.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.