media: meson: vdec: Fix lifecycles, race conditions, and stability bugs

drivers/staging/media/meson/vdec/vdec.c static void vdec_remove(struct platform_device *pdev) · line 1109

[Severity: High]
Does releasing the m2m device here introduce a use-after-free if a file descriptor remains open?

If a user unbinds the driver, vdec_remove() synchronously frees core->m2m_dev. However, if an application still holds a file descriptor open, it will eventually close it, triggering vdec_close():

vdec_close()
v4l2_m2m_ctx_release(sess->m2m_ctx)

    v4l2_m2m_cancel_job()
      spin_lock_irqsave(&m2m_dev->job_spinlock, flags);

Since core->m2m_dev was already freed during unbind, could this dereference cause memory corruption or a crash?

[Severity: High]
This is a pre-existing issue, but does returning from vdec_remove() without waiting for open file descriptors to close lead to a use-after-free of the core driver structure?

The core structure and its mutex are allocated via devres (devm_kzalloc) during vdec_probe(). If the device is unbound, devres will automatically free the core structure as soon as vdec_remove() returns.

If an ioctl is currently blocked waiting on vdev->lock (which points to core->lock), it will attempt to unlock a freed mutex when it resumes.

Additionally, if the asynchronous esparser_queue_work runs after unbind, it will dereference the freed sess->core.

Could the driver synchronize active sessions or use a different lifecycle for the core structure to prevent this?

drivers/staging/media/meson/vdec/vdec.c static int vdec_start_streaming(struct vb2_queue *q, unsigned int count) · line 318

[Severity: High]
If we jump to err_unlock_no_hw here, doesn't it bypass the err_cleanup_session label that clears the streamon flags?

If session B calls VIDIOC_STREAMON on its OUTPUT queue and hits this path, sess->streamon_out will remain 1.

Could a subsequent VIDIOC_QBUF from session B cause vdec_vb2_buf_queue to schedule esparser_queue_work, potentially overwriting the active hardware registers of the current session?

[ ... ]

drivers/staging/media/meson/vdec/vdec.c static int vdec_start_streaming(struct vb2_queue *q, unsigned int count) · line 358

[Severity: High]
This is a pre-existing issue, but does this code properly handle thread creation failure?

If kthread_run fails due to memory constraints, it returns an ERR_PTR. Later in vdec_stop_streaming, this pointer is passed unconditionally to kthread_stop.

Could this cause a kernel panic when kthread_stop attempts to dereference the ERR_PTR?

drivers/staging/media/meson/vdec/vdec.c static int vdec_start_streaming(struct vb2_queue *q, unsigned int count) · line 361

[Severity: High]
This is a pre-existing issue, but does closing the file descriptor safely clean up the workqueue scheduled here?

When the client queues a buffer and then closes the file descriptor, vdec_close destroys mutexes and frees the amvdec_session memory via kfree without cancelling the pending esparser_queue_work.

When the workqueue executes vdec_esparser_queue_work, could it access the freed session memory and attempt to take a destroyed mutex?

[ ... ]

drivers/staging/media/meson/vdec/vdec.c static void vdec_stop_streaming(struct vb2_queue *q) · line 470

[Severity: High]
If a userspace client starts streaming on both queues and then calls VIDIOC_STREAMOFF on the CAPTURE queue, the newly added condition evaluating q->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE or !sess->streamon_out will evaluate to false, skipping kthread_stop.

Since full_cleanup is true, the code proceeds to tear down resources via vdec_reset_bufs_recycle and kfree of sess->priv.

Could this lead to a use-after-free or list corruption if the recycle thread continues executing while its backing resources are freed?

drivers/staging/media/meson/vdec/vdec.c static void vdec_stop_streaming(struct vb2_queue *q) · line 480

[Severity: High]
This is a pre-existing issue, but does vdec_reset_timestamps safely access the timestamps list here?

The function iterates and frees elements of the sess->timestamps list without holding ts_spinlock.

If a concurrent threaded ISR executes amvdec_dst_buf_done_idx and calls amvdec_remove_ts, it will acquire ts_spinlock and attempt to remove an element.

Could this race condition lead to linked list corruption and use-after-free?

Review

[Severity: Medium]
This is a pre-existing issue, but does the err_poweroff unwinding path leak sess->priv?

If kthread_run() fails, execution jumps to the err_poweroff label which unwinds various hardware states but misses calling kfree(sess->priv). When the application retries VIDIOC_STREAMON, the codec's start routine unconditionally allocates a new structure over sess->priv, permanently stranding the previous allocation.

Review

[ ... ]
@@ -464,7 +478,10 @@ static void vdec_stop_streaming(struct vb2_queue *q)

Review

[Severity: High]
This is a pre-existing issue, but does this logic orphan the recycle thread leading to a use-after-free during partial STREAMOFF?

If userspace invokes VIDIOC_STREAMOFF exclusively on the capture queue while the output queue remains active, this condition evaluates to false, bypassing the kthread_stop() call.

Because old_status == STATUS_RUNNING, full_cleanup evaluates to true, proceeding to dismantle the session context and free sess->priv. The orphaned recycle thread may concurrently awaken and attempt to access this freed memory.

Review

[ ... ]
@@ -970,6 +987,31 @@ static int vdec_open(struct file *file)

Review

[Severity: Critical]
Can this unconditional invocation of vdec_poweroff() and clearing of core->cur_sess cause Data Abort panics and cross-session corruption?

If userspace opens two instances (FDs), starts streaming on instance A (acquiring hardware ownership), and then closes instance B:

vdec_close() for instance B will unconditionally call vdec_poweroff(sess) and clear core->cur_sess, without verifying if instance B actually owns the hardware.

This forces clock disablements while instance A is actively decoding. When the hardware for instance A fires an interrupt, vdec_isr() will dereference core->cur_sess, which was just wrongfully set to NULL, causing a panic.

[Severity: Critical]
Will this unconditional vdec_poweroff() call also lead to a double free of codec DMA allocations?

If userspace cleanly shuts down streaming via VIDIOC_STREAMOFF, vdec_stop_streaming() executes vdec_poweroff(). This invokes the codec stop callback (like codec_h264_stop) which frees memory buffers but does not set their pointers to NULL.

When userspace subsequently calls close(), vdec_close() calls vdec_poweroff() again. The codec stop routine will execute a second time, passing the same non-NULL pointers to dma_free_coherent() resulting in a catastrophic double-free.

Review

[Severity: High]
Should we call cancel_work_sync(&sess->esparser_queue_work) before freeing the session context and DMA buffers here?

If vdec_close() is called while the background workqueue is actively processing frames, the DMA buffer (sess->vififo_vaddr) and session structure (sess->priv) are immediately freed.

The concurrently executing workqueue continues its operation inside esparser_queue(), dereferencing the freed sess->priv pointer and instructing the hardware to perform DMA transfers into the freshly freed physical address, potentially causing severe memory corruption.

Review

[Severity: Critical]
Does this sequence trigger a NULL pointer dereference during queue teardown?

