mirror of
https://github.com/torvalds/linux.git
synced 2026-09-23 22:14:03 +02:00
media: ti: vpe: quiesce overflow recovery before freeing streams
The VIP overflow recovery worker is armed from the hardirq handler when a
FIFO overflow is detected, and the list-complete path looks the stream up
through the VPDMA list private pointer. Both keep touching stream, port
and device state; the recovery worker also resets the parser and VPDMA,
repopulates the descriptor list, and re-enables the per-list IRQs.
vip_stop_streaming() masks and clears the per-list IRQs, but it neither
synchronizes the hardirq handler nor disables recovery_work. An overflow
IRQ that has already queued recovery_work, or a list-complete IRQ in
flight when the stream is torn down, can therefore still dereference the
stream after its resources are released: the descriptor list is freed by
vip_release_stream() on file release, and the stream itself by
free_stream() on unbind/remove.
Drain the recovery worker and the IRQ handler at both teardown points
through a shared vip_quiesce_stream() helper, before any stream-owned
resource is released. disable_work_sync() cancels pending recovery_work,
drains a running instance, and raises its disable depth, so a subsequent
schedule_work() issued by a racing IRQ handler is rejected at the
workqueue scheduler: recovery_work cannot be requeued after
disable_work_sync() takes effect. The worker may still re-enable the
per-list IRQs before disable_work_sync() returns; disable_irqs() then
masks those sources and synchronize_irq() waits for any in-flight handler
that still dereferences stream state. In vip_stop_streaming() the helper
runs before the parser is stopped, since a worker drained by
disable_work_sync() may re-enable the parser before exiting and would
otherwise undo the stop. recovery_work is created disabled and enabled in
vip_start_streaming() before IRQs, pairing the enable with the teardown
disable across the streaming lifecycle.
This issue was found by an in-house static analysis tool and confirmed
by manual code review.
Fixes: fc2873aa4a ("media: ti: vpe: Add the VIP driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
Reviewed-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
Tested-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
This commit is contained in:
parent
bbd4218310
commit
aeaacc3001
|
|
@ -815,6 +815,22 @@ static void clear_irqs(struct vip_dev *dev, int irq_num, int list_num)
|
|||
vpdma_clear_list_stat(dev->shared->vpdma, irq_num, dev->slice_id);
|
||||
}
|
||||
|
||||
/*
|
||||
* Quiesce recovery work and per-list IRQs before releasing stream resources.
|
||||
* disable_work_sync() prevents the overflow handler from requeueing recovery
|
||||
* work. Mask and synchronize IRQs afterwards because a running worker may
|
||||
* have re-enabled them before exiting.
|
||||
*/
|
||||
static void vip_quiesce_stream(struct vip_stream *stream)
|
||||
{
|
||||
struct vip_dev *dev = stream->port->dev;
|
||||
|
||||
disable_work_sync(&stream->recovery_work);
|
||||
disable_irqs(dev, dev->slice_id, stream->list_num);
|
||||
clear_irqs(dev, dev->slice_id, stream->list_num);
|
||||
synchronize_irq(dev->irq);
|
||||
}
|
||||
|
||||
static void populate_desc_list(struct vip_stream *stream)
|
||||
{
|
||||
struct vip_port *port = stream->port;
|
||||
|
|
@ -2429,6 +2445,7 @@ static int vip_start_streaming(struct vb2_queue *vq, unsigned int count)
|
|||
goto err;
|
||||
|
||||
stream->num_recovery = 0;
|
||||
enable_work(&stream->recovery_work);
|
||||
|
||||
clear_irqs(dev, dev->slice_id, stream->list_num);
|
||||
enable_irqs(dev, dev->slice_id, stream->list_num);
|
||||
|
|
@ -2453,13 +2470,17 @@ static void vip_stop_streaming(struct vb2_queue *vq)
|
|||
struct vip_dev *dev = port->dev;
|
||||
int ret;
|
||||
|
||||
/*
|
||||
* A running recovery worker may re-enable the parser, so quiesce it
|
||||
* and its IRQ handler before stopping the parser or releasing the
|
||||
* descriptor list.
|
||||
*/
|
||||
vip_quiesce_stream(stream);
|
||||
|
||||
vip_parser_stop_imm(port, true);
|
||||
vip_enable_parser(port, false);
|
||||
unset_fmt_params(stream);
|
||||
|
||||
disable_irqs(dev, dev->slice_id, stream->list_num);
|
||||
clear_irqs(dev, dev->slice_id, stream->list_num);
|
||||
|
||||
if (port->subdev) {
|
||||
ret = v4l2_subdev_call(port->subdev, video, s_stream, 0);
|
||||
if (ret)
|
||||
|
|
@ -3075,6 +3096,8 @@ static int alloc_stream(struct vip_port *port, int stream_id, int vfl_type)
|
|||
goto do_free_hwlist;
|
||||
|
||||
INIT_WORK(&stream->recovery_work, vip_overflow_recovery_work);
|
||||
/* Start disabled; vip_start_streaming() enables it before IRQs. */
|
||||
disable_work(&stream->recovery_work);
|
||||
|
||||
INIT_LIST_HEAD(&stream->vidq);
|
||||
|
||||
|
|
@ -3140,6 +3163,13 @@ static void free_stream(struct vip_stream *stream)
|
|||
return;
|
||||
|
||||
dev = stream->port->dev;
|
||||
/*
|
||||
* Quiesce the IRQ handler and recovery worker, then drop the stream
|
||||
* from cap_streams[], before releasing stream-owned resources.
|
||||
*/
|
||||
vip_quiesce_stream(stream);
|
||||
stream->port->cap_streams[stream->stream_id] = NULL;
|
||||
|
||||
/* Free up the Drop queue */
|
||||
list_for_each_safe(pos, q, &stream->dropq) {
|
||||
buf = list_entry(pos,
|
||||
|
|
@ -3151,7 +3181,6 @@ static void free_stream(struct vip_stream *stream)
|
|||
|
||||
video_unregister_device(stream->vfd);
|
||||
vpdma_hwlist_release(dev->shared->vpdma, stream->list_num);
|
||||
stream->port->cap_streams[stream->stream_id] = NULL;
|
||||
kfree(stream);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user