Re: [PATCH 06/10] Input: synaptics-rmi4 - propagate F54 worker errors to V4L2 queue
From: sashiko-bot@kernel.org
Date: 2026-06-26 07:30:08
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] The V4L2 video_device, vb2_queue, and mutex lock are embedded inside struct f54_data, which is allocated using devm_kzalloc(). When the device is unbound, devres frees f54_data immediately after rmi_f54_remove() returns. If a userspace process holds the /dev/videoX file descriptor open, subsequent V4L2 ioctl calls or close() operations will trigger a Use-After-Free. - [Medium] The V4L2 buf_queue callback executes synchronous, blocking hardware polling via wait_for_completion_timeout(), turning an asynchronous queueing mechanism into a fully blocking operation that starves other ioctls. -- commit ed5dda6c6cb42efd8468fff80b16a819c0e59afb Author: Dmitry Torokhov [off-list ref] Input: synaptics-rmi4 - propagate F54 worker errors to V4L2 queue Previously, rmi_f54_buffer_queue() waited for the worker thread to finish but ignored whether it succeeded. This patch adds a report_error field to struct f54_data to store the worker's exit status and marks the buffer with VB2_BUF_STATE_ERROR if the worker failed.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c index e86dfc9ce7d9a..c86bc81845bb1 100644 --- a/drivers/input/rmi4/rmi_f54.c +++ b/drivers/input/rmi4/rmi_f54.c@@ -106,6 +106,7 @@ struct f54_data { u8 *report_data; size_t max_report_size; int report_size; + int report_error;
[Severity: High]
This is a pre-existing issue, but does using devres to allocate struct
f54_data create a use-after-free risk during device removal?
Since the V4L2 video_device, vb2_queue, and mutex lock are embedded inside
this structure, and it is allocated via devm_kzalloc() in rmi_f54_probe():
rmi_f54_probe() {
...
f54 = devm_kzalloc(&fn->dev, sizeof(struct f54_data), GFP_KERNEL);
...
}
The memory will be freed immediately when the device is unbound. If a
userspace process still holds the /dev/videoX file descriptor open,
subsequent V4L2 ioctl calls or close() operations could access the freed
video_device or queue.
[ ... ]
quoted hunk ↗ jump to hunk
@@ -340,6 +341,12 @@ static void rmi_f54_buffer_queue(struct vb2_buffer *vb) mutex_lock(&f54->data_mutex); } + if (f54->report_error) { + dev_err(&f54->fn->dev, "Error acquiring report: %d\n", f54->report_error); + state = VB2_BUF_STATE_ERROR; + goto data_done; + } +
[Severity: Medium]
This isn't a bug introduced by this patch, but does waiting for the hardware
completion inside rmi_f54_buffer_queue() starve other concurrent V4L2
operations?
Just before this error check, rmi_f54_buffer_queue() executes:
rmi_f54_buffer_queue() {
...
while (f54->is_busy) {
mutex_unlock(&f54->data_mutex);
if (!wait_for_completion_timeout(&f54->cmd_done, ...)) {
...
}
This executes synchronously while holding the V4L2 queue lock. Blocking here
via wait_for_completion_timeout() for up to 1000ms could prevent other users
from executing concurrent ioctls like DQBUF or STREAMOFF.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626051802.4033172-1-dmitry.torokhov@gmail.com?part=6