[PATCH v1 0/4] media: ipu6: A few defensive fixes for ipu7 buttress, fw-com and mmu paths

[PATCH v1 0/4] media: ipu6: A few defensive fixes for ipu7 buttress, fw-com and mmu paths

[Manik Bajpai]

This series fixes several bugs found while reviewing the ipu7 buttress
power on/off, fw-com and mmu paths: a boot-config leak on a queue memory
allocation failure, a skipped cleanup step on a buttress power on/off
timeout, unchecked firmware-supplied queue indices used in pointer
arithmetic in ipu7_fw_com_get_token(), and a missing "is this iova mapped"
check in ipu6_mmu_iova_to_phys() that could dereference an invalid pointer
for an unmapped iova.

A related fix for a missing NULL check on the output pin queue in the ipu7
isr was dropped from this series, since an equivalent patch is already on
the list:

https://lore.kernel.org/linux-media/20260927201631.153126-2-devnexen@gmail.com/

Manik Bajpai (4):
  media: ipu6: Free boot config on queue memory alloc failure
  media: ipu6: Always run cleanup in ipu7 power on/off on timeout
  media: ipu6: Validate fw-com queue indices before use
  media: ipu6: Fix NULL deref in ipu6_mmu_iova_to_phys()

 drivers/media/pci/intel/ipu6/ipu6-buttress.c | 24 ++++++++++++--------
 drivers/media/pci/intel/ipu6/ipu6-mmu.c      | 11 ++++++++-
 drivers/media/pci/intel/ipu6/ipu7-boot.c     |  1 +
 drivers/media/pci/intel/ipu6/ipu7-fw-com.c   | 14 +++++++++++-
 drivers/media/pci/intel/ipu6/ipu7-fw-com.h   |  5 +++-
 drivers/media/pci/intel/ipu6/ipu7-fw-isys.c  |  7 +++---
 6 files changed, 47 insertions(+), 15 deletions(-)

-- 
2.53.0

[PATCH v1 1/4] media: ipu6: Free boot config on queue memory alloc failure

[Manik Bajpai]

If the queue_mem allocation in ipu7_init_boot_config() fails, the
function returned -ENOMEM without freeing the boot_config buffer it
had already allocated. The current caller happens to clean up via
ipu7_fw_isys_cleanup() -> ipu7_release_boot_config() on any failure
from this function, so this is not an active leak today, but
ipu7_init_boot_config() should not rely on a specific caller undoing
its partial allocations.

Call the existing ipu7_release_boot_config() helper before returning,
so the function cleans up after itself on its own error path,
independent of what the caller does.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu7-boot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/pci/intel/ipu6/ipu7-boot.c b/drivers/media/pci/intel/ipu6/ipu7-boot.c
index 5bd5281ec107..225f802cfd1c 100644
--- a/drivers/media/pci/intel/ipu6/ipu7-boot.c
+++ b/drivers/media/pci/intel/ipu6/ipu7-boot.c
@@ -246,6 +246,7 @@ int ipu7_init_boot_config(struct ipu6_bus_device *adev,
 					  GFP_KERNEL, 0);
 	if (!fwctx->queue_mem) {
 		dev_err(dev, "Failed to allocate queue memory.\n");
+		ipu7_release_boot_config(adev);
 		return -ENOMEM;
 	}
 	fwctx->queue_mem_size = total_queue_size_aligned;
-- 
2.53.0

[PATCH v1 2/4] media: ipu6: Always run cleanup in ipu7 power on/off on timeout

[Manik Bajpai]

__ipu7_power_on() and __ipu7_power_off() both return early when the
pwr_status readl_poll_timeout() call times out, skipping cleanup that
was previously done only on the success path:

 - __ipu7_power_on() left the clock-ownership override bit set in
   SLEEP_LEVEL_CFG instead of clearing it, which could break
   subsequent power transitions.

 - __ipu7_power_off() skipped D2D power-down and NDE-disable, leaving
   those subsystems powered despite reporting a failure.

 - Additionally, the return value of ipu7_isys_d2d_power() in
   __ipu7_power_off() was discarded, silently masking D2D power-down
   failures.

