media: ipu6: Fix ipu7 firmware context leak on stream start

PATCH Superseded David Carlier 2 participants 3 messages Sep 27 – Sep 29

Subsystems (2)

Media (V4L/DVB) Intel IPU6 input system driver

lore

Share & tools

lore

David Carlier

author

2026-09-27 20:16 UTC #0 lore raw

PATCH 1/2 media: ipu6: Fix ipu7 firmware context leak on stream start

ipu7_fw_isys_init() runs on every first stream start, but allocates the firmware context and queue configs with devm and never frees them, so each STREAMON/STREAMOFF cycle leaks them. Use kzalloc and free them in ipu7_fw_isys_cleanup().

Fixes 9ab793dbc176 ("media: ipu6: Add ipu7 fw isys ops")

Signed-off-by David Carlier <[email protected]>

---
 drivers/media/pci/intel/ipu6/ipu7-fw-isys.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

drivers/media/pci/intel/ipu6/ipu7-fw-isys.c +7 -6

index aee9227fd66c..19e063b22a15 100644
@@ -34,6 +34,8 @@ static void ipu7_fw_isys_cleanup(struct ipu6_isys *isys)
 	}
 
 	isys->fwctx = NULL;
+	kfree(fwctx->queue_configs);
+	kfree(fwctx);
 }
 
 static int ipu7_fw_isys_open(struct ipu6_isys *isys)

@@ -67,8 +69,7 @@ static int ipu7_fw_isys_init(struct ipu6_isys *isys, unsigned int num_streams)
 	int ret;
 
 	/* Allocate and init firmware context. */
-	fwctx = devm_kzalloc(dev, sizeof(struct ipu7_fw_com_context),
-			     GFP_KERNEL);
+	fwctx = kzalloc_obj(*fwctx);
 	if (!fwctx)
 		return -ENOMEM;

@@ -76,10 +77,9 @@ static int ipu7_fw_isys_init(struct ipu6_isys *isys, unsigned int num_streams)
 	fwctx->num_output_queues = IPU7_INSYS_MAX_OUTPUT_QUEUES;
 	num_queues = fwctx->num_input_queues + fwctx->num_output_queues;
 
-	queue_configs = devm_kcalloc(dev, num_queues, sizeof(*queue_configs),
-				     GFP_KERNEL);
+	queue_configs = kzalloc_objs(*queue_configs, num_queues);
 	if (!queue_configs) {
-		ipu7_fw_isys_cleanup(isys);
+		kfree(fwctx);
 		return -ENOMEM;
 	}
 	fwctx->fw_entry = adev->fw_entry;

@@ -111,7 +111,8 @@ static int ipu7_fw_isys_init(struct ipu6_isys *isys, unsigned int num_streams)
 				   &fw_config_dma_addr, GFP_KERNEL, 0);
 	if (!fw_config) {
 		dev_err(dev, "Failed to allocate isys subsys config.\n");
-		ipu7_fw_isys_cleanup(isys);
+		kfree(queue_configs);
+		kfree(fwctx);
 		return -ENOMEM;
 	}
 	fwctx->fw_config = fw_config;

Signature · 1 line

2.55.0

David Carlier

author

2026-09-27 20:16 UTC #1 lore raw

PATCH 2/2 media: ipu6: Check the ipu7 output pin queue before completing a buffer

A PIN_DATA_READY response for an unconfigured pin dereferences a NULL output_pins_queue entry in interrupt context. Check the queue like the ipu6 path does.

Fixes f268203d74ff ("media: ipu6: Add ipu7 isr handler")

Signed-off-by David Carlier <[email protected]>

---
 drivers/media/pci/intel/ipu6/ipu7-fw-isys.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/media/pci/intel/ipu6/ipu7-fw-isys.c +2 -1