If a user closes the file descriptor without calling VIDIOC_STREAMOFF, vdec_close() will free sess->priv and set it to NULL.

Immediately after, it calls v4l2_m2m_ctx_release(), which triggers vdec_stop_streaming(). Since sess->status is still STATUS_RUNNING, the driver will execute codec_ops->drain(sess).

The codec's drain function unconditionally dereferences sess->priv, which was just set to NULL, causing an immediate panic.

drivers/staging/media/meson/vdec/vdec.c static void vdec_stop_streaming(struct vb2_queue *q) · line 489

[Severity: High]
This isn't a bug introduced by this patch, but does calling synchronize_irq() before vdec_poweroff() actually prevent the race condition?

Since synchronize_irq() only waits for currently executing handlers to finish and does not disable the IRQ line or prevent new hardware interrupts, it seems new interrupts can still fire immediately after it returns.

Because the hardware isn't stopped until vdec_poweroff() runs, could a newly triggered ISR observe a valid session pointer and execute concurrently with the rest of the teardown sequence?

Should the hardware be stopped via vdec_poweroff() before synchronizing IRQs?

drivers/staging/media/meson/vdec/vdec.c static int vdec_close(struct file *file) · line 997

[Severity: High]
This is a pre-existing issue, but does the driver handle unbind safely if user-space keeps the video device file open?

If the driver is unbound, vdec_remove() executes:

vdec_remove()

    video_unregister_device(core->vdev_dec);
    v4l2_m2m_release(core->m2m_dev);
    v4l2_device_unregister(&core->v4l2_dev);

Since devres automatically frees the amvdec_core structure upon unbind completion, and video_unregister_device() does not forcefully close open file descriptors, could a later call to vdec_close() dereference the freed core structure?

For instance, this newly added synchronize_irq(core->vdec_irq) call would use the freed core pointer. Similarly, v4l2_m2m_ctx_release(sess->m2m_ctx) later in vdec_close() would use the freed core->m2m_dev.