Always run the clock-override clear / D2D-NDE teardown regardless of
whether the status poll timed out, and propagate a D2D power-down
failure through the return value when the poll itself succeeded.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-buttress.c | 24 ++++++++++++--------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.c b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
index 105de1744dff..06a773b5b66b 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-buttress.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
@@ -530,16 +530,15 @@ static int __ipu7_power_on(struct device *dev,
 	ret = readl_poll_timeout(isp->base + isp->buttress.regs->pwr_status,
 				 val, (val & ctrl->pwr_sts_mask) == pwr_sts,
 				 100, BUTTRESS_POWER_TIMEOUT_US);
-	if (ret) {
+	if (ret)
 		dev_err(&isp->pdev->dev,
 			"Change power status timeout with 0x%x\n", val);
-		return ret;
-	}
 
+	/* Always drop the clock override, even if the poll above timed out. */
 	slp = readl(isp->base + IPU7_BUTTRESS_REG_SLEEP_LEVEL_CFG);
 	writel(slp & ~ovrd_clk, isp->base + IPU7_BUTTRESS_REG_SLEEP_LEVEL_CFG);
 
-	return 0;
+	return ret;
 }
 
 static int __ipu7_power_off(struct device *dev,
@@ -555,18 +554,25 @@ static int __ipu7_power_off(struct device *dev,
 	ret = readl_poll_timeout(isp->base + isp->buttress.regs->pwr_status,
 				 val, (val & ctrl->pwr_sts_mask) == pwr_sts,
 				 100, BUTTRESS_POWER_TIMEOUT_US);
-	if (ret) {
+	if (ret)
 		dev_err(&isp->pdev->dev,
 			"Change power status timeout with 0x%x\n", val);
-		return ret;
-	}
 
+	/* Always release D2D/NDE, even if the power status poll timed out. */
 	if (ctrl->subsys_id == IPU_ISYS) {
-		ipu7_isys_d2d_power(isp, false);
+		int d2d_ret;
+
+		d2d_ret = ipu7_isys_d2d_power(isp, false);
+		if (d2d_ret) {
+			dev_err(&isp->pdev->dev,
+				"D2D power down failed (%d)\n", d2d_ret);
+			if (!ret)
+				ret = d2d_ret;
+		}
 		ipu7_nde_control(isp, false);
 	}
 
-	return 0;
+	return ret;
 }
 
 static int __ipu6_power(struct device *dev,
-- 
2.53.0

[PATCH v1 3/4] media: ipu6: Validate fw-com queue indices before use

[Manik Bajpai]

ipu7_fw_com_get_token() reads read_index/write_index directly from
firmware-shared memory and uses them unchecked in pointer arithmetic
to compute a token address:

	token = queue_params->token_array_mem +
		read_index * queue_params->token_size_in_bytes;

If firmware ever writes a corrupted or out-of-range index into that
shared memory, this computes a pointer outside token_array_mem.

Reject indices that are not smaller than max_capacity before doing any
pointer arithmetic.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu7-fw-com.c  | 14 +++++++++++++-
 drivers/media/pci/intel/ipu6/ipu7-fw-com.h  |  5 ++++-
 drivers/media/pci/intel/ipu6/ipu7-fw-isys.c |  7 ++++---
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu7-fw-com.c b/drivers/media/pci/intel/ipu6/ipu7-fw-com.c
index 7dd1e683aa92..3d0a8fcac15e 100644
--- a/drivers/media/pci/intel/ipu6/ipu7-fw-com.c
+++ b/drivers/media/pci/intel/ipu6/ipu7-fw-com.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2026 Intel Corporation
  */
 
+#include <linux/device.h>
 #include <linux/io.h>
 
 #include "ipu7-fw-com.h"
@@ -13,7 +14,8 @@ static void __iomem *ipu7_fw_com_get_indices(struct ipu7_fw_com_context *ctx,
 	return ctx->queue_indices + (q * sizeof(struct ipu7_fw_com_queue_indices));
 }
 
-void *ipu7_fw_com_get_token(struct ipu7_fw_com_context *ctx, int q)
+void *ipu7_fw_com_get_token(struct device *dev, struct ipu7_fw_com_context *ctx,
+			    int q)
 {
 	struct ipu7_fw_com_queue_config *queue_params = &ctx->queue_configs[q];
 	void __iomem *queue_indices = ipu7_fw_com_get_indices(ctx, q);
@@ -25,6 +27,16 @@ void *ipu7_fw_com_get_token(struct ipu7_fw_com_context *ctx, int q)
 					read_index));
 	void *token = NULL;
 
+	/* Indices come from firmware-shared memory; don't trust them blindly. */
+	if (read_index >= queue_params->max_capacity ||
+	    write_index >= queue_params->max_capacity) {
+		dev_err_once(dev,
+			     "ipu7-fw-com: bad queue %d index (r=%u w=%u cap=%u)\n",
+			     q, read_index, write_index,
+			     queue_params->max_capacity);
+		return NULL;
+	}
+
 	if (q < ctx->num_output_queues) {
 		/* Output queue */
 		bool empty = (write_index == read_index);
diff --git a/drivers/media/pci/intel/ipu6/ipu7-fw-com.h b/drivers/media/pci/intel/ipu6/ipu7-fw-com.h
index 097eaab99547..2921d097e580 100644
--- a/drivers/media/pci/intel/ipu6/ipu7-fw-com.h
+++ b/drivers/media/pci/intel/ipu6/ipu7-fw-com.h
@@ -6,6 +6,8 @@
 
 #include <linux/types.h>
 
+struct device;
+
 struct ipu7_fw_com_queue_config {
 	void *token_array_mem;
 	u32 queue_size;
@@ -46,7 +48,8 @@ struct ipu7_fw_com_queue_indices {
 };
 
 void ipu7_fw_com_put_token(struct ipu7_fw_com_context *ctx, int q);
-void *ipu7_fw_com_get_token(struct ipu7_fw_com_context *ctx, int q);
+void *ipu7_fw_com_get_token(struct device *dev, struct ipu7_fw_com_context *ctx,
+			    int q);
 struct ipu7_fw_com_queue_params_config *
 ipu7_fw_com_get_queue_config(struct ipu7_fw_com_config *config);
 
diff --git a/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c b/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
index e74e2b2566aa..60e29e2d63a7 100644
--- a/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
+++ b/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
@@ -147,7 +147,8 @@ static int ipu7_fw_isys_init(struct ipu6_isys *isys, unsigned int num_streams)
 
 static struct ipu7_insys_resp *ipu7_fw_isys_get_resp(struct ipu6_isys *isys)
 {
-	return ipu7_fw_com_get_token(isys->fwctx, IPU7_INSYS_OUTPUT_MSG_QUEUE);
+	return ipu7_fw_com_get_token(&isys->adev->auxdev.dev, isys->fwctx,
+				     IPU7_INSYS_OUTPUT_MSG_QUEUE);
 }
 
 static void ipu7_fw_isys_put_resp(struct ipu6_isys *isys)
@@ -219,7 +220,6 @@ ipu7_fw_isys_send_cmd(struct ipu6_isys *isys, const unsigned int stream_handle,
 		      size_t size, u16 send_type)
 {
 	struct ipu7_fw_com_context *ctx = isys->fwctx;
-	/*struct device *dev = &isys->adev->auxdev.dev;*/
 	struct ipu7_insys_send_queue_token *token;
 
 	if (send_type >= N_IPU7_INSYS_SEND_TYPE)
@@ -228,7 +228,8 @@ ipu7_fw_isys_send_cmd(struct ipu6_isys *isys, const unsigned int stream_handle,
 	if (cpu_mapped_buf)
 		clflush_cache_range(cpu_mapped_buf, size);
 
-	token = ipu7_fw_com_get_token(ctx, stream_handle +
+	token = ipu7_fw_com_get_token(&isys->adev->auxdev.dev, ctx,
+				      stream_handle +
 				      IPU7_INSYS_INPUT_MSG_QUEUE);
 	if (!token)
 		return -EBUSY;
-- 
2.53.0

[PATCH v1 4/4] media: ipu6: Fix NULL deref in ipu6_mmu_iova_to_phys()

[Manik Bajpai]

ipu6_mmu_iova_to_phys() looked up the L2 page table for the given iova
and dereferenced it immediately, without checking whether the
corresponding L1 entry is actually mapped. l2_map() and l2_unmap()
already guard the equivalent lookup by checking the L1 entry against
mmu_info->dummy_l2_pteval before touching l2_pts[].

An unmapped iova (e.g. a stale address passed from a debug path)
would dereference an invalid pointer and crash.

Add the same "is this L1 entry mapped" check used by l2_map()/
l2_unmap(), returning 0 for an unmapped iova instead of crashing.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-mmu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-mmu.c b/drivers/media/pci/intel/ipu6/ipu6-mmu.c
index 243d438786ba..b8a8ed622b8d 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-mmu.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-mmu.c
@@ -566,10 +566,19 @@ phys_addr_t ipu6_mmu_iova_to_phys(struct ipu6_mmu_info *mmu_info,
 {
 	phys_addr_t phy_addr;
 	unsigned long flags;
+	u32 l1_idx = iova >> ISP_L1PT_SHIFT;
 	u32 *l2_pt;
 
 	spin_lock_irqsave(&mmu_info->lock, flags);
-	l2_pt = mmu_info->l2_pts[iova >> ISP_L1PT_SHIFT];
+	if (mmu_info->l1_pt[l1_idx] == mmu_info->dummy_l2_pteval) {
+		spin_unlock_irqrestore(&mmu_info->lock, flags);
+		dev_err(mmu_info->dev,
+			"iova_to_phys: no mapping for iova %pad\n",
+			&iova);
+		return 0;
+	}
+
+	l2_pt = mmu_info->l2_pts[l1_idx];
 	phy_addr = (phys_addr_t)l2_pt[(iova & ISP_L2PT_MASK) >> ISP_L2PT_SHIFT];
 	phy_addr <<= ISP_PAGE_SHIFT;
 	spin_unlock_irqrestore(&mmu_info->lock, flags);
-- 
2.53.0

Re: [PATCH v1 4/4] media: ipu6: Fix NULL deref in ipu6_mmu_iova_to_phys()

[Sakari Ailus]

Hi Manik,

On Mon, Sep 28, 2026 at 05:26:46PM +0530, Manik Bajpai wrote:
> ipu6_mmu_iova_to_phys() looked up the L2 page table for the given iova
> and dereferenced it immediately, without checking whether the
> corresponding L1 entry is actually mapped. l2_map() and l2_unmap()
> already guard the equivalent lookup by checking the L1 entry against
> mmu_info->dummy_l2_pteval before touching l2_pts[].
> 
> An unmapped iova (e.g. a stale address passed from a debug path)
> would dereference an invalid pointer and crash.
> 
> Add the same "is this L1 entry mapped" check used by l2_map()/
> l2_unmap(), returning 0 for an unmapped iova instead of crashing.

Have you observed this somewhere? I guess it shouldn't happen, but if it
has happened, I think we should add a Fixes: tag and Cc: stable here.

-- 
Regards,

Sakari Ailus

[PATCH v2 0/4] media: ipu6: A few defensive fixes for ipu7 buttress, fw-com and mmu paths

[Manik Bajpai]

This series fixes several bugs found while reviewing the ipu7 buttress
power on/off, fw-com and mmu paths: a boot-config leak on a queue memory
allocation failure, a skipped cleanup step on a buttress power on/off
timeout, unchecked firmware-supplied queue indices used in pointer
arithmetic in ipu7_fw_com_get_token(), and a missing "is this iova mapped"
check in ipu6_mmu_iova_to_phys() that could dereference an invalid pointer
for an unmapped iova.

A related fix for a missing NULL check on the output pin queue in the ipu7
isr was dropped from this series, since an equivalent patch is already on
the list:

https://lore.kernel.org/linux-media/20260927201631.153126-2-devnexen@gmail.com/

v2:
- ipu6_mmu_iova_to_phys(): this was found via code review, not from an
  observed crash (comparing against l2_map()/l2_unmap(), which already
  guard the equivalent lookup). Added a Fixes: tag pointing at the
  commit that introduced the unguarded lookup, but skipped a
  stable-tree backport since there's no confirmed real-world trigger
  (Sakari).

Manik Bajpai (4):
  media: ipu6: Free boot config on queue memory alloc failure
  media: ipu6: Always run cleanup in ipu7 power on/off on timeout
  media: ipu6: Validate fw-com queue indices before use
  media: ipu6: Fix NULL deref in ipu6_mmu_iova_to_phys()

 drivers/media/pci/intel/ipu6/ipu6-buttress.c | 24 ++++++++++++--------
 drivers/media/pci/intel/ipu6/ipu6-mmu.c      | 11 ++++++++-
 drivers/media/pci/intel/ipu6/ipu7-boot.c     |  1 +
 drivers/media/pci/intel/ipu6/ipu7-fw-com.c   | 14 +++++++++++-
 drivers/media/pci/intel/ipu6/ipu7-fw-com.h   |  5 +++-
 drivers/media/pci/intel/ipu6/ipu7-fw-isys.c  |  7 +++---
 6 files changed, 47 insertions(+), 15 deletions(-)

-- 
2.53.0

[PATCH v2 1/4] media: ipu6: Free boot config on queue memory alloc failure

[Manik Bajpai]

If the queue_mem allocation in ipu7_init_boot_config() fails, the
function returned -ENOMEM without freeing the boot_config buffer it
had already allocated. The current caller happens to clean up via
ipu7_fw_isys_cleanup() -> ipu7_release_boot_config() on any failure
from this function, so this is not an active leak today, but
ipu7_init_boot_config() should not rely on a specific caller undoing
its partial allocations.

Call the existing ipu7_release_boot_config() helper before returning,
so the function cleans up after itself on its own error path,
independent of what the caller does.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu7-boot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/pci/intel/ipu6/ipu7-boot.c b/drivers/media/pci/intel/ipu6/ipu7-boot.c
index 5bd5281ec107..225f802cfd1c 100644
--- a/drivers/media/pci/intel/ipu6/ipu7-boot.c
+++ b/drivers/media/pci/intel/ipu6/ipu7-boot.c
@@ -246,6 +246,7 @@ int ipu7_init_boot_config(struct ipu6_bus_device *adev,
 					  GFP_KERNEL, 0);
 	if (!fwctx->queue_mem) {
 		dev_err(dev, "Failed to allocate queue memory.\n");
+		ipu7_release_boot_config(adev);
 		return -ENOMEM;
 	}
 	fwctx->queue_mem_size = total_queue_size_aligned;
-- 
2.53.0

Re: [PATCH v2 1/4] media: ipu6: Free boot config on queue memory alloc failure

[Sakari Ailus]

Hi Manik,

On Tue, Sep 29, 2026 at 02:01:27PM +0530, Manik Bajpai wrote:
> If the queue_mem allocation in ipu7_init_boot_config() fails, the
> function returned -ENOMEM without freeing the boot_config buffer it
> had already allocated. The current caller happens to clean up via
> ipu7_fw_isys_cleanup() -> ipu7_release_boot_config() on any failure
> from this function, so this is not an active leak today, but
> ipu7_init_boot_config() should not rely on a specific caller undoing
> its partial allocations.
> 
> Call the existing ipu7_release_boot_config() helper before returning,
> so the function cleans up after itself on its own error path,
> independent of what the caller does.
> 
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
> ---
>  drivers/media/pci/intel/ipu6/ipu7-boot.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu7-boot.c b/drivers/media/pci/intel/ipu6/ipu7-boot.c
> index 5bd5281ec107..225f802cfd1c 100644
> --- a/drivers/media/pci/intel/ipu6/ipu7-boot.c
> +++ b/drivers/media/pci/intel/ipu6/ipu7-boot.c
> @@ -246,6 +246,7 @@ int ipu7_init_boot_config(struct ipu6_bus_device *adev,
>  					  GFP_KERNEL, 0);
>  	if (!fwctx->queue_mem) {
>  		dev_err(dev, "Failed to allocate queue memory.\n");
> +		ipu7_release_boot_config(adev);

I believe on error, the caller already calls ipu7_fw_isys_cleanup() that
includes a call to ipu7_release_boot_config().

The rest of the patches seem fine to me.

>  		return -ENOMEM;
>  	}
>  	fwctx->queue_mem_size = total_queue_size_aligned;

-- 
Kind regards,

Sakari Ailus

[PATCH v2 2/4] media: ipu6: Always run cleanup in ipu7 power on/off on timeout

[Manik Bajpai]

__ipu7_power_on() and __ipu7_power_off() both return early when the
pwr_status readl_poll_timeout() call times out, skipping cleanup that
was previously done only on the success path:

 - __ipu7_power_on() left the clock-ownership override bit set in
   SLEEP_LEVEL_CFG instead of clearing it, which could break
   subsequent power transitions.

 - __ipu7_power_off() skipped D2D power-down and NDE-disable, leaving
   those subsystems powered despite reporting a failure.

 - Additionally, the return value of ipu7_isys_d2d_power() in
   __ipu7_power_off() was discarded, silently masking D2D power-down
   failures.

Always run the clock-override clear / D2D-NDE teardown regardless of
whether the status poll timed out, and propagate a D2D power-down
failure through the return value when the poll itself succeeded.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-buttress.c | 24 ++++++++++++--------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.c b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
index 105de1744dff..06a773b5b66b 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-buttress.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
@@ -530,16 +530,15 @@ static int __ipu7_power_on(struct device *dev,
 	ret = readl_poll_timeout(isp->base + isp->buttress.regs->pwr_status,
 				 val, (val & ctrl->pwr_sts_mask) == pwr_sts,
 				 100, BUTTRESS_POWER_TIMEOUT_US);
-	if (ret) {
+	if (ret)
 		dev_err(&isp->pdev->dev,
 			"Change power status timeout with 0x%x\n", val);
-		return ret;
-	}
 
+	/* Always drop the clock override, even if the poll above timed out. */
 	slp = readl(isp->base + IPU7_BUTTRESS_REG_SLEEP_LEVEL_CFG);
 	writel(slp & ~ovrd_clk, isp->base + IPU7_BUTTRESS_REG_SLEEP_LEVEL_CFG);
 
-	return 0;
+	return ret;
 }
 
 static int __ipu7_power_off(struct device *dev,
@@ -555,18 +554,25 @@ static int __ipu7_power_off(struct device *dev,
 	ret = readl_poll_timeout(isp->base + isp->buttress.regs->pwr_status,
 				 val, (val & ctrl->pwr_sts_mask) == pwr_sts,
 				 100, BUTTRESS_POWER_TIMEOUT_US);
-	if (ret) {
+	if (ret)
 		dev_err(&isp->pdev->dev,
 			"Change power status timeout with 0x%x\n", val);
-		return ret;
-	}
 
+	/* Always release D2D/NDE, even if the power status poll timed out. */
 	if (ctrl->subsys_id == IPU_ISYS) {
-		ipu7_isys_d2d_power(isp, false);
+		int d2d_ret;
+
+		d2d_ret = ipu7_isys_d2d_power(isp, false);
+		if (d2d_ret) {
+			dev_err(&isp->pdev->dev,
+				"D2D power down failed (%d)\n", d2d_ret);
+			if (!ret)
+				ret = d2d_ret;
+		}
 		ipu7_nde_control(isp, false);
 	}
 
-	return 0;
+	return ret;
 }
 
 static int __ipu6_power(struct device *dev,
-- 
2.53.0

Re: [PATCH v2 2/4] media: ipu6: Always run cleanup in ipu7 power on/off on timeout

[Antti Laakso]

Hi Manik,
On Tue, Sep 29, 2026 at 02:01:28PM +0530, Manik Bajpai wrote:
> __ipu7_power_on() and __ipu7_power_off() both return early when the
> pwr_status readl_poll_timeout() call times out, skipping cleanup that
> was previously done only on the success path:
> 
>  - __ipu7_power_on() left the clock-ownership override bit set in
>    SLEEP_LEVEL_CFG instead of clearing it, which could break
>    subsequent power transitions.
> 
>  - __ipu7_power_off() skipped D2D power-down and NDE-disable, leaving
>    those subsystems powered despite reporting a failure.
> 
>  - Additionally, the return value of ipu7_isys_d2d_power() in
>    __ipu7_power_off() was discarded, silently masking D2D power-down
>    failures.
> 
> Always run the clock-override clear / D2D-NDE teardown regardless of
> whether the status poll timed out, and propagate a D2D power-down
> failure through the return value when the poll itself succeeded.
> 
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
> ---
>  drivers/media/pci/intel/ipu6/ipu6-buttress.c | 24 ++++++++++++--------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.c b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
> index 105de1744dff..06a773b5b66b 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-buttress.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
> @@ -530,16 +530,15 @@ static int __ipu7_power_on(struct device *dev,
>  	ret = readl_poll_timeout(isp->base + isp->buttress.regs->pwr_status,
>  				 val, (val & ctrl->pwr_sts_mask) == pwr_sts,
>  				 100, BUTTRESS_POWER_TIMEOUT_US);
> -	if (ret) {
> +	if (ret)
>  		dev_err(&isp->pdev->dev,
>  			"Change power status timeout with 0x%x\n", val);
> -		return ret;
> -	}
>  
> +	/* Always drop the clock override, even if the poll above timed out. */
>  	slp = readl(isp->base + IPU7_BUTTRESS_REG_SLEEP_LEVEL_CFG);
>  	writel(slp & ~ovrd_clk, isp->base + IPU7_BUTTRESS_REG_SLEEP_LEVEL_CFG);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int __ipu7_power_off(struct device *dev,
> @@ -555,18 +554,25 @@ static int __ipu7_power_off(struct device *dev,
>  	ret = readl_poll_timeout(isp->base + isp->buttress.regs->pwr_status,
>  				 val, (val & ctrl->pwr_sts_mask) == pwr_sts,
>  				 100, BUTTRESS_POWER_TIMEOUT_US);
> -	if (ret) {
> +	if (ret)
>  		dev_err(&isp->pdev->dev,
>  			"Change power status timeout with 0x%x\n", val);
> -		return ret;
> -	}
>  

I wonder if powering down D2D/NDE is right thing to do. In a caller,
bus_pm_runtime_suspend(), we try to recover from error and call
isys_runtime_pm_resume(). But after this change the D2D and NDE
would be powered off and recovery would not make sense.

> +	/* Always release D2D/NDE, even if the power status poll timed out. */
>  	if (ctrl->subsys_id == IPU_ISYS) {
> -		ipu7_isys_d2d_power(isp, false);
> +		int d2d_ret;
> +
> +		d2d_ret = ipu7_isys_d2d_power(isp, false);
> +		if (d2d_ret) {
> +			dev_err(&isp->pdev->dev,
> +				"D2D power down failed (%d)\n", d2d_ret);
> +			if (!ret)
> +				ret = d2d_ret;
> +		}
>  		ipu7_nde_control(isp, false);
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int __ipu6_power(struct device *dev,
> -- 
> 2.53.0
> 

[PATCH v2 3/4] media: ipu6: Validate fw-com queue indices before use

[Manik Bajpai]

ipu7_fw_com_get_token() reads read_index/write_index directly from
firmware-shared memory and uses them unchecked in pointer arithmetic
to compute a token address:

	token = queue_params->token_array_mem +
		read_index * queue_params->token_size_in_bytes;

If firmware ever writes a corrupted or out-of-range index into that
shared memory, this computes a pointer outside token_array_mem.

Reject indices that are not smaller than max_capacity before doing any
pointer arithmetic.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu7-fw-com.c  | 14 +++++++++++++-
 drivers/media/pci/intel/ipu6/ipu7-fw-com.h  |  5 ++++-
 drivers/media/pci/intel/ipu6/ipu7-fw-isys.c |  7 ++++---
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu7-fw-com.c b/drivers/media/pci/intel/ipu6/ipu7-fw-com.c
index 7dd1e683aa92..3d0a8fcac15e 100644
--- a/drivers/media/pci/intel/ipu6/ipu7-fw-com.c
+++ b/drivers/media/pci/intel/ipu6/ipu7-fw-com.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2026 Intel Corporation
  */
 
+#include <linux/device.h>
 #include <linux/io.h>
 
 #include "ipu7-fw-com.h"
@@ -13,7 +14,8 @@ static void __iomem *ipu7_fw_com_get_indices(struct ipu7_fw_com_context *ctx,
 	return ctx->queue_indices + (q * sizeof(struct ipu7_fw_com_queue_indices));
 }
 
-void *ipu7_fw_com_get_token(struct ipu7_fw_com_context *ctx, int q)
+void *ipu7_fw_com_get_token(struct device *dev, struct ipu7_fw_com_context *ctx,
+			    int q)
 {
 	struct ipu7_fw_com_queue_config *queue_params = &ctx->queue_configs[q];
 	void __iomem *queue_indices = ipu7_fw_com_get_indices(ctx, q);
@@ -25,6 +27,16 @@ void *ipu7_fw_com_get_token(struct ipu7_fw_com_context *ctx, int q)
 					read_index));
 	void *token = NULL;
 
+	/* Indices come from firmware-shared memory; don't trust them blindly. */
+	if (read_index >= queue_params->max_capacity ||
+	    write_index >= queue_params->max_capacity) {
+		dev_err_once(dev,
+			     "ipu7-fw-com: bad queue %d index (r=%u w=%u cap=%u)\n",
+			     q, read_index, write_index,
+			     queue_params->max_capacity);
+		return NULL;
+	}
+
 	if (q < ctx->num_output_queues) {
 		/* Output queue */
 		bool empty = (write_index == read_index);
diff --git a/drivers/media/pci/intel/ipu6/ipu7-fw-com.h b/drivers/media/pci/intel/ipu6/ipu7-fw-com.h
index 097eaab99547..2921d097e580 100644
--- a/drivers/media/pci/intel/ipu6/ipu7-fw-com.h
+++ b/drivers/media/pci/intel/ipu6/ipu7-fw-com.h
@@ -6,6 +6,8 @@
 
 #include <linux/types.h>
 
+struct device;
+
 struct ipu7_fw_com_queue_config {
 	void *token_array_mem;
 	u32 queue_size;
@@ -46,7 +48,8 @@ struct ipu7_fw_com_queue_indices {
 };
 
 void ipu7_fw_com_put_token(struct ipu7_fw_com_context *ctx, int q);
-void *ipu7_fw_com_get_token(struct ipu7_fw_com_context *ctx, int q);
+void *ipu7_fw_com_get_token(struct device *dev, struct ipu7_fw_com_context *ctx,
+			    int q);
 struct ipu7_fw_com_queue_params_config *
 ipu7_fw_com_get_queue_config(struct ipu7_fw_com_config *config);
 
diff --git a/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c b/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
index e74e2b2566aa..60e29e2d63a7 100644
--- a/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
+++ b/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
@@ -147,7 +147,8 @@ static int ipu7_fw_isys_init(struct ipu6_isys *isys, unsigned int num_streams)
 
 static struct ipu7_insys_resp *ipu7_fw_isys_get_resp(struct ipu6_isys *isys)
 {
-	return ipu7_fw_com_get_token(isys->fwctx, IPU7_INSYS_OUTPUT_MSG_QUEUE);
+	return ipu7_fw_com_get_token(&isys->adev->auxdev.dev, isys->fwctx,
+				     IPU7_INSYS_OUTPUT_MSG_QUEUE);
 }
 
 static void ipu7_fw_isys_put_resp(struct ipu6_isys *isys)
@@ -219,7 +220,6 @@ ipu7_fw_isys_send_cmd(struct ipu6_isys *isys, const unsigned int stream_handle,
 		      size_t size, u16 send_type)
 {
 	struct ipu7_fw_com_context *ctx = isys->fwctx;
-	/*struct device *dev = &isys->adev->auxdev.dev;*/
 	struct ipu7_insys_send_queue_token *token;
 
 	if (send_type >= N_IPU7_INSYS_SEND_TYPE)
@@ -228,7 +228,8 @@ ipu7_fw_isys_send_cmd(struct ipu6_isys *isys, const unsigned int stream_handle,
 	if (cpu_mapped_buf)
 		clflush_cache_range(cpu_mapped_buf, size);
 
-	token = ipu7_fw_com_get_token(ctx, stream_handle +
+	token = ipu7_fw_com_get_token(&isys->adev->auxdev.dev, ctx,
+				      stream_handle +
 				      IPU7_INSYS_INPUT_MSG_QUEUE);
 	if (!token)
 		return -EBUSY;
-- 
2.53.0

[PATCH v2 4/4] media: ipu6: Fix NULL deref in ipu6_mmu_iova_to_phys()

[Manik Bajpai]

ipu6_mmu_iova_to_phys() looked up the L2 page table for the given iova
and dereferenced it immediately, without checking whether the
corresponding L1 entry is actually mapped. l2_map() and l2_unmap()
already guard the equivalent lookup by checking the L1 entry against
mmu_info->dummy_l2_pteval before touching l2_pts[].

An unmapped iova (e.g. a stale address passed from a debug path)
would dereference an invalid pointer and crash.

Add the same "is this L1 entry mapped" check used by l2_map()/
l2_unmap(), returning 0 for an unmapped iova instead of crashing.

Fixes: 9163d83573e4 ("media: intel/ipu6: add IPU6 DMA mapping API and MMU table")
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Manik Bajpai <manik.bajpai@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-mmu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-mmu.c b/drivers/media/pci/intel/ipu6/ipu6-mmu.c
index 243d438786ba..b8a8ed622b8d 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-mmu.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-mmu.c
@@ -566,10 +566,19 @@ phys_addr_t ipu6_mmu_iova_to_phys(struct ipu6_mmu_info *mmu_info,
 {
 	phys_addr_t phy_addr;
 	unsigned long flags;
+	u32 l1_idx = iova >> ISP_L1PT_SHIFT;
 	u32 *l2_pt;
 
 	spin_lock_irqsave(&mmu_info->lock, flags);
-	l2_pt = mmu_info->l2_pts[iova >> ISP_L1PT_SHIFT];
+	if (mmu_info->l1_pt[l1_idx] == mmu_info->dummy_l2_pteval) {
+		spin_unlock_irqrestore(&mmu_info->lock, flags);
+		dev_err(mmu_info->dev,
+			"iova_to_phys: no mapping for iova %pad\n",
+			&iova);
+		return 0;
+	}
+
+	l2_pt = mmu_info->l2_pts[l1_idx];
 	phy_addr = (phys_addr_t)l2_pt[(iova & ISP_L2PT_MASK) >> ISP_L2PT_SHIFT];
 	phy_addr <<= ISP_PAGE_SHIFT;
 	spin_unlock_irqrestore(&mmu_info->lock, flags);
-- 
2.53.0