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

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

Hi Guangshuo,

kernel test robot noticed the following build errors:

[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on jejb-scsi/for-next akpm-mm/mm-everything linus/master v6.18-rc7 next-20251124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Guangshuo-Li/hptiop-Add-inbound-queue-offset-bounds-check-in-iop_get_config_itl/20251124-230112
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/20251124145848.45687-1-lgs201920130244%40gmail.com
patch subject: [PATCH] [SCSI] hptiop: Add inbound queue offset bounds check in iop_get_config_itl
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20251125/202511251457.AUHUgeyl-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251125/202511251457.AUHUgeyl-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511251457.AUHUgeyl-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/scsi/hptiop.c:170:17: error: no member named 'pdev' in 'struct hptiop_hba'
     170 |                 dev_err(&hba->pdev->dev,
         |                          ~~~  ^
   include/linux/dev_printk.h:154:44: note: expanded from macro 'dev_err'
     154 |         dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                   ^~~
   include/linux/dev_printk.h:110:11: note: expanded from macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                         ^~~
   1 error generated.


vim +170 drivers/scsi/hptiop.c

   160	
   161	static void mv_inbound_write(u64 p, struct hptiop_hba *hba)
   162	{
   163		u32 inbound_head = readl(&hba->u.mv.mu->inbound_head);
   164		u32 head = inbound_head + 1;
   165	
   166		if (head == MVIOP_QUEUE_LEN)
   167			head = 0;
   168	
   169		if (inbound_head >= MVIOP_QUEUE_LEN) {
 > 170			dev_err(&hba->pdev->dev,
   171				"hptiop: inbound_head out of range (%u)\n",
   172				inbound_head);
   173			inbound_head = 0;
   174			head = 1;
   175		}
   176	
   177		memcpy_toio(&hba->u.mv.mu->inbound_q[inbound_head], &p, 8);
   178		writel(head, &hba->u.mv.mu->inbound_head);
   179		writel(MVIOP_MU_INBOUND_INT_POSTQUEUE,
   180				&hba->u.mv.regs->inbound_doorbell);
   181	}
   182	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ 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: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

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:58 [PATCH] [SCSI] hptiop: Add inbound queue offset bounds check in iop_get_config_itl Guangshuo Li
2025-11-25  6:59 ` kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2025-11-24 14:20 Guangshuo Li

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®