[LTS 9.4] media: uvcvideo: Remove dangling pointers #548

pvts-mat · 2025-09-03T22:04:40Z

[LTS 9.4]
CVE-2024-58002
VULN-53465

Problem

https://access.redhat.com/security/cve/CVE-2024-58002

A dangling pointer vulnerability was found in the Linux kernel. When an async control is written, a copy of a pointer is made in the file handle that started the operation. 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, leading to denial of service of the system.

Applicability: yes

The affected module is "USB Video Class", uvcvideo.ko. Enabled by CONFIG_USB_VIDEO_CLASS, set to m in all config files of ciqlts9_4:

$ grep 'USB_VIDEO_CLASS\b' configs/*.config

configs/kernel-aarch64-64k-debug-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-aarch64-64k-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-aarch64-debug-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-aarch64-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-aarch64-rt-debug-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-aarch64-rt-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-ppc64le-debug-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-ppc64le-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-s390x-debug-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-s390x-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-s390x-zfcpdump-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-x86_64-debug-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-x86_64-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-x86_64-rt-debug-rhel.config:CONFIG_USB_VIDEO_CLASS=m
configs/kernel-x86_64-rt-rhel.config:CONFIG_USB_VIDEO_CLASS=m

The fixing commit 221cd51 is not backported, the "Fixes" commit e5225c8 can be found natively in ciqlts9_4's history.

Solution

The fixing commit 221cd51 could not have been cherry picked without conflicts. It's useful to compare the timeline of the modified files drivers/media/usb/uvc/{uvc_ctrl.c,uvc_v4l2.c,uvcvideo.h}, for LTS 9.4 and Linux stable 5.15 as this fix was backported to 5.15 and the 5.15 version is only a few commits ahead of LTS 9.4:

   kernel-mainline                                                                                                   linux-5.15.y            ciqlts9_4
   ----------------------------------------------------------------------------------------------------------------  ----------------------  ----------------------
   …
   8869eb654 2024-12-19 media: uvcvideo: Allow changing noparam on the fly
   d6b874ff9 2024-12-19 media: uvcvideo: Flush the control cache when we get an event
   02baaa09d 2024-12-19 media: uvcvideo: Annotate lock requirements for uvc_ctrl_set
1> 221cd51ef 2024-12-19 media: uvcvideo: Remove dangling pointers                                                    ~ 117f7a297 2025-03-13
   04d3398f6 2024-12-19 media: uvcvideo: Remove redundant NULL assignment                                            ~ 14810fb99 2025-03-13
2> d9fecd096 2024-12-19 media: uvcvideo: Only save async fh if success                                               ~ d775f9e9ek 2025-03-13
   840fb2c33 2024-12-19 media: uvcvideo: Remove duplicated cap/out code
   c31cffd5a 2024-12-19 media: uvcvideo: Fix event flags in uvc_ctrl_send_events                                     ~ 74512c021 2025-03-13
   a9ea1a3d8 2024-12-19 media: uvcvideo: Fix crash during unbind if gpio unit is in use                              ~ 0fdd7cc59 2025-03-13
   44f703386 2024-10-08 media: uvcvideo: Refactor the status irq API
   66558537c 2024-07-22 media: uvcvideo: Fix custom control mapping probing
   8c40efeda 2024-06-17 media: uvcvideo: Remove mappings form uvc_device_info
   e5cbddd09 2024-06-17 media: uvcvideo: Remove PLF device quirking
   6c7f1f756 2024-06-17 media: uvcvideo: Cleanup version-specific mapping
   b2b5fcb1c 2024-06-17 media: uvcvideo: Probe the PLF characteristics
   a8505ad3b 2024-06-17 media: uvcvideo: Refactor Power Line Frequency limit selection
   8f4362a8d 2024-06-17 media: uvcvideo: Allow custom control mapping
   86419686e 2024-06-17 media: uvcvideo: Override default flags                                                      ~ 3a1e47f47 2024-08-19
   53d799538 2024-06-17 media: uvcvideo: Fix hw timestamp handling for slow FPS
   9183c6f1a 2024-06-17 media: uvcvideo: Quirk for invalid dev_sof in Logitech C922
3> 64627daf0 2024-05-04 media: uvcvideo: Refactor iterators                                                          ~ e0360e009 2025-03-13
   9a6f13261 2024-05-03 media: uvcvideo: Use max() macro
   3de6df64f 2024-04-19 media: uvcvideo: Disable autosuspend for Insta360 Link
   07731053d 2024-04-19 media: uvcvideo: Add quirk for Logitech Rally Bar
   41ebaa5e0 2023-09-14 media: uvcvideo: Fix OOB read
   6d00f4ec1 2023-07-28 media: uvcvideo: Fix menu count handling for userspace XU mappings                                                   ~ d90d3f3ce 2023-11-21
   af621ba2e 2023-06-09 media: uvcvideo: Constify formats, frames and intervals                                                              ~ ba54197fe 2023-11-21
   aa8db3adc 2023-06-09 media: uvcvideo: Rename uvc_format 'frame' field to 'frames'                                                         ~ eb571a6b4 2023-11-21
   ccfad4e85 2023-06-09 media: uvcvideo: Rename uvc_streaming 'format' field to 'formats'                                                    ~ 86ce1a698 2023-11-21
   …
   8643d237a 2018-09-11 media: uvcvideo: Make some structs const                                                     = 8643d237a 2018-09-11  = 8643d237a 2018-09-11
   394dc5888 2018-08-31 media: videobuf2-v4l2: integrate with media requests                                         = 394dc5888 2018-08-31  = 394dc5888 2018-08-31
0> e5225c820 2018-07-27 media: uvcvideo: Send a control event when a Control Change interrupt arrives                = e5225c820 2018-07-27  = e5225c820 2018-07-27
   222964eaf 2018-07-27 media: uvcvideo: Remove a redundant check                                                    = 222964eaf 2018-07-27  = 222964eaf 2018-07-27
   557a5c7fe 2018-07-27 media: uvcvideo: Add KSMedia 8-bit IR format support                                         = 557a5c7fe 2018-07-27  = 557a5c7fe 2018-07-27
   …

(Full timeline.log)

Legend:

0	`e5225c8`	The commit introducing the bug
1	`221cd51`	The commit fixing the bug

Since the latest version 6d00f4e for ciqlts9_4 there was a set of 6 commits to consider as the domain of possible prerequisites: 04d3398, d9fecd0, c31cffd, a9ea1a3, 8641968, e0360e0 (actually could have been more, but the solution was found within this heuristic starting point). The subset 64627da, d9fecd0 was found to secure the clean cherry pick of the mainline fix 221cd51 while also being the smallest. Thus, the legend continued:

2	`d9fecd0`	First prerequisite for the fix
3	`64627da`	Second prerequisite for the fix

It can be noticed that the 5.15 backport actually differs slightly from the mainline version in how mutex is locked in the uvc_ctrl_cleanup_fh(…) function.

Mainline:

kernel-src-tree/drivers/media/usb/uvc/uvc_ctrl.c

Lines 2810 to 2828 in 221cd51

    
           void uvc_ctrl_cleanup_fh(struct uvc_fh *handle) 
        
           { 
        
           	struct uvc_entity *entity; 
        
           	guard(mutex)(&handle->chain->ctrl_mutex); 
        
           	if (!handle->pending_async_ctrls) 
        
           		return; 
        
           	list_for_each_entry(entity, &handle->chain->dev->entities, list) { 
        
           		for (unsigned int i = 0; i < entity->ncontrols; ++i) { 
        
           			if (entity->controls[i].handle != handle) 
        
           				continue; 
        
           			uvc_ctrl_set_handle(handle, &entity->controls[i], NULL); 
        
           		} 
        
           	} 
        
           	WARN_ON(handle->pending_async_ctrls); 
        
           }

Stable 5.15:

kernel-src-tree/drivers/media/usb/uvc/uvc_ctrl.c

Lines 2590 to 2612 in 117f7a2

    
           void uvc_ctrl_cleanup_fh(struct uvc_fh *handle) 
        
           { 
        
           	struct uvc_entity *entity; 
        
           	mutex_lock(&handle->chain->ctrl_mutex); 
        
           	if (!handle->pending_async_ctrls) { 
        
           		mutex_unlock(&handle->chain->ctrl_mutex); 
        
           		return; 
        
           	} 
        
           	list_for_each_entry(entity, &handle->chain->dev->entities, list) { 
        
           		unsigned int i; 
        
           		for (i = 0; i < entity->ncontrols; ++i) { 
        
           			if (entity->controls[i].handle != handle) 
        
           				continue; 
        
           			uvc_ctrl_set_handle(handle, &entity->controls[i], NULL); 
        
           		} 
        
           	} 
        
           	WARN_ON(handle->pending_async_ctrls); 
        
           	mutex_unlock(&handle->chain->ctrl_mutex); 
        
           }

This is because the the include/linux/cleanup.h file, where the guard(…) macro is defined, is missing from the 5.15 version. It was backported to ciqlts9_4, however, in the 880fe86 commit, therefore the mainline version of the patch was used in this backport.

kABI check: passed

$ DEBUG=1 CVE=CVE-2024-58002 ./ninja.sh _kabi_checked__x86_64--test--ciqlts9_4-CVE-2024-58002

[0/1] Check ABI of kernel [ciqlts9_4-CVE-2024-58002]
++ uname -m
+ python3 /data/src/ctrliq-github/kernel-dist-git-el-9.4/SOURCES/check-kabi -k /data/src/ctrliq-github/kernel-dist-git-el-9.4/SOURCES/Module.kabi_x86_64 -s vms/x86_64--build--ciqlts9_4/build_files/kernel-src-tree-ciqlts9_4-CVE-2024-58002/Module.symvers
kABI check passed
+ touch state/kernels/ciqlts9_4-CVE-2024-58002/x86_64/kabi_checked

Boot test: passed

Including the "boot test" of the uvcvideo.ko module, that is its loading:

boot-test.log

Kselftests: passed relative

Coverage

bpf (only test_cgroup_storage, test_tag, test_sysctl, test_verifier, test_tcpnotify_user, test_sock, test_lpm_map, test_lru_map), breakpoints (only breakpoint_test), capabilities, clone3, cpu-hotplug, cpufreq, drivers/dma-buf, drivers/net/bonding (all except bond_macvlan.sh), drivers/net/team, exec, filesystems/binderfs, filesystems/epoll, firmware, fpu, ftrace, futex, gpio, intel_pstate, iommu, ipc, ir, kcmp, kexec, kvm, landlock, lib, livepatch, membarrier, memfd, memory-hotplug, mincore, mount, mqueue, nci, net/forwarding (all except sch_tbf_prio.sh, ipip_hier_gre_keys.sh, sch_tbf_root.sh, tc_actions.sh, gre_inner_v6_multipath.sh, q_in_vni.sh, tc_police.sh, sch_ets.sh, mirror_gre_vlan_bridge_1q.sh, mirror_gre_bridge_1d_vlan.sh, router_bridge_1d_lag.sh, sch_red.sh, bridge_igmp.sh, ip6gre_inner_v6_multipath.sh, dual_vxlan_bridge.sh, vxlan_bridge_1d_ipv6.sh, router_bridge_lag.sh, sch_tbf_ets.sh), net/hsr, net/mptcp (all except simult_flows.sh, userspace_pm.sh, mptcp_join.sh), net (all except reuseport_addr_any.sh, srv6_end_flavors_test.sh, fib_nexthops.sh, gro.sh, reuseaddr_conflict, srv6_end_dt4_l3vpn_test.sh, srv6_end_dt6_l3vpn_test.sh, txtimestamp.sh, udpgso_bench.sh, srv6_end_dt46_l3vpn_test.sh, xfrm_policy.sh, ip_defrag.sh, udpgro_fwd.sh), netfilter (all except nft_trans_stress.sh), nsfs, pid_namespace, pidfd, proc (all except proc-uptime-001, proc-pid-vm), pstore, ptrace, rlimits, rseq, seccomp, sgx, sigaltstack, size, splice, static_keys, syscall_user_dispatch, tc-testing, tdx, timens, timers, tmpfs, tpm2, tty, vDSO, x86, zram

Reference

kselftests–ciqlts9_4–run1.log

Patch

kselftests–ciqlts9_4-CVE-2024-58002–run1.log
kselftests–ciqlts9_4-CVE-2024-58002–run2.log

Comparison

The tests results for the reference and the patch are the same.

$ ktests.xsh diff -d kselftests*.log

Column    File
--------  ----------------------------------------------
Status0   kselftests--ciqlts9_4--run1.log
Status1   kselftests--ciqlts9_4-CVE-2024-58002--run1.log
Status2   kselftests--ciqlts9_4-CVE-2024-58002--run2.log

Specific tests: skipped

jira VULN-53465 cve-pre CVE-2024-58002 commit-author Ricardo Ribalda <[email protected]> commit 64627da Avoid using the iterators after the list_for_each() constructs. This patch should be a NOP, but makes cocci, happier: drivers/media/usb/uvc/uvc_ctrl.c:1861:44-50: ERROR: invalid reference to the index variable of the iterator on line 1850 drivers/media/usb/uvc/uvc_ctrl.c:2195:17-23: ERROR: invalid reference to the index variable of the iterator on line 2179 Reviewed-by: Sergey Senozhatsky <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> Signed-off-by: Ricardo Ribalda <[email protected]> Signed-off-by: Hans Verkuil <[email protected]> (cherry picked from commit 64627da) Signed-off-by: Marcin Wcisło <[email protected]>

jira VULN-53465 cve-pre CVE-2024-58002 commit-author Ricardo Ribalda <[email protected]> commit d9fecd0 Now we keep a reference to the active fh for any call to uvc_ctrl_set, regardless if it is an actual set or if it is a just a try or if the device refused the operation. We should only keep the file handle if the device actually accepted applying the operation. Cc: [email protected] Fixes: e5225c8 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives") Suggested-by: Hans de Goede <[email protected]> Reviewed-by: Hans de Goede <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> Signed-off-by: Ricardo Ribalda <[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 d9fecd0) Signed-off-by: Marcin Wcisło <[email protected]>

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]>

bmastbergen

🥌

PlaidCat

pvts-mat added 3 commits September 3, 2025 01:49

PlaidCat requested review from kerneltoast, PlaidCat, bmastbergen, thefossguy-ciq and shreeya-patel98 September 4, 2025 14:02

bmastbergen approved these changes Sep 4, 2025

View reviewed changes

PlaidCat approved these changes Sep 4, 2025

View reviewed changes

PlaidCat merged commit a12f9cb into ctrliq:ciqlts9_4 Sep 4, 2025
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LTS 9.4] media: uvcvideo: Remove dangling pointers #548

[LTS 9.4] media: uvcvideo: Remove dangling pointers #548

Uh oh!

pvts-mat commented Sep 3, 2025

Uh oh!

bmastbergen left a comment

Uh oh!

PlaidCat left a comment

Uh oh!

Uh oh!

Uh oh!

	void uvc_ctrl_cleanup_fh(struct uvc_fh *handle)
	{
	struct uvc_entity *entity;

	guard(mutex)(&handle->chain->ctrl_mutex);

	if (!handle->pending_async_ctrls)
	return;

	list_for_each_entry(entity, &handle->chain->dev->entities, list) {
	for (unsigned int i = 0; i < entity->ncontrols; ++i) {
	if (entity->controls[i].handle != handle)
	continue;
	uvc_ctrl_set_handle(handle, &entity->controls[i], NULL);
	}
	}

	WARN_ON(handle->pending_async_ctrls);
	}

	void uvc_ctrl_cleanup_fh(struct uvc_fh *handle)
	{
	struct uvc_entity *entity;

	mutex_lock(&handle->chain->ctrl_mutex);

	if (!handle->pending_async_ctrls) {
	mutex_unlock(&handle->chain->ctrl_mutex);
	return;
	}

	list_for_each_entry(entity, &handle->chain->dev->entities, list) {
	unsigned int i;
	for (i = 0; i < entity->ncontrols; ++i) {
	if (entity->controls[i].handle != handle)
	continue;
	uvc_ctrl_set_handle(handle, &entity->controls[i], NULL);
	}
	}

	WARN_ON(handle->pending_async_ctrls);
	mutex_unlock(&handle->chain->ctrl_mutex);
	}

[LTS 9.4] media: uvcvideo: Remove dangling pointers #548

[LTS 9.4] media: uvcvideo: Remove dangling pointers #548

Uh oh!

Conversation

pvts-mat commented Sep 3, 2025

Problem

Applicability: yes

Solution

kABI check: passed

Boot test: passed

Kselftests: passed relative

Coverage

Reference

Patch

Comparison

Specific tests: skipped

Uh oh!

bmastbergen left a comment

Choose a reason for hiding this comment

Uh oh!

PlaidCat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!