[PATCH] staging: media: ipu3: fix out-of-bounds access in imgu_map_node()

[PATCH] staging: media: ipu3: fix out-of-bounds access in imgu_map_node()

[Sajja Easwar Sai]

imgu_map_node() walks imgu_node_map[] looking for a CSS queue ID. When
no match is found the loop exits with i == IMGU_NODE_NUM, which is one
past the end of every array that is indexed by node id.  The value is
returned without any bounds check, so callers that use it immediately
as an array subscript produce out-of-bounds reads.

The most critical caller is the threaded IRQ handler
imgu_isr_threaded(), where b->queue comes directly from firmware; a
malformed or buggy firmware return could therefore trigger a kernel
oops.

Harden the code in three steps:
 1. Add a WARN_ON() inside imgu_map_node() so the 'not-found' sentinel
    is made explicit and any future regression surfaces immediately.
 2. Guard imgu_isr_threaded(): skip the affected buffer and emit a
    dev_err() rather than indexing imgu_node_map[] out of bounds.
 3. Guard imgu_dummybufs_init(): continue the loop if the lookup fails
    (this cannot happen today, but protects against future queue-table
    changes).

Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver")
Signed-off-by: Sajja Easwar Sai <eshwarsajja20@gmail.com>
---
diff --git a/drivers/staging/media/ipu3/ipu3.c b/drivers/staging/media/ipu3/ipu3.c
index 84c4d0bf027d..b231e7246f52 100644
--- a/drivers/staging/media/ipu3/ipu3.c
+++ b/drivers/staging/media/ipu3/ipu3.c
@@ -62,6 +62,12 @@ unsigned int imgu_map_node(struct imgu_device *imgu, unsigned int css_queue)
 		if (imgu_node_map[i].css_queue == css_queue)
 			break;
 
+	/*
+	 * If no entry matched, i == IMGU_NODE_NUM which is one past the end
+	 * of every array indexed by node id.  Callers must check for this
+	 * sentinel before using the returned value as an array index.
+	 */
+	WARN_ON(i >= IMGU_NODE_NUM);
 	return i;
 }
 
@@ -115,6 +121,8 @@ static int imgu_dummybufs_init(struct imgu_device *imgu, unsigned int pipe)
 	/* Allocate a dummy buffer for each queue where buffer is optional */
 	for (i = 0; i < IPU3_CSS_QUEUES; i++) {
 		node = imgu_map_node(imgu, i);
+		if (node >= IMGU_NODE_NUM)
+			continue;
 		if (!imgu_pipe->queue_enabled[node] || i == IMGU_QUEUE_MASTER)
 			continue;
 
@@ -535,6 +543,12 @@ static irqreturn_t imgu_isr_threaded(int irq, void *imgu_ptr)
 		}
 
 		node = imgu_map_node(imgu, b->queue);
+		if (node >= IMGU_NODE_NUM) {
+			dev_err(&imgu->pci_dev->dev,
+				"dequeued buffer with unknown css queue %u, skipping\n",
+				b->queue);
+			continue;
+		}
 		pipe = b->pipe;
 		dummy = imgu_dummybufs_check(imgu, b, pipe);
 		if (!dummy)

Re: [PATCH] staging: media: ipu3: fix out-of-bounds access in imgu_map_node()

[Greg KH]

On Wed, Apr 22, 2026 at 11:49:51AM +0530, Sajja Easwar Sai wrote:
> imgu_map_node() walks imgu_node_map[] looking for a CSS queue ID. When
> no match is found the loop exits with i == IMGU_NODE_NUM, which is one
> past the end of every array that is indexed by node id.  The value is
> returned without any bounds check, so callers that use it immediately
> as an array subscript produce out-of-bounds reads.
> 
> The most critical caller is the threaded IRQ handler
> imgu_isr_threaded(), where b->queue comes directly from firmware; a
> malformed or buggy firmware return could therefore trigger a kernel
> oops.
> 
> Harden the code in three steps:
>  1. Add a WARN_ON() inside imgu_map_node() so the 'not-found' sentinel
>     is made explicit and any future regression surfaces immediately.

And then you just rebooted the machine, causing all data to be lost when
panic-on-warn is enabled :(

If this condition can be hit, then great, handle it properly and
recover, please do not crash machines.  WARN_ON() should not be used for
anything that a user can ever cause to have happen.

thanks,

greg k-h

Re: [PATCH] staging: media: ipu3: fix out-of-bounds access in imgu_map_node()

[Sakari Ailus]

Hi Sajja,

On Wed, Apr 22, 2026 at 11:49:51AM +0530, Sajja Easwar Sai wrote:
> imgu_map_node() walks imgu_node_map[] looking for a CSS queue ID. When
> no match is found the loop exits with i == IMGU_NODE_NUM, which is one
> past the end of every array that is indexed by node id.  The value is
> returned without any bounds check, so callers that use it immediately
> as an array subscript produce out-of-bounds reads.
> 
> The most critical caller is the threaded IRQ handler
> imgu_isr_threaded(), where b->queue comes directly from firmware; a
> malformed or buggy firmware return could therefore trigger a kernel
> oops.

Have you seen this happen in practice?

-- 
Regards,

Sakari Ailus