mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] [SCSI] hptiop: Add inbound queue offset bounds check in iop_get_config_itl
@ 2025-11-24 14:20 Guangshuo Li
  0 siblings, 0 replies; 3+ messages in thread
From: Guangshuo Li @ 2025-11-24 14:20 UTC (permalink / raw)
  To: HighPoint Linux Team, James E.J. Bottomley, Martin K. Petersen,
	James Bottomley, linux-scsi, linux-kernel
  Cc: Guangshuo Li, stable

The function iop_get_config_itl() reads a 32‑bit offset (req32) from the
inbound queue register (hba->u.itl.iop->inbound_queue) and then uses it
without validation to compute:
    req = (base + req32)
followed by memcpy_fromio(config, req, sizeof(*config)).

Without verifying that req32 is within the valid I/O region and that
req32 + sizeof(*config) does not overflow the mapped I/O region, a
malicious or faulty device/firmware could cause the driver to read memory
outside the intended request structure — leading to an out‑of‑bounds I/O read.

According to kernel documentation:
  "The value returned from the inbound queue port is an offset relative
  to the IOP BAR0." ([docs.kernel.org](https://docs.kernel.org/scsi/hptiop.html))
However, the documentation does *not* specify a maximum offset, nor a bound
such as “offset + size ≤ IOP memory region size”.

In the driver code, hptiop_map_pci_bar_itl() does:
    hba->u.itl.iop = hptiop_map_pci_bar(hba, 0);
and uses pci_resource_len(hba->pcidev, 0) to obtain the mapped region size
for BAR0. Therefore we can use that size at runtime to bound req32 safely.

To implement the fix in iop_get_config_itl():
  - Retrieve the BAR0 region size via:
        struct pci_dev *pcidev = hba->pcidev;
        u32 length = pci_resource_len(pcidev, 0);
  - Then check:
        if (req32 == IOPMU_QUEUE_EMPTY || req32 + sizeof(*config) > length)
            return -EINVAL;
  - This ensures we do not rely on a hard‑coded maximum, but use the actual
    mapped region size for the bound.

Fixes: ede1e6f8b4324 ("[SCSI] hptiop: HighPoint RocketRAID 3xxx controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/scsi/hptiop.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
index f18b770626e6..c01370893a81 100644
--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -404,7 +404,10 @@ static int iop_get_config_itl(struct hptiop_hba *hba,
 	struct hpt_iop_request_get_config __iomem *req;
 
 	req32 = readl(&hba->u.itl.iop->inbound_queue);
-	if (req32 == IOPMU_QUEUE_EMPTY)
+
+	struct pci_dev *pcidev = hba->pcidev;
+	u32 length = pci_resource_len(pcidev, 0);
+	if (req32 == IOPMU_QUEUE_EMPTY || req32 + sizeof(*config) > length)
 		return -1;
 
 	req = (struct hpt_iop_request_get_config __iomem *)
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread
* [PATCH] [SCSI] hptiop: Add inbound queue offset bounds check in iop_get_config_itl
@ 2025-11-24 14:58 Guangshuo Li
  2025-11-25  6:59 ` kernel test robot
  0 siblings, 1 reply; 3+ messages in thread
From: Guangshuo Li @ 2025-11-24 14:58 UTC (permalink / raw)
  To: HighPoint Linux Team, James E.J. Bottomley, Martin K. Petersen,
	James Bottomley, Andrew Morton, linux-scsi, linux-kernel
  Cc: Guangshuo Li, stable

In the driver code for the MV‑based queue variant (struct hpt_iopmu_mv of the
hptiop driver), the field "inbound_head" is read from the hardware register
and used as an index into the array "inbound_q[MVIOP_QUEUE_LEN]". For example:

    u32 inbound_head = readl(&hba->u.mv.mu->inbound_head);
    /* ... */
    memcpy_toio(&hba->u.mv.mu->inbound_q[inbound_head], &p, 8);

The code then increments head and wraps it to zero when it equals MVIOP_QUEUE_LEN.
However, the driver does *not* check that the initial value of "inbound_head"
is strictly less than "MVIOP_QUEUE_LEN". If the hardware (or attacker‑controlled
firmware/hardware device) writes a malicious value into the inbound_head register
(which could be ≥ MVIOP_QUEUE_LEN), then subsequent "memcpy_toio" will write
past the end of "inbound_q", leading to an out‑of‑bounds write condition.

Since inbound_q is allocated with exactly MVIOP_QUEUE_LEN entries (see:

    __le64 inbound_q[MVIOP_QUEUE_LEN];  /* MVIOP_QUEUE_LEN == 512 */

), indexing at e.g. "inbound_head == 512" or greater results in undefined memory access
and potential corruption of adjacent fields or memory regions.

This issue is particularly concerning in scenarios where an attacker has control
or influence over the hardware/firmware on the adapter card (for example a malicious
or compromised controller), because they could deliberately set "inbound_head" to
a value outside the expected [0, MVIOP_QUEUE_LEN‑1] range, thus forcing the driver
to write arbitrary data beyond the queue bounds.

To mitigate this issue, we add a check to validate the value of "inbound_head"
before it is used as an index. If "inbound_head" is found to be out of bounds (≥ MVIOP_QUEUE_LEN),
the head will be reset to 0, and "head" will be set to 1 to ensure that a valid entry is written to
the queue. The resetting of "inbound_head" to 0 ensures that the queue processing can continue
safely and predictably, while the adjustment of "head = 1" ensures that the next valid index is used
for subsequent writes.

This prevents any out-of-bounds writes and ensures that the queue continues to operate safely
even if the hardware is compromised.

Fixes: 00f5970193e22 ("[SCSI] hptiop: add more adapter models and other fixes")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/scsi/hptiop.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
index c01370893a81..a1a3840e6ea8 100644
--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -166,6 +166,14 @@ static void mv_inbound_write(u64 p, struct hptiop_hba *hba)
 	if (head == MVIOP_QUEUE_LEN)
 		head = 0;
 
+	if (inbound_head >= MVIOP_QUEUE_LEN) {
+		dev_err(&hba->pdev->dev,
+			"hptiop: inbound_head out of range (%u)\n",
+			inbound_head);
+		inbound_head = 0;
+		head = 1;
+	}
+
 	memcpy_toio(&hba->u.mv.mu->inbound_q[inbound_head], &p, 8);
 	writel(head, &hba->u.mv.mu->inbound_head);
 	writel(MVIOP_MU_INBOUND_INT_POSTQUEUE,
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-25  7:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-24 14:20 [PATCH] [SCSI] hptiop: Add inbound queue offset bounds check in iop_get_config_itl Guangshuo Li
2025-11-24 14:58 Guangshuo Li
2025-11-25  6:59 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®