index 19e063b22a15..dca9c8fbc3ac 100644
@@ -638,7 +638,8 @@ static int ipu7_isys_isr_one(struct ipu6_bus_device *adev)
 					   struct isys_fw_msgs, dummy);
 
 		ipu6_put_fw_msg_buf(ipu6_bus_get_drvdata(adev), isys_fw_msg);
-		if (resp->pin_id < IPU6_ISYS_OUTPUT_PINS)
+		if (resp->pin_id < IPU6_ISYS_OUTPUT_PINS &&
+		    stream->output_pins_queue[resp->pin_id])
 			ipu6_stream_buf_ready(stream, resp->pin_id,
 					      resp->pin.addr, ts, 0);
 		else

Signature · 1 line

2.55.0

Sakari Ailus

maintainer

2026-09-29 12:00 UTC #2 lore raw

In reply to David Carlier

Hi David,

Thank you for the set.

Quoted 48 lines · David Carlier

Fixes 9ab793dbc176 ("media: ipu6: Add ipu7 fw isys ops")

Signed-off-by David Carlier <[email protected]>

---
 drivers/media/pci/intel/ipu6/ipu7-fw-isys.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

drivers/media/pci/intel/ipu6/ipu7-fw-isys.c +5 -5

index aee9227fd66c..19e063b22a15 100644
@@ -34,6 +34,8 @@ static void ipu7_fw_isys_cleanup(struct ipu6_isys *isys)
 	}
 
 	isys->fwctx = NULL;
+	kfree(fwctx->queue_configs);
+	kfree(fwctx);
 }
 
 static int ipu7_fw_isys_open(struct ipu6_isys *isys)

@@ -67,8 +69,7 @@ static int ipu7_fw_isys_init(struct ipu6_isys *isys, unsigned int num_streams)
 	int ret;
 
 	/* Allocate and init firmware context. */
-	fwctx = devm_kzalloc(dev, sizeof(struct ipu7_fw_com_context),
-			     GFP_KERNEL);
+	fwctx = kzalloc_obj(*fwctx);
 	if (!fwctx)
 		return -ENOMEM;

@@ -76,10 +77,9 @@ static int ipu7_fw_isys_init(struct ipu6_isys *isys, unsigned int num_streams)
 	fwctx->num_output_queues = IPU7_INSYS_MAX_OUTPUT_QUEUES;
 	num_queues = fwctx->num_input_queues + fwctx->num_output_queues;
 
-	queue_configs = devm_kcalloc(dev, num_queues, sizeof(*queue_configs),
-				     GFP_KERNEL);
+	queue_configs = kzalloc_objs(*queue_configs, num_queues);
 	if (!queue_configs) {
-		ipu7_fw_isys_cleanup(isys);
+		kfree(fwctx);
 		return -ENOMEM;
 	}
 	fwctx->fw_entry = adev->fw_entry;

drivers/media/pci/intel/ipu6/ipu7-fw-isys.c static int ipu7_fw_isys_init(struct ipu6_isys *isys, unsigned int num_streams) · line 115

Quoted 6 lines

drivers/media/pci/intel/ipu6/ipu7-fw-isys.c +2 -1

@@ -111,7 +111,8 @@ static int ipu7_fw_isys_init(struct ipu6_isys *isys, unsigned int num_streams)
 				   &fw_config_dma_addr, GFP_KERNEL, 0);
 	if (!fw_config) {
 		dev_err(dev, "Failed to allocate isys subsys config.\n");
-		ipu7_fw_isys_cleanup(isys);
+		kfree(queue_configs);
+		kfree(fwctx);

Instead of releasing the memory here, could you add a few labels for error handling?

Doesn't ipu6_isys_fwcom_cfg_init() suffer from the same?

I think it could also make sense to keep these allocations around until the device is unbound, but it's best to fix the issues first in any case.

Quoted 3 lines

drivers/media/pci/intel/ipu6/ipu7-fw-isys.c +0 -0
 		return -ENOMEM;
 	}
 	fwctx->fw_config = fw_config;

Signature · 3 lines

Regards,

Sakari Ailus