drivers/staging/media/meson/vdec/vdec.c static const struct v4l2_file_operations vdec_fops = { · line 1052

[Severity: High]
Can returning IRQ_HANDLED here cause an infinite interrupt storm?

If the handler evaluates the session to NULL and returns IRQ_HANDLED without acknowledging or clearing the interrupt at the hardware level, the generic IRQ framework will unmask the interrupt line.

Since the hardware interrupt source hasn't been cleared, won't the interrupt controller immediately re-trigger the interrupt, resulting in a soft lockup or CPU stall?

Should this return IRQ_NONE so the IRQ subsystem can detect it as spurious and eventually disable the IRQ line?

drivers/staging/media/meson/vdec/vdec.c static irqreturn_t vdec_isr(int irq, void *data) · line 1067

[Severity: High]
Does returning IRQ_HANDLED here also incorrectly propagate the same interrupt storm behavior?

drivers/staging/media/meson/vdec/esparser.c static irqreturn_t esparser_isr(int irq, void *dev) · line 68

[Severity: High]
Is it safe to return IRQ_HANDLED here without interacting with the hardware to clear the interrupt?

Because esparser_irq is registered with IRQF_SHARED, returning IRQ_HANDLED when cur_sess is NULL falsely tells the IRQ subsystem that this driver handled the interrupt.

Will this cause an interrupt storm for level-triggered interrupts, or break spurious interrupt detection for other devices sharing this IRQ line?

drivers/staging/media/meson/vdec/vdec.c static void vdec_stop_streaming(struct vb2_queue *q) · line 493

[Severity: High]
This is a pre-existing issue, but does this synchronization logic fully close the race condition?

The synchronize_irq() call waits for in-flight ISRs, but the interrupt line remains enabled. Later in this teardown path:

vdec_stop_streaming()

    vdec_poweroff(sess);
    ...
    smp_store_release(&core->cur_sess, NULL);

If the interrupt fires after vdec_poweroff() completes, but before cur_sess is set to NULL, the ISR will pass the !sess check and attempt to read PARSER_INT_STATUS from unclocked hardware.

Could this lead to a synchronous bus fault and kernel panic?

[ ... ]

drivers/staging/media/meson/vdec/vdec.c static int vdec_close(struct file *file) · line 1003

[Severity: High]
This is a pre-existing issue, but does vdec_close() unconditionally call vdec_poweroff() even if the device was just opened without starting streaming?

If vdec_open() is called and immediately followed by vdec_close(), calling vdec_poweroff() unconditionally would execute clk_disable_unprepare() on dos_clk and dos_parser_clk which were never enabled.

Can this cause clock framework underflow warnings and potentially disable clocks for other hardware sharing the same clock tree?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5

drivers/staging/media/meson/vdec/vdec.c static void vdec_stop_streaming(struct vb2_queue *q) · line 493

[Severity: High]
Can this cause a deadlock?

When userspace calls the streamoff ioctl, the v4l2 core acquires the queue lock (&sess->lock) before calling this vdec_stop_streaming() callback. Since cancel_work_sync() waits for the worker thread to finish, and the worker esparser_queue_all_src() attempts to acquire mutex_lock(&sess->lock), won't these threads wait on each other indefinitely if the worker is already running?

[Severity: High]
Could this teardown ordering lead to a use-after-free?

Because cancel_work_sync() and synchronize_irq() are called before vdec_poweroff() disables the hardware, the hardware is still active and can generate interrupts. An interrupt could fire immediately after cancel_work_sync() returns, causing dst_buf_done() to schedule the work again. If the sess structure is subsequently freed, won't the newly scheduled work dereference a freed pointer?

drivers/staging/media/meson/vdec/vdec.c static int vdec_close(struct file *file) · line 1006

[Severity: High]
Does this have the same teardown ordering race as vdec_stop_streaming()?

The cancel_work_sync() call happens before vdec_poweroff(sess). If the hardware generates a late interrupt, could the work be scheduled again right before the session is freed?

drivers/staging/media/meson/vdec/vdec.c static int vdec_close(struct file *file) · line 1010

Later in vdec_close(), the hardware is powered down and the session is cleared:

    mutex_lock(&core->lock);

    vdec_poweroff(sess);
    vdec_free_canvas(sess);
    core->cur_sess = NULL;

[Severity: High]
Does this unconditionally terminate the hardware state of the active session?

Because this lacks an if (core->cur_sess == sess) check (which is safely used in vdec_stop_streaming), if a secondary client opens and immediately closes the device node, won't it forcefully power off the hardware and clear the session while the primary client is actively streaming?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6

drivers/staging/media/meson/vdec/esparser.c esparser_queue(struct amvdec_session *sess, struct vb2_v4l2_buffer *vbuf) · line 352

[Severity: Critical]
This is a pre-existing issue, but can maliciously crafted VP9 superframe headers cause an integer overflow in vp9_update_header(), leading to a massive out-of-bounds memory copy?

Looking at vp9_update_header() in the same file:

    for (cur_mag = 0; cur_mag < mag; cur_mag++) {
        ...
        frame_size[cur_frame] |=
            (dp[mag_ptr] << (cur_mag * 8));
        mag_ptr++;
    }
    ...
    total_datasize += frame_size[cur_frame];

If untrusted user data shifted into the signed 32-bit frame_size becomes negative, multiple negative values can sum to a small positive total_datasize, which bypasses the size checks:

    new_frame_size = total_datasize + num_frames * VP9_HEADER_SIZE;
    if (new_frame_size >= vb2_plane_size(buf, 0)) {
        return -ENOMEM;
    }

Then, later in the same function:

    int framesize = frame_size[cur_frame];
    ...
    memmove(fdata + VP9_HEADER_SIZE, old_framedata, framesize);

The negative framesize is implicitly converted to a size_t, which causes an out-of-bounds memmove(). Could this lead to memory corruption when handling untrusted V4L2 video stream data?

[ ... ]

drivers/staging/media/meson/vdec/esparser.c esparser_queue(struct amvdec_session *sess, struct vb2_v4l2_buffer *vbuf) · line 417

[Severity: High]
Does this code inadvertently create an infinite loop?

Because scoped_guard() is implemented internally as a for-loop, the break statements inside the scoped_guard() block will only exit the lock scope, not the outer while (1) loop.

If the queue is empty (!vbuf), we break out of the scoped_guard(), processed_frame remains false, and the while (1) loop continues infinitely, consuming 100% CPU.

Conversely, if a frame is successfully processed, processed_frame becomes true, which causes the outer while (1) loop to exit after just one frame instead of continuing to drain the queue.

Could the logic here be inverted, or should the inner loop use goto instead of break to exit the outer loop?

drivers/staging/media/meson/vdec/esparser.c void esparser_queue_all_src(struct work_struct *work) · line 383

[Severity: High]
Could the workqueue get stuck in an infinite loop due to how scoped_guard expands?

Because scoped_guard(mutex, &sess->lock) expands to a for loop, if the inner code hits an empty queue or -EAGAIN and calls break, it will only exit the scoped_guard loop, leaving processed_frame as false.

The code then evaluates if (processed_frame) break; which fails, executes cond_resched(), and loops the outer while (1) infinitely.

Conversely, if a frame is successfully processed (processed_frame is true), does the loop break entirely, causing the workqueue to exit after processing just a single frame?

[ ... ]

drivers/staging/media/meson/vdec/vdec.c static int vdec_start_streaming(struct vb2_queue *q, unsigned int count) · line 402

[Severity: Medium]
Is there a memory leak of sess->priv on the error paths in vdec_start_streaming()?

If vdec_poweron() succeeds (which dynamically allocates sess->priv), but a later step like kthread_run() fails, the code jumps to err_poweroff. While the hardware state and vififo_vaddr are rolled back, kfree(sess->priv) appears to be missing.

If VIDIOC_STREAMON repeatedly fails, could this leak kernel memory?

[ ... ]

drivers/staging/media/meson/vdec/vdec.c static void vdec_stop_streaming(struct vb2_queue *q) · line 502

[Severity: High]
Can this cause an AB-BA deadlock between the V4L2 core and the workqueue?

When userspace calls VIDIOC_STREAMOFF, the V4L2 core acquires the queue lock (sess->lock), then calls vdec_stop_streaming(), which calls cancel_work_sync() here.

Meanwhile, the esparser_queue_all_src() workqueue loop uses scoped_guard(mutex, &sess->lock) to acquire the exact same lock. If the workqueue is active, it will block waiting for sess->lock, while vdec_stop_streaming() blocks indefinitely waiting for the workqueue to exit.

drivers/staging/media/meson/vdec/vdec.c static int vdec_close(struct file *file) · line 1057

[Severity: Critical]
Does this unconditionally clear the hardware ownership, even if this session doesn't own it?

If session A is streaming and session B is closed, vdec_close() for session B will execute smp_store_release(&core->cur_sess, NULL) outside of the if (core->cur_sess == sess) check.

If core->cur_sess becomes NULL while session A's hardware is active, could the next hardware interrupt trigger vdec_isr() which sees !core->cur_sess and returns IRQ_HANDLED without clearing the hardware interrupt status?

Since the interrupt is level-triggered, could this result in an immediate interrupt storm that hangs the CPU, and permanently leak session A's vififo_vaddr DMA buffers when session A is eventually closed?

drivers/staging/media/meson/vdec/esparser.c static int vp9_update_header(struct amvdec_core *core, struct vb2_buffer *buf) · line 101

[Severity: Critical]
Does returning a negative error code here cause an issue in the caller?

Looking at esparser_queue(), it assigns the return value of vp9_update_header() to payload_size and checks if (payload_size == 0). If this returns -EINVAL and the caller assigns the result to a u32 variable, could the negative error code underflow to a massive positive integer (like 4294967274)?

This might completely bypass validation and be passed to esparser_pad_start_code(), causing an out-of-bounds memset and out-of-bounds DMA commands to the hardware.

drivers/staging/media/meson/vdec/esparser.c static int vp9_update_header(struct amvdec_core *core, struct vb2_buffer *buf) · line 127

[Severity: Critical]
This isn't a bug introduced by this patch, but does this shift operation cause an implicit sign extension?

Because dp is a pointer to u8, the byte at dp[mag_ptr] undergoes integer promotion to a signed int before shifting. If cur_mag is 3 and the byte is 128 or larger, shifting it by 24 bits produces a negative value due to the sign bit.

Could this allow crafting combinations of large positive and negative frame_size values so their sum wraps around to a small positive integer, bypassing the bounds check later on and causing a massive out-of-bounds memmove()?

drivers/staging/media/meson/vdec/esparser.c static int vp9_update_header(struct amvdec_core *core, struct vb2_buffer *buf) · line 151

[Severity: High]
This isn't a bug introduced by this patch, but does validating against the total buffer capacity instead of the actual payload size allow information disclosure?

If the check uses vb2_plane_size(buf, 0) instead of the actual received payload length in dsize, could a valid short payload with a crafted superframe index declare frame sizes that exceed the payload length but remain under the total buffer capacity?

If so, memmove() could copy trailing uninitialized or stale kernel memory from the buffer into the active payload stream.

Outside of the immediate diff, there are a few other questions about the esparser implementation that may be worth reviewing:

[Severity: High]
This isn't a bug introduced by this patch, but does esparser_queue_all_src() queue hardware operations without acquiring the global hardware lock?

The worker runs concurrently for multiple sessions because the work queue is initialized per-session. The worker locks the per-session sess->lock but does not appear to acquire the global core->lock or verify hardware ownership before calling esparser_write_data():

esparser_queue_all_src() {
    ...
    ret = esparser_queue(sess, vbuf);
    ...
}

Could multiple sessions invoke esparser_write_data() simultaneously, causing conflicting writes to the hardware parser registers and corruption of the global search_done waitqueue flag?

[Severity: High]
Could the early return in esparser_isr() bypass clearing the hardware interrupt?

The interrupt service routine starts with a check:

esparser_isr() {
    if (!sess)
        return IRQ_HANDLED;
    ...
}

If a hardware interrupt asserts just as the session is torn down, returning early leaves the hardware interrupt un-cleared. Would this cause the hardware to hold the interrupt line asserted indefinitely, leading to an unrecoverable IRQ storm and system lockup?

[Severity: High]
Does the break statement inside esparser_queue_all_src() properly exit the worker loop?

When checking if a buffer is available inside a scoped_guard():

esparser_queue_all_src() {
    ...
    vbuf = v4l2_m2m_next_src_buf(sess->m2m_ctx);
    if (!vbuf)
        break;
    ...
}

Since scoped_guard() is implemented internally as a for loop, breaking here would only escape the guard's inner loop. If processed_frame remains false, the outer while (1) loop proceeds past its own exit condition:

    if (processed_frame)
        break;
    /* Give other threads and IRQ routines a window ... */
    cond_resched();

Could this pin a CPU core at 100% in an infinite busy-wait loop when the queue is empty?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9

drivers/staging/media/meson/vdec/esparser.c static int vp9_update_header(struct amvdec_core *core, struct vb2_buffer *buf) · line 210

[Severity: High]
Does returning 0 here skip the start code injection and cause an out-of-bounds DMA scan?

By bypassing the injection of the SEARCH_PATTERN start code, the caller esparser_queue() might interpret the 0 as success and instruct the DMA engine to fetch the buffer.

Since the search pattern is missing, could the hardware parser scan indefinitely past the valid buffer into adjacent physical memory, resulting in an out-of-bounds DMA read?

drivers/staging/media/meson/vdec/esparser.c static int vp9_update_header(struct amvdec_core *core, struct vb2_buffer *buf)

[Severity: High]
This is a pre-existing issue, but does this memset() write out of bounds?

If a small buffer is queued, this calculation executes before the overall bounds check is evaluated.

Could this overwrite adjacent kernel memory with zeroes before validating the size?

[ ... ]

drivers/staging/media/meson/vdec/esparser.c esparser_queue(struct amvdec_session *sess, struct vb2_v4l2_buffer *vbuf) · line 359

[Severity: Critical]
This is a pre-existing issue, but does vp9_update_header() have an integer overflow?

By extracting untrusted payload bytes and shifting them left up to 24 bits into a signed integer, can malicious payloads construct negative frame sizes?

If a highly negative signed integer is produced and passed to memmove() as an unsigned size_t, could this cause massive out-of-bounds reads and writes on the kernel heap or DMA memory?

drivers/staging/media/meson/vdec/esparser.c esparser_queue(struct amvdec_session *sess, struct vb2_v4l2_buffer *vbuf) · line 374

[Severity: High]
Does this new mutex_lock() introduce an AB-BA lock inversion?

The background workqueue thread esparser_queue_all_src() acquires sess->lock first (via scoped_guard) and then calls esparser_queue().

By adding mutex_lock(&core->lock) here, could this conflict with V4L2 ioctls that implicitly acquire core->lock before sess->lock, resulting in an AB-BA deadlock?

drivers/staging/media/meson/vdec/esparser.c esparser_queue(struct amvdec_session *sess, struct vb2_v4l2_buffer *vbuf) · line 381

[Severity: High]
Could sleeping inside esparser_write_data() block all other V4L2 ioctls?

Because core->lock is mapped to vdev->lock, sleeping via wait_event_interruptible_timeout() while waiting for the hardware interrupt blocks all other V4L2 ioctls across the entire device for the duration of the fetch.

Can this severely degrade system performance?

drivers/staging/media/meson/vdec/esparser.c esparser_queue(struct amvdec_session *sess, struct vb2_v4l2_buffer *vbuf) · line 393

I also have some questions about related code paths outside the diff:

[Severity: High]
Is there an infinite loop in esparser_queue_all_src()?

When looking at the worker thread:

drivers/staging/media/meson/vdec/esparser.c:esparser_queue_all_src() {
    ...
		bool processed_frame = false;
        ...
		if (processed_frame)
			break;
    ...
}

Because the break statements inside the scoped_guard macro only exit the internal macro loop, processed_frame remains false when the queue is empty.

Does the inverted logic cause the thread to bypass the outer break and loop infinitely, soft-locking the CPU?

[Severity: High]
Do vdec_start_streaming() and vdec_stop_streaming() self-deadlock?

In the current code:

drivers/staging/media/meson/vdec/vdec.c:vdec_start_streaming() {
    ...
	mutex_lock(&core->lock);
    ...
}

Because the device's vdev->lock is mapped directly to &core->lock, the V4L2 core framework automatically acquires it before dispatching standard ioctls.

Does explicitly calling mutex_lock(&core->lock) inside these callbacks result in a permanent self-deadlock?

[Severity: High]
Is there an AB-BA deadlock during stream teardown?

During streamoff:

drivers/staging/media/meson/vdec/vdec.c:vdec_stop_streaming() {
    ...
		cancel_work_sync(&sess->esparser_queue_work);
    ...
}

The ioctl thread acquires sess->lock before invoking this callback. Concurrently, the background workqueue thread esparser_queue_all_src() blocks waiting for sess->lock.

Does calling cancel_work_sync() here cause the ioctl thread to block forever waiting for the worker thread, resulting in an AB-BA deadlock?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10

drivers/staging/media/meson/vdec/vdec.c static int vdec_start_streaming(struct vb2_queue *q, unsigned int count) · line 305

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in vdec_start_streaming() mistakenly strand buffers if one queue fails to start?

If the CAPTURE queue starts successfully, but the OUTPUT queue fails to start due to the hardware being busy, the error path flushes both queues:

drivers/staging/media/meson/vdec/vdec.c:vdec_start_streaming() {
    ...
err_unlock_no_hw:
	while ((buf = v4l2_m2m_src_buf_remove(sess->m2m_ctx)))
		v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
	while ((buf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx)))
		v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
    ...
}

