Skip to content

Commit 98202de

Browse files
committed
media: uvcvideo: Remove dangling pointers
jira VULN-53466 cve CVE-2024-58002 commit-author Ricardo Ribalda <[email protected]> commit 221cd51 upstream-diff - Refenced kernel-lt 5.15 commit 117f7a2 This is due to missing: - 54da6a0 - locking: Introduce __cleanup() based infrastructure Leaving the loop as the mainline since this kernel branch contains: - e8c0708 - Kbuild: move to -std=gnu11 at c1bb5aa 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: Jonathan Maple <[email protected]>
1 parent 11ce137 commit 98202de

File tree

3 files changed

+70
-3
lines changed

3 files changed

+70
-3
lines changed

drivers/media/usb/uvc/uvc_ctrl.c

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1378,6 +1378,40 @@ static void uvc_ctrl_send_slave_event(struct uvc_video_chain *chain,
13781378
uvc_ctrl_send_event(chain, handle, ctrl, mapping, val, changes);
13791379
}
13801380

1381+
static void uvc_ctrl_set_handle(struct uvc_fh *handle, struct uvc_control *ctrl,
1382+
struct uvc_fh *new_handle)
1383+
{
1384+
lockdep_assert_held(&handle->chain->ctrl_mutex);
1385+
1386+
if (new_handle) {
1387+
if (ctrl->handle)
1388+
dev_warn_ratelimited(&handle->stream->dev->udev->dev,
1389+
"UVC non compliance: Setting an async control with a pending operation.");
1390+
1391+
if (new_handle == ctrl->handle)
1392+
return;
1393+
1394+
if (ctrl->handle) {
1395+
WARN_ON(!ctrl->handle->pending_async_ctrls);
1396+
if (ctrl->handle->pending_async_ctrls)
1397+
ctrl->handle->pending_async_ctrls--;
1398+
}
1399+
1400+
ctrl->handle = new_handle;
1401+
handle->pending_async_ctrls++;
1402+
return;
1403+
}
1404+
1405+
/* Cannot clear the handle for a control not owned by us.*/
1406+
if (WARN_ON(ctrl->handle != handle))
1407+
return;
1408+
1409+
ctrl->handle = NULL;
1410+
if (WARN_ON(!handle->pending_async_ctrls))
1411+
return;
1412+
handle->pending_async_ctrls--;
1413+
}
1414+
13811415
void uvc_ctrl_status_event(struct uvc_video_chain *chain,
13821416
struct uvc_control *ctrl, const u8 *data)
13831417
{
@@ -1388,7 +1422,8 @@ void uvc_ctrl_status_event(struct uvc_video_chain *chain,
13881422
mutex_lock(&chain->ctrl_mutex);
13891423

13901424
handle = ctrl->handle;
1391-
ctrl->handle = NULL;
1425+
if (handle)
1426+
uvc_ctrl_set_handle(handle, ctrl, NULL);
13921427

13931428
list_for_each_entry(mapping, &ctrl->info.mappings, list) {
13941429
s32 value = __uvc_ctrl_get_value(mapping, data);
@@ -1660,7 +1695,7 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev,
16601695

16611696
if (!rollback && handle &&
16621697
ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
1663-
ctrl->handle = handle;
1698+
uvc_ctrl_set_handle(handle, ctrl, handle);
16641699
}
16651700

16661701
return 0;
@@ -2573,6 +2608,29 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
25732608
return 0;
25742609
}
25752610

2611+
void uvc_ctrl_cleanup_fh(struct uvc_fh *handle)
2612+
{
2613+
struct uvc_entity *entity;
2614+
2615+
mutex_lock(&handle->chain->ctrl_mutex);
2616+
2617+
if (!handle->pending_async_ctrls) {
2618+
mutex_unlock(&handle->chain->ctrl_mutex);
2619+
return;
2620+
}
2621+
2622+
list_for_each_entry(entity, &handle->chain->dev->entities, list) {
2623+
for (unsigned int i = 0; i < entity->ncontrols; ++i) {
2624+
if (entity->controls[i].handle != handle)
2625+
continue;
2626+
uvc_ctrl_set_handle(handle, &entity->controls[i], NULL);
2627+
}
2628+
}
2629+
2630+
WARN_ON(handle->pending_async_ctrls);
2631+
mutex_unlock(&handle->chain->ctrl_mutex);
2632+
}
2633+
25762634
/*
25772635
* Cleanup device controls.
25782636
*/

drivers/media/usb/uvc/uvc_v4l2.c

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

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

609+
uvc_ctrl_cleanup_fh(handle);
610+
609611
/* Only free resources if this is a privileged handle. */
610612
if (uvc_has_privileges(handle))
611613
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
@@ -475,7 +475,11 @@ struct uvc_video_chain {
475475
struct uvc_entity *processing; /* Processing unit */
476476
struct uvc_entity *selector; /* Selector unit */
477477

478-
struct mutex ctrl_mutex; /* Protects ctrl.info */
478+
struct mutex ctrl_mutex; /*
479+
* Protects ctrl.info,
480+
* ctrl.handle and
481+
* uvc_fh.pending_async_ctrls
482+
*/
479483

480484
struct v4l2_prio_state prio; /* V4L2 priority state */
481485
u32 caps; /* V4L2 chain-wide caps */
@@ -728,6 +732,7 @@ struct uvc_fh {
728732
struct uvc_video_chain *chain;
729733
struct uvc_streaming *stream;
730734
enum uvc_handle_state state;
735+
unsigned int pending_async_ctrls;
731736
};
732737

733738
struct uvc_driver {
@@ -910,6 +915,8 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
910915
int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
911916
struct uvc_xu_control_query *xqry);
912917

918+
void uvc_ctrl_cleanup_fh(struct uvc_fh *handle);
919+
913920
/* Utility functions */
914921
void uvc_simplify_fraction(u32 *numerator, u32 *denominator,
915922
unsigned int n_terms, unsigned int threshold);

0 commit comments

Comments
 (0)