From 1506a5efe815e8cfbb578af657952800100b3281 Mon Sep 17 00:00:00 2001 From: Naushir Patuck Date: Wed, 13 Aug 2025 12:09:08 +0100 Subject: [PATCH 1/2] drivers: media: pisp_be: Fix for job queue removal in stop_streaming() The existing code unconditionally removes jobs from the job_queue list when all the nodes in a node group have stopped streaming. This will also remove jobs for any other node groups as the job_queue is a common list. Fix this by only conditionally deleting jobs that belong to the current node group. Additionally, delete jobs as soon as the first node in the node group stops streaming. Running a job with an incomplete set of active nodes is invalid. Fixes: 880153e0d0a9 ("media: pisp_be: Split jobs creation and scheduling") Signed-off-by: Naushir Patuck --- .../platform/raspberrypi/pisp_be/pisp_be.c | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c index 7dce1f83de4b18..fbbecc3eb8e41c 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c @@ -941,7 +941,6 @@ static void pispbe_node_stop_streaming(struct vb2_queue *q) struct pispbe_dev *pispbe = node_group->pispbe; struct pispbe_job_descriptor *job, *temp; struct pispbe_buffer *buf; - LIST_HEAD(tmp_list); /* * Now this is a bit awkward. In a simple M2M device we could just wait @@ -968,20 +967,18 @@ static void pispbe_node_stop_streaming(struct vb2_queue *q) spin_lock_irq(&pispbe->hw_lock); node_group->streaming_map &= ~BIT(node->id); - if (node_group->streaming_map == 0) { - /* - * If all nodes have stopped streaming release all jobs - * without holding the lock. - */ - list_splice_init(&pispbe->job_queue, &tmp_list); + /* + * If a node has stopped streaming release all jobs belonging to the + * node group immediately. + */ + list_for_each_entry_safe(job, temp, &pispbe->job_queue, queue) { + if (job->node_group == node->node_group) { + list_del(&job->queue); + kfree(job); + } } spin_unlock_irq(&pispbe->hw_lock); - list_for_each_entry_safe(job, temp, &tmp_list, queue) { - list_del(&job->queue); - kfree(job); - } - pm_runtime_mark_last_busy(pispbe->dev); pm_runtime_put_autosuspend(pispbe->dev); From 3e4f99e060ba49deb3d2b0202d288e0ee9366556 Mon Sep 17 00:00:00 2001 From: Naushir Patuck Date: Thu, 14 Aug 2025 07:48:56 +0100 Subject: [PATCH 2/2] drivers: media: pisp_be: Fix use after free in job queue logic pispbe_schedule() currently takes a node group as a parameter, which is left over from before the job prepare/scheduling refactoring. This is now invalid, as jobs are executed in the order which they were queued. As part of this old code, there was a check if the current node group mached the node group of the job, and if unmathched, use a "continue" statement. This is invalid as there is no loop to iterate over any more. The reason this was not a compile bug is because of the for loop used as part of the scoped_guard macro. A consequence of breaking out of the scoped_guard loop early is that the job structure gets freed, but not actually removed from the queue and may be accessed after freeing. Fix this by removing the node group test in pispbe_schedule() as it is no longer valid to use. Signed-off-by: Naushir Patuck --- .../media/platform/raspberrypi/pisp_be/pisp_be.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c index fbbecc3eb8e41c..2db4211b2b35a1 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c @@ -594,9 +594,7 @@ static int pispbe_prepare_job(struct pispbe_node_group *node_group) return -ENODEV; } -static void pispbe_schedule(struct pispbe_dev *pispbe, - struct pispbe_node_group *node_group, - bool clear_hw_busy) +static void pispbe_schedule(struct pispbe_dev *pispbe, bool clear_hw_busy) { struct pispbe_job_descriptor *job; @@ -613,9 +611,6 @@ static void pispbe_schedule(struct pispbe_dev *pispbe, if (!job) return; - if (node_group && job->node_group != node_group) - continue; - list_del(&job->queue); for (unsigned int i = 0; i < PISPBE_NUM_NODES; i++) @@ -703,7 +698,7 @@ static irqreturn_t pispbe_isr(int irq, void *dev) } /* check if there's more to do before going to sleep */ - pispbe_schedule(pispbe, NULL, can_queue_another); + pispbe_schedule(pispbe, can_queue_another); return IRQ_HANDLED; } @@ -894,7 +889,7 @@ static void pispbe_node_buffer_queue(struct vb2_buffer *buf) * to do, but only for this client. */ if (!pispbe_prepare_job(node_group)) - pispbe_schedule(pispbe, node_group, false); + pispbe_schedule(pispbe, false); } static int pispbe_node_start_streaming(struct vb2_queue *q, unsigned int count) @@ -921,7 +916,7 @@ static int pispbe_node_start_streaming(struct vb2_queue *q, unsigned int count) /* Maybe we're ready to run. */ if (!pispbe_prepare_job(node_group)) - pispbe_schedule(pispbe, node_group, false); + pispbe_schedule(pispbe, false); return 0;