Skip to content

Commit 68987aa

Browse files
committed
media: uvcvideo: Remove dangling pointers
jira VULN-53465 cve CVE-2024-58002 commit-author Ricardo Ribalda <[email protected]> commit 221cd51 When an async control is written, we copy a pointer to the file handle that started the operation. That pointer will be used when the device is done. Which could be anytime in the future. If the user closes that file descriptor, its structure will be freed, and there will be one dangling pointer per pending async control, that the driver will try to use. Clean all the dangling pointers during release(). To avoid adding a performance penalty in the most common case (no async operation), a counter has been introduced with some logic to make sure that it is properly handled. Cc: [email protected] Fixes: e5225c8 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives") Reviewed-by: Hans de Goede <[email protected]> Signed-off-by: Ricardo Ribalda <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Laurent Pinchart <[email protected]> Signed-off-by: Mauro Carvalho Chehab <[email protected]> (cherry picked from commit 221cd51) Signed-off-by: Marcin Wcisło <[email protected]>
1 parent f5e17ff commit 68987aa

File tree

3 files changed

+67
-3
lines changed

3 files changed

+67
-3
lines changed

drivers/media/usb/uvc/uvc_ctrl.c

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,6 +1529,40 @@ static void uvc_ctrl_send_slave_event(struct uvc_video_chain *chain,
15291529
uvc_ctrl_send_event(chain, handle, ctrl, mapping, val, changes);
15301530
}
15311531

1532+
static void uvc_ctrl_set_handle(struct uvc_fh *handle, struct uvc_control *ctrl,
1533+
struct uvc_fh *new_handle)
1534+
{
1535+
lockdep_assert_held(&handle->chain->ctrl_mutex);
1536+
1537+
if (new_handle) {
1538+
if (ctrl->handle)
1539+
dev_warn_ratelimited(&handle->stream->dev->udev->dev,
1540+
"UVC non compliance: Setting an async control with a pending operation.");
1541+
1542+
if (new_handle == ctrl->handle)
1543+
return;
1544+
1545+
if (ctrl->handle) {
1546+
WARN_ON(!ctrl->handle->pending_async_ctrls);
1547+
if (ctrl->handle->pending_async_ctrls)
1548+
ctrl->handle->pending_async_ctrls--;
1549+
}
1550+
1551+
ctrl->handle = new_handle;
1552+
handle->pending_async_ctrls++;
1553+
return;
1554+
}
1555+
1556+
/* Cannot clear the handle for a control not owned by us.*/
1557+
if (WARN_ON(ctrl->handle != handle))
1558+
return;
1559+
1560+
ctrl->handle = NULL;
1561+
if (WARN_ON(!handle->pending_async_ctrls))
1562+
return;
1563+
handle->pending_async_ctrls--;
1564+
}
1565+
15321566
void uvc_ctrl_status_event(struct uvc_video_chain *chain,
15331567
struct uvc_control *ctrl, const u8 *data)
15341568
{
@@ -1539,7 +1573,8 @@ void uvc_ctrl_status_event(struct uvc_video_chain *chain,
15391573
mutex_lock(&chain->ctrl_mutex);
15401574

15411575
handle = ctrl->handle;
1542-
ctrl->handle = NULL;
1576+
if (handle)
1577+
uvc_ctrl_set_handle(handle, ctrl, NULL);
15431578

15441579
list_for_each_entry(mapping, &ctrl->info.mappings, list) {
15451580
s32 value = __uvc_ctrl_get_value(mapping, data);
@@ -1815,7 +1850,7 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev,
18151850

18161851
if (!rollback && handle &&
18171852
ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
1818-
ctrl->handle = handle;
1853+
uvc_ctrl_set_handle(handle, ctrl, handle);
18191854
}
18201855

18211856
return 0;
@@ -2746,6 +2781,26 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
27462781
return 0;
27472782
}
27482783

2784+
void uvc_ctrl_cleanup_fh(struct uvc_fh *handle)
2785+
{
2786+
struct uvc_entity *entity;
2787+
2788+
guard(mutex)(&handle->chain->ctrl_mutex);
2789+
2790+
if (!handle->pending_async_ctrls)
2791+
return;
2792+
2793+
list_for_each_entry(entity, &handle->chain->dev->entities, list) {
2794+
for (unsigned int i = 0; i < entity->ncontrols; ++i) {
2795+
if (entity->controls[i].handle != handle)
2796+
continue;
2797+
uvc_ctrl_set_handle(handle, &entity->controls[i], NULL);
2798+
}
2799+
}
2800+
2801+
WARN_ON(handle->pending_async_ctrls);
2802+
}
2803+
27492804
/*
27502805
* Cleanup device controls.
27512806
*/

drivers/media/usb/uvc/uvc_v4l2.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,8 @@ static int uvc_v4l2_release(struct file *file)
659659

660660
uvc_dbg(stream->dev, CALLS, "%s\n", __func__);
661661

662+
uvc_ctrl_cleanup_fh(handle);
663+
662664
/* Only free resources if this is a privileged handle. */
663665
if (uvc_has_privileges(handle))
664666
uvc_queue_release(&stream->queue);

drivers/media/usb/uvc/uvcvideo.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,11 @@ struct uvc_video_chain {
329329
struct uvc_entity *processing; /* Processing unit */
330330
struct uvc_entity *selector; /* Selector unit */
331331

332-
struct mutex ctrl_mutex; /* Protects ctrl.info */
332+
struct mutex ctrl_mutex; /*
333+
* Protects ctrl.info,
334+
* ctrl.handle and
335+
* uvc_fh.pending_async_ctrls
336+
*/
333337

334338
struct v4l2_prio_state prio; /* V4L2 priority state */
335339
u32 caps; /* V4L2 chain-wide caps */
@@ -604,6 +608,7 @@ struct uvc_fh {
604608
struct uvc_video_chain *chain;
605609
struct uvc_streaming *stream;
606610
enum uvc_handle_state state;
611+
unsigned int pending_async_ctrls;
607612
};
608613

609614
struct uvc_driver {
@@ -789,6 +794,8 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
789794
int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
790795
struct uvc_xu_control_query *xqry);
791796

797+
void uvc_ctrl_cleanup_fh(struct uvc_fh *handle);
798+
792799
/* Utility functions */
793800
struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
794801
u8 epaddr);

0 commit comments

Comments
 (0)