Because the CAPTURE queue is actively streaming, wouldn't returning these buffers with VB2_BUF_STATE_QUEUED cause the vb2 core to leave them in the queued list without putting them on the done list, permanently freezing the userspace application?

[ ... ]

drivers/staging/media/meson/vdec/vdec.c static void vdec_stop_streaming(struct vb2_queue *q) · line 537

[Severity: High]
This is a pre-existing issue, but does vdec_stop_streaming() return active DMA buffers to the videobuf2 framework before the hardware is stopped?

When flushing buffers, it immediately returns them to the V4L2 core:

drivers/staging/media/meson/vdec/vdec.c:vdec_stop_streaming() {
    ...
	/* flush buffers to kill background workqueue thread */
	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
		while ((buf = v4l2_m2m_src_buf_remove(sess->m2m_ctx)))
			v4l2_m2m_buf_done(buf, VB2_BUF_STATE_ERROR);
		sess->streamon_out = 0;
	} else {
    ...
}

If the vb2 framework unmaps these buffers while the hardware is still running and writing to physical memory, can this cause DMA memory corruption or IOMMU faults before vdec_poweroff() is finally called later in the function?

[Severity: High]
This is a pre-existing issue, but does the teardown sequence in vdec_stop_streaming() leave a race window for a use-after-free?

The synchronization happens before the hardware is halted:

drivers/staging/media/meson/vdec/vdec.c:vdec_stop_streaming() {
    ...
		/* Synchronize and flush pending hardware interrupt service routines */
		synchronize_irq(core->vdec_irq);
		/* Ensure esparser ISR finishes executing */
		synchronize_irq(core->esparser_irq);

		cancel_work_sync(&sess->esparser_queue_work);
		mutex_lock(&core->lock);

		if (core->cur_sess == sess) {
			vdec_poweroff(sess);
            ...
			kfree(sess->priv);
    ...
}

If a new hardware interrupt fires immediately after the synchronization completes, and vdec_stop_streaming() proceeds to free sess->priv, could the concurrent ISR access the freed session data?

drivers/staging/media/meson/vdec/vdec.c:vdec_isr() {
    ...
	/* Secure an atomic acquire snapshot to protect against concurrent teardown */
	sess = smp_load_acquire(&core->cur_sess);
	if (!sess)
		return IRQ_HANDLED;

	sess->last_irq_jiffies = get_jiffies_64();

	return sess->fmt_out->codec_ops->isr(sess);
    ...
}

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11

drivers/staging/media/meson/vdec/esparser.c void esparser_queue_all_src(struct work_struct *work) · line 446

[Severity: High]
Does this worker thread run into an infinite busy-loop on empty queues or end-of-stream buffers?

In esparser_queue_all_src(), the code uses a scoped_guard for the lock:

	while (1) {
		...
		scoped_guard(mutex, &sess->lock) {
			if (!vbuf)
				break;
		}
	}

Since scoped_guard expands into a hidden for-loop, the break statement only exits the scoped_guard's loop, not the outer while (1) loop. This could trap execution indefinitely, consuming 100% CPU when the queue is empty, hits an EOS buffer, or the hardware FIFO is full.

[Severity: High]
Could the V4L2 M2M job context permanently leak if the session shuts down?

Looking at esparser_queue_all_src():

	scoped_guard(mutex, &sess->lock) {
		/* Safe atomic tracking check: exit loop if session is shutting down */
		if (READ_ONCE(sess->should_stop))
			return;

Returning early directly bypasses the newly added v4l2_m2m_job_finish() call. This could permanently hang the V4L2 memory-to-memory framework because it considers the device busy forever, preventing further decoding jobs.

[Severity: Critical]
Is there a risk of an AB-BA deadlock between the V4L2 VIDIOC_STREAMOFF ioctl and the workqueue worker?

The V4L2 core framework acquires sess->lock during VIDIOC_STREAMOFF and then calls vdec_stop_streaming(), which waits for the active worker thread:

drivers/staging/media/meson/vdec/vdec.c:vdec_stop_streaming() {
	...
	cancel_work_sync(&sess->esparser_queue_work);
}

Concurrently, the worker thread esparser_queue_all_src spins up and blocks attempting to acquire the identical sess->lock:

drivers/staging/media/meson/vdec/esparser.c:esparser_queue_all_src() {
	...
	scoped_guard(mutex, &sess->lock) {
}

This sequence would entirely deadlock the system when stopping the video stream.

drivers/staging/media/meson/vdec/vdec.c vdec_queue_recycle(struct amvdec_session *sess, struct vb2_buffer *vb) · line 165

[Severity: Critical]
Will acquiring core->lock manually here and in vdec_start_streaming result in a recursive mutex self-deadlock?

Because vdev->queue is intentionally left NULL for M2M devices, the V4L2 core ioctl wrapper unconditionally acquires vdev->lock (which is mapped to &core->lock) before delegating execution to the ioctl implementations.

The underlying driver callback vdec_start_streaming then attempts to manually acquire it again:

drivers/staging/media/meson/vdec/vdec.c:vdec_start_streaming() {
	...
	mutex_lock(&core->lock);
	if (core->cur_sess && core->cur_sess != sess) {
}

This would immediately self-deadlock the kernel thread processing VIDIOC_STREAMON or VIDIOC_STREAMOFF.

[Severity: High]
Does closing a secondary file descriptor unintentionally strip hardware ownership from the active session?

In vdec_close(), the state modification is placed outside the active session check block:

drivers/staging/media/meson/vdec/vdec.c:vdec_close() {
	...
	/* Safely clear hardware ownership since we were confirmed as the owner */
	smp_store_release(&core->cur_sess, NULL);
}

If a user-space process opens the video node twice, streams on the first session, and then closes the second session, this clears core->cur_sess unconditionally. This destroys the ownership state of the active session, bypassing its cleanup sequence and permanently leaking DMA buffers and canvases.

[Severity: Critical]
This is a pre-existing issue, but can esparser_pad_start_code() cause an out-of-bounds heap write when handling artificially small buffers?

If userspace negotiates a small sizeimage and queues a tiny buffer, the padding function computes pad_size based on ESPARSER_MIN_PACKET_SIZE and executes a memset before verifying the vb2_plane_size:

drivers/staging/media/meson/vdec/esparser.c:esparser_pad_start_code() {
	...
		pad_size = ESPARSER_MIN_PACKET_SIZE - payload_size;
		memset(vaddr + payload_size, 0, pad_size);
	}

	if ((payload_size + pad_size + SEARCH_PATTERN_LEN) >
						vb2_plane_size(vb, 0)) {
}

This could allow a local user to overwrite memory with zeroes by passing a payload_size of 0, resulting in a 4096-byte memset.

[Severity: Critical]
This is a pre-existing issue, but might an integer overflow in vp9_update_header() allow attackers to trigger an out-of-bounds memmove?

The parser reconstructs frame lengths by bit-shifting byte payloads:

drivers/staging/media/meson/vdec/esparser.c:vp9_update_header() {
	...
				frame_size[cur_frame] |=
					(dp[mag_ptr] << (cur_mag * 8));
}

If dp[mag_ptr] is >= 128 and cur_mag is 3, the unsigned 8-bit value promotes to an int and shifts into the sign bit, resulting in a large negative integer.

Later, this negative frame_size is passed to memmove():

drivers/staging/media/meson/vdec/esparser.c:vp9_update_header() {
	...
		memmove(fdata + VP9_HEADER_SIZE, old_framedata, framesize);
}

The negative framesize implicitly converts to a massive size_t, which could corrupt arbitrary kernel memory.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12

drivers/staging/media/meson/vdec/vdec.c static void vdec_m2m_device_run(void *priv) · line 174

[Severity: High]
Does this introduce an AB-BA deadlock with VIDIOC_STREAMOFF?

The V4L2 core acquires the queue lock (sess->lock) during VIDIOC_STREAMOFF and calls vdec_m2m_job_abort(), which blocks here indefinitely waiting for the worker.

Meanwhile, the worker thread unconditionally attempts to acquire sess->lock in esparser_queue_all_src():

drivers/staging/media/meson/vdec/esparser.c:esparser_queue_all_src() {
    ...
    scoped_guard(mutex, &sess->lock) {
    ...
}

drivers/staging/media/meson/vdec/vdec.c static void vdec_m2m_device_run(void *priv) · line 179

[Severity: High]
Does clearing cur_sess here prematurely bypass hardware teardown?

During a normal VIDIOC_STREAMOFF, vdec_m2m_job_abort() is called before the stop_streaming hook. If cur_sess is cleared here, the later teardown in vdec_stop_streaming() will skip powering off the hardware and freeing the DMA buffer (vififo_size) because core->cur_sess == sess evaluates to false:

drivers/staging/media/meson/vdec/vdec.c:vdec_stop_streaming() {
    ...
    if (core->cur_sess == sess) {
        vdec_poweroff(sess);
        vdec_free_canvas(sess);
    ...
}

[Severity: Critical]
Can this cause an interrupt storm and CPU lockup?

Since the physical hardware isn't stopped, it may complete its active task and assert a level-triggered interrupt.

The ISR checks cur_sess, reads NULL, and immediately returns IRQ_HANDLED without acknowledging or clearing the hardware register:

drivers/staging/media/meson/vdec/vdec.c:vdec_isr() {
    ...
    if (!sess)
        return IRQ_HANDLED;
    ...
}

Will this cause the unacknowledged interrupt to immediately fire again, locking up the CPU?

[Severity: High]
Could this allow concurrent DMA overwrites and hardware state corruption?

By clearing core->cur_sess here, the V4L2 framework considers the hardware idle and can schedule a new session.

Since the hardware is actually still running, a newly scheduled session might blindly overwrite the active hardware's registers in esparser_queue(), corrupting the ongoing DMA operations:

drivers/staging/media/meson/vdec/esparser.c:esparser_queue() {
    ...
    amvdec_write_parser(core, PARSER_VIDEO_START_PTR, payload_paddr);
    ...
}

drivers/staging/media/meson/vdec/vdec.c static void vdec_m2m_device_run(void *priv) · line 183

[Severity: High]
Does the esparser_queue_all_src() worker have an infinite loop?

When looking into the interaction with esparser_queue_work, the outer loop in esparser_queue_all_src() appears to never terminate when the queue is empty:

drivers/staging/media/meson/vdec/esparser.c:esparser_queue_all_src() {
    while (1) {
        struct vb2_v4l2_buffer *vbuf = NULL;
        bool processed_frame = false;
        ...
        scoped_guard(mutex, &sess->lock) {
            ...
            vbuf = v4l2_m2m_next_src_buf(sess->m2m_ctx);
            if (!vbuf)
                break;
            ...
        }
        ...
    }
}

Since scoped_guard is implemented as a hidden for-loop, the break statement only exits the scoped_guard's internal loop, not the outer while(1) loop.

Because processed_frame remains false, the code calls cond_resched() and spins indefinitely. Could this cause a 100% CPU lockup?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13

drivers/staging/media/meson/vdec/vdec_helpers.c static void dst_buf_done(struct amvdec_session *sess, · line 319

[Severity: Critical]
Does this introduce a double decrement? Since the callers like amvdec_dst_buf_done() and amvdec_dst_buf_done_offset() still contain their original atomic_dec() calls, this new decrement causes the counter to underflow.

When the counter becomes negative, the EOS condition atomic_read(&sess->esparser_queued_bufs) <= 1 evaluates to true, which can falsely trigger V4L2_EVENT_EOS on the wrong buffer and cause a permanent -EAGAIN deadlock.

drivers/staging/media/meson/vdec/vdec_helpers.c static void dst_buf_done(struct amvdec_session *sess, · line 322

[Severity: High]
This is a pre-existing issue, but does the error path in amvdec_dst_buf_done() permanently leak logical buffer slots?

drivers/staging/media/meson/vdec/vdec_helpers.c:amvdec_dst_buf_done() {
        ...
        if (list_empty(&sess->timestamps)) {
                v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
                spin_unlock_irqrestore(&sess->ts_spinlock, flags);
                return;
        }
        ...
}

When list_empty() is true, the function returns early without decrementing sess->esparser_queued_bufs. If this triggers enough times, the counter artificially hits the max limit, preventing new buffers from being queued.

[Severity: High]
Does the break statement in esparser_queue_all_src() inadvertently cause an infinite loop?

drivers/staging/media/meson/vdec/esparser.c:esparser_queue_all_src() {
        while (1) {
                scoped_guard(mutex, &sess->lock) {
                        ...
                        if (!vbuf)
                                break;
                        ...
                }
                ...
        }
}

Because scoped_guard() is implemented using an internal for-loop, calling break here only exits the scoped_guard's internal scope, not the outer while (1) loop. This causes the worker thread to permanently hang in an infinite busy-wait loop.

[Severity: High]
Can the failure to propagate an error in esparser_pad_start_code() cause hardware out-of-bounds reads?

drivers/staging/media/meson/vdec/esparser.c:esparser_pad_start_code() {
        ...
        if (!sess || READ_ONCE(sess->should_stop) || !sess->priv || !vaddr)
                return 0;
        ...
}

If a client provides an unmapped buffer, !vaddr is true and this returns 0, skipping the mandatory 512-byte start code padding.

Its caller esparser_queue() ignores this and passes the physical address to esparser_write_data(), which unconditionally adds 512 to the payload size. Since the padding wasn't written, the hardware DMA engine reads past the valid buffer boundary.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14

drivers/staging/media/meson/vdec/vdec_1.c vdec_1_load_firmware(struct amvdec_session *sess, const char *fwname) · line 64

[Severity: High]
This is a pre-existing issue, but is there a missing memory barrier between the firmware memcpy and the DMA start?

Earlier in vdec_1_load_firmware(), the firmware is copied to a DMA-coherent buffer:

    memcpy(mc_addr, fw->data, MC_SIZE);

Since amvdec_write_dos() maps to writel_relaxed(), there is no memory-to-I/O ordering guarantee. Could the CPU issue the MMIO write to start the DMA before the firmware data is flushed to main memory, causing the hardware to read stale data?

drivers/staging/media/meson/vdec/vdec_1.c vdec_1_load_firmware(struct amvdec_session *sess, const char *fwname) · line 67

[Severity: Low]
Does this need to use the atomic polling variant?

Since vdec_1_load_firmware() runs in a sleepable context (it successfully calls request_firmware() and allocates memory with GFP_KERNEL just prior), could this use the non-atomic readl_poll_timeout() macro instead?

Using the atomic variant forces the CPU to busy-wait in a udelay() loop for up to 10ms, which might be inefficient.

drivers/staging/media/meson/vdec/vdec_1.c vdec_1_load_firmware(struct amvdec_session *sess, const char *fwname) · line 71

[Severity: High]
This is a pre-existing issue, but does this create a DMA use-after-free if the firmware DMA transfer times out?

When a timeout occurs, the code jumps to free_mc:

free_mc:

    dma_free_coherent(core->dev, MC_SIZE, mc_addr, mc_addr_map);

If the hardware is merely slow and still executing the DMA transfer, will it continue to read from these freed memory pages? Should the DMA controller be explicitly halted (for example, by clearing the IMEM_DMA_CTRL register) before releasing the buffer?

drivers/staging/media/meson/vdec/vdec_1.c vdec_1_load_firmware(struct amvdec_session *sess, const char *fwname) · line 71

[Severity: High]
This is a pre-existing issue, but does the same problem still exist in the HEVC firmware loader?

Looking at vdec_hevc_load_firmware() in vdec_hevc.c, it uses the exact same static variables and fast decrement loop that this patch fixes in vdec_1.c:

static int vdec_hevc_load_firmware(...)

{
    ...
    static void *mc_addr;
    static dma_addr_t mc_addr_map;
    ...
    while (i && (readl(core->dos_base + HEVC_IMEM_DMA_CTRL) & 0x8000))
        i--;
    ...
}

Will HEVC decoding also fail consistently on modern CPUs due to false DMA timeouts? And could multiple HEVC decoding sessions corrupt state by sharing these static variables?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15

drivers/staging/media/meson/vdec/vdec.c static int vdec_probe(struct platform_device *pdev) · line 1200

[Severity: Medium]
This isn't a bug introduced by this patch, but can this lead to a NULL pointer dereference if the driver is probed without a device tree node? If a user with privileges binds the driver manually via sysfs without an associated device tree node, dev->of_node is NULL. The of_match_node() call will return NULL, causing of_id->data to be dereferenced blindly.

[Severity: High]
Can this regression cause a use-after-free of sess->priv in the interrupt handler?

drivers/staging/media/meson/vdec/vdec.c:vdec_stop_streaming() {
...

		/* Synchronize and flush pending hardware interrupt service routines */
		synchronize_irq(core->vdec_irq);
		/* Ensure esparser ISR finishes executing */
		synchronize_irq(core->esparser_irq);

		cancel_work_sync(&sess->esparser_queue_work);
		mutex_lock(&core->lock);

		if (core->cur_sess == sess) {
			vdec_poweroff(sess);

...
}

It appears hardware interrupts are not masked prior to calling synchronize_irq(), which could allow an interrupt to fire immediately after synchronize_irq() returns. Because vdec_poweroff() is responsible for masking the hardware interrupts, could the main thread then free sess->priv while the concurrently executing ISR dereferences it?

[Severity: High]
Does this loop correctly process the buffer queue or does it spin infinitely?

drivers/staging/media/meson/vdec/esparser.c:esparser_queue_all_src() {
...

	while (1) {
		struct vb2_v4l2_buffer *vbuf = NULL;
		bool processed_frame = false;

		scoped_guard(mutex, &sess->lock) {
			/* Safe atomic tracking check: exit loop if session is shutting down */
			if (READ_ONCE(sess->should_stop))
				return;

			/* Queue completely empty: exit work loop cleanly */
			vbuf = v4l2_m2m_next_src_buf(sess->m2m_ctx);
			if (!vbuf)
				break;

...
}

When v4l2_m2m_next_src_buf() returns NULL, the break statement seems to only exit the inner for loop created by the scoped_guard macro. This bypasses the subsequent loop exit conditions and repeats the while (1) loop endlessly, potentially causing a CPU lockup on the workqueue thread. Additionally, if a buffer is present, could this break out of the loop completely instead of processing the queue?

[Severity: Medium]
Can the lockless ISR observe an uninitialized or stale session state due to missing release barriers?

drivers/staging/media/meson/vdec/vdec.c:vdec_start_streaming() {
...

	/* If already half-initialized, do not re-initialize */
	if (sess->status == STATUS_INIT) {
		mutex_unlock(&core->lock);
		return 0;
	}

	sess->status = STATUS_INIT;
	core->cur_sess = sess;
	mutex_unlock(&core->lock);

...
}

Because core->cur_sess is assigned using a plain store, could the compiler or CPU reorder this store before earlier initialization steps like sess->status = STATUS_INIT? If the lockless ISR uses smp_load_acquire(), it might observe the new session pointer but the old memory state.

[Severity: High]
This is a pre-existing issue, but does this code cause a use-after-free if the provider driver is unbound?

drivers/soc/amlogic/meson-canvas.c:meson_canvas_get() {
...

	canvas_pdev = of_find_device_by_node(canvas_node);
	of_node_put(canvas_node);
	if (!canvas_pdev)
		return ERR_PTR(-EPROBE_DEFER);
...
	canvas = dev_get_drvdata(&canvas_pdev->dev);
	put_device(&canvas_pdev->dev);
	if (!canvas)
		return ERR_PTR(-EINVAL);

	return canvas;
}

The put_device() call immediately drops the device reference before returning the pointer. Since no device link is maintained, could an admin or unprivileged user with sysfs access unbind the amlogic-canvas driver, freeing the memory while the vdec driver still holds and uses the dangling pointer?

[Severity: Critical]
This is a pre-existing issue, but does this size calculation prevent integer overflows and out-of-bounds memory accesses?

drivers/staging/media/meson/vdec/esparser.c:vp9_update_header() {
...

		for (cur_frame = 0; cur_frame < num_frames; cur_frame++) {
			frame_size[cur_frame] = 0;
			for (cur_mag = 0; cur_mag < mag; cur_mag++) {
				if (mag_ptr >= dsize)
					return -EINVAL;

				frame_size[cur_frame] |=
					(dp[mag_ptr] << (cur_mag * 8));
				mag_ptr++;
			}
			if (cur_frame == 0)
				tot_frame_size[cur_frame] =
					frame_size[cur_frame];
			else
				tot_frame_size[cur_frame] =
					tot_frame_size[cur_frame - 1] +
					frame_size[cur_frame];
			total_datasize += frame_size[cur_frame];
		}

...
}

It appears user-provided frame sizes are added blindly without overflow protection. If a user crafts a V4L2 VP9 buffer payload with two frames specifying sizes that sum to 0 (e.g., 0x40000000 and 0xC0000000), could this bypass the new_frame_size bounds check? The driver might then use one of the huge frame sizes in a memmove operation, leading to out-of-bounds reads and writes on the kernel heap.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic

drivers/soc/amlogic/meson-canvas.c struct meson_canvas *meson_canvas_get(struct device *dev) · line 73

[Severity: High]
Is there a race condition between retrieving the driver data and creating the device link?

If the canvas provider unbinds immediately after dev_get_drvdata() but before device_link_add(), devres_release_all() will free the canvas data.

Because the driver core allows links to unbound suppliers, device_link_add() would still succeed and create a dormant link, and then meson_canvas_get() would return a freed pointer to the caller.

drivers/soc/amlogic/meson-canvas.c struct meson_canvas *meson_canvas_get(struct device *dev) · line 81

[Severity: High]
This is a pre-existing issue, but does this device link fully resolve the use-after-free for the vdec driver?

Because vdec is a V4L2 driver, userspace file descriptors can outlive the consumer driver's unbind sequence. If a user holds the video device open:

1. The canvas provider unbinds.
2. The device link forces the vdec driver to unbind.
3. devres_release_all() frees the devm-managed canvas and vdec core contexts.
4. Userspace later closes the file descriptor.

When the file descriptor is closed, vdec_close() calls vdec_free_canvas(), which attempts to dereference the already freed sess->core->canvas pointer.

drivers/staging/media/meson/vdec/vdec.c struct dummy_buf { · line 33

[Severity: High]
Does increasing the buffer to 32 MiB cause the bitstream size to overflow the 32-bit hardware PARSER_CONTROL register calculation?

In drivers/staging/media/meson/vdec/esparser.c:esparser_write_data():

	amvdec_write_parser(core, PARSER_CONTROL,
			    ES_WRITE |
			    ES_PARSER_START |
			    ES_SEARCH |
			    (size << ES_PACK_SIZE_BIT));

With ES_PACK_SIZE_BIT being 8, for a size >= 16 MiB this left shift will overflow a 32-bit unsigned integer. This can result in the DMA engine receiving a truncated payload size and crashing.

[Severity: High]
This is a pre-existing issue, but does increasing the buffer size make it highly probable that the hardware ring buffer end pointer truncates, causing immediate DMA corruption?

In drivers/staging/media/meson/vdec/vdec_hevc.c:vdec_hevc_stbuf_init():

	amvdec_write_dos(core, HEVC_STREAM_END_ADDR,
			 sess->vififo_paddr + sess->vififo_size);

If the VIFIFO buffer is allocated near the end of the 32-bit address space, the calculation sess->vififo_paddr + sess->vififo_size can exceed 32 bits, resulting in a value like 0x100000000. This is silently truncated to 0 when passed to amvdec_write_dos() which accepts a 32-bit u32.

[Severity: Medium]
This is a pre-existing issue, but how does the driver handle timestamps when the 32-bit hardware offset wraps around at the 4 GB bitstream boundary?

In drivers/staging/media/meson/vdec/vdec_helpers.c:amvdec_dst_buf_done_offset():

	list_for_each_entry_safe(tmp, n, &sess->timestamps, list) {
		if (tmp->offset > offset) {
			...
			break;
		}

The stream offset accumulates into a 32-bit integer in esparser_get_offset() that naturally wraps at 4 GB. The list_for_each_entry_safe loop uses a non-wrap-safe raw unsigned comparison. When the offset wraps around to 0, a large tmp->offset immediately evaluates as greater and incorrectly breaks the loop, stranding or dropping timestamps.

[Severity: High]
Are there Use-After-Free vulnerabilities in vdec_close() during device unbind due to devm_kzalloc() and v4l2_m2m_release() lifecycle mismatches? This isn't a bug introduced by this patch, but...

In drivers/staging/media/meson/vdec/vdec.c:vdec_remove():

	video_unregister_device(core->vdev_dec);
	v4l2_m2m_release(core->m2m_dev);

The unbind path frees the core structure while V4L2 file descriptors may still be actively held by userspace.

Later, if an application closes the file descriptor, vdec_close() is invoked:

	synchronize_irq(core->vdec_irq);

This could dereference the freed core pointer and the freed m2m_dev.

[Severity: High]
Does the esparser_queue_all_src() workqueue handler contain an infinite loop that could monopolize a CPU core?

In drivers/staging/media/meson/vdec/esparser.c:esparser_queue_all_src():

	while (1) {
		struct vb2_v4l2_buffer *vbuf = NULL;
		bool processed_frame = false;

		scoped_guard(mutex, &sess->lock) {
			...
			vbuf = v4l2_m2m_next_src_buf(sess->m2m_ctx);
			if (!vbuf)
				break;
			...
		}

		if (processed_frame)
			break;
		...
	}

The break statements are intended to exit the function when no buffers are available. However, they only escape the scoped_guard's internal for loop. This leaves the outer while (1) loop intact since processed_frame is false, which could result in a lockup of a kernel worker thread.

[Severity: High]
Does this code double decrement the esparser_queued_bufs atomic counter?

In drivers/staging/media/meson/vdec/vdec_helpers.c, dst_buf_done() calls atomic_dec(&sess->esparser_queued_bufs). However, its callers also execute their own atomic_dec():

amvdec_dst_buf_done():

	dst_buf_done(sess, vbuf, field, timestamp, timecode, vbuf_flags);
	atomic_dec(&sess->esparser_queued_bufs);

amvdec_dst_buf_done_offset():

	dst_buf_done(sess, vbuf, field, timestamp, timecode, vbuf_flags);
	if (match)
		atomic_dec(&sess->esparser_queued_bufs);

This could lead to counter underflow, bypassing buffer limit checks and causing hardware queue overruns.

[Severity: Medium]
This is a pre-existing issue, but does esparser_write_data() contain a race condition that overwrites the completion flag?

In drivers/staging/media/meson/vdec/esparser.c:esparser_write_data():

	amvdec_write_parser(core, PARSER_FETCH_CMD,
			    (7 << FETCH_ENDIAN_BIT) |
			    (size + SEARCH_PATTERN_LEN));

	search_done = 0;
	return wait_event_interruptible_timeout(wq, search_done, (HZ / 5));

The CPU initiates hardware processing by writing to PARSER_FETCH_CMD, which can instantly trigger esparser_isr(). If the hardware interrupt fires immediately, the ISR sets search_done = 1. The CPU sequentially executes search_done = 0 afterwards, which could erase the completion signal before waiting on it.

[Severity: Critical]
This is a pre-existing issue, but does vp9_update_header() have an integer overflow when decoding frame magnitudes that can bypass size validations?

In drivers/staging/media/meson/vdec/esparser.c:vp9_update_header():

				frame_size[cur_frame] |=
					(dp[mag_ptr] << (cur_mag * 8));

The magnitude byte is promoted to a signed 32-bit integer. When shifted by 24 bits, it can become a large negative number if the maliciously crafted payload has dp[mag_ptr] >= 0x80.

Later, this negative frame size bypasses total_datasize validations and triggers an out-of-bounds copy in memmove:

		memmove(fdata + VP9_HEADER_SIZE, old_framedata, framesize);

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=18

drivers/gpu/drm/meson/meson_drv.c static int meson_drv_bind_master(struct device *dev, bool has_components) · line 207

[Severity: High]
If dma_set_mask_and_coherent() fails in meson_drv_bind_master(), the code jumps to the free_drm error path before the priv pointer is allocated and initialized.

Because the free_drm label unconditionally passes priv to encoder removal functions:

drivers/gpu/drm/meson/meson_drv.c:meson_drv_bind_master() {
...
free_drm:

	drm_dev_put(drm);

	meson_encoder_dsi_remove(priv);

...
}

And meson_encoder_dsi_remove() dereferences priv immediately:

drivers/gpu/drm/meson/meson_encoder_dsi.c:meson_encoder_dsi_remove() {
...

	if (priv->encoders[MESON_ENC_DSI]) {

...
}

Will this result in dereferencing an uninitialized stack pointer?