[PATCH 1/2] usb: gadget: uvc: replace mutex lock/unlock with scoped_guard in uvc_function_bind()

[Xu Yang]

From: Xu Yang <xu.yang_2@xxxxxxx>

Commit 68aa70648b62 ("usb: gadget: uvc: hold opts->lock across XU walks
in uvc_function_bind") introduced an error_unlock label to release
opts->lock on failure paths. The label is misplaced between the return
statement and v4l2_error, causing it to fall through into v4l2_error
and call v4l2_device_unregister() on a device that was never registered.

Replace the manual mutex_lock/unlock pair and the error_unlock label
with scoped_guard(mutex), removing the need for explicit lock cleanup on
error paths.

Fixes: 68aa70648b62 ("usb: gadget: uvc: hold opts->lock across XU walks in uvc_function_bind")
Assisted-by: Claude:claude-sonnet-4.6
Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
---
 drivers/usb/gadget/function/f_uvc.c | 80 +++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index d1bf3ea75197..a4fb2790f4ff 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -768,23 +768,17 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
 	uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
 
-	/*
-	 * Hold opts->lock across both the XU string-descriptor fixup below and
-	 * the descriptor-copy block further down.  Without this, configfs
-	 * uvcg_extension_drop() (which takes opts->lock) can race with the
-	 * list_for_each_entry() walks here and inside uvc_copy_descriptors(),
-	 * leading to a UAF on a freed struct uvcg_extension.  See
-	 * drivers/usb/gadget/function/uvc_configfs.c::uvcg_extension_drop().
-	 */
-	mutex_lock(&opts->lock);
-
 	/*
 	 * XUs can have an arbitrary string descriptor describing them. If they
-	 * have one pick up the ID.
+	 * have one pick up the ID. Hold opts->lock here to avoid race with configfs
+	 * uvcg_extension_make() and uvcg_extension_drop().
 	 */
-	list_for_each_entry(xu, &opts->extension_units, list)
-		if (xu->string_descriptor_index)
-			xu->desc.iExtension = cdev->usb_strings[xu->string_descriptor_index].id;
+	scoped_guard(mutex, &opts->lock) {
+		list_for_each_entry(xu, &opts->extension_units, list)
+			if (xu->string_descriptor_index)
+				xu->desc.iExtension =
+					cdev->usb_strings[xu->string_descriptor_index].id;
+	}
 
 	/*
 	 * We attach the hard-coded defaults incase the user does not provide
@@ -795,7 +789,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 				 ARRAY_SIZE(uvc_en_us_strings));
 	if (IS_ERR(us)) {
 		ret = PTR_ERR(us);
-		goto error_unlock;
+		goto error;
 	}
 
 	uvc_iad.iFunction = opts->iad_index ? cdev->usb_strings[opts->iad_index].id :
@@ -809,50 +803,50 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 
 	/* Allocate interface IDs. */
 	if ((ret = usb_interface_id(c, f)) < 0)
-		goto error_unlock;
+		goto error;
 	uvc_iad.bFirstInterface = ret;
 	uvc_control_intf.bInterfaceNumber = ret;
 	uvc->control_intf = ret;
 	opts->control_interface = ret;
 
 	if ((ret = usb_interface_id(c, f)) < 0)
-		goto error_unlock;
+		goto error;
 	uvc_streaming_intf_alt0.bInterfaceNumber = ret;
 	uvc_streaming_intf_alt1.bInterfaceNumber = ret;
 	uvc->streaming_intf = ret;
 	opts->streaming_interface = ret;
 
 	/* Copy descriptors */
-	f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
-	if (IS_ERR(f->fs_descriptors)) {
-		ret = PTR_ERR(f->fs_descriptors);
-		f->fs_descriptors = NULL;
-		goto error_unlock;
-	}
+	scoped_guard(mutex, &opts->lock) {
+		f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
+		if (IS_ERR(f->fs_descriptors)) {
+			ret = PTR_ERR(f->fs_descriptors);
+			f->fs_descriptors = NULL;
+			goto error;
+		}
 
-	f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH);
-	if (IS_ERR(f->hs_descriptors)) {
-		ret = PTR_ERR(f->hs_descriptors);
-		f->hs_descriptors = NULL;
-		goto error_unlock;
-	}
+		f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH);
+		if (IS_ERR(f->hs_descriptors)) {
+			ret = PTR_ERR(f->hs_descriptors);
+			f->hs_descriptors = NULL;
+			goto error;
+		}
 
-	f->ss_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER);
-	if (IS_ERR(f->ss_descriptors)) {
-		ret = PTR_ERR(f->ss_descriptors);
-		f->ss_descriptors = NULL;
-		goto error_unlock;
-	}
+		f->ss_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER);
+		if (IS_ERR(f->ss_descriptors)) {
+			ret = PTR_ERR(f->ss_descriptors);
+			f->ss_descriptors = NULL;
+			goto error;
+		}
 
-	f->ssp_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER_PLUS);
-	if (IS_ERR(f->ssp_descriptors)) {
-		ret = PTR_ERR(f->ssp_descriptors);
-		f->ssp_descriptors = NULL;
-		goto error_unlock;
+		f->ssp_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER_PLUS);
+		if (IS_ERR(f->ssp_descriptors)) {
+			ret = PTR_ERR(f->ssp_descriptors);
+			f->ssp_descriptors = NULL;
+			goto error;
+		}
 	}
 
-	mutex_unlock(&opts->lock);
-
 	/* Preallocate control endpoint request. */
 	uvc->control_req = usb_ep_alloc_request(cdev->gadget->ep0, GFP_KERNEL);
 	uvc->control_buf = kmalloc(UVC_MAX_REQUEST_SIZE, GFP_KERNEL);
@@ -884,8 +878,6 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 
 	return 0;
 
-error_unlock:
-	mutex_unlock(&opts->lock);
 v4l2_error:
 	v4l2_device_unregister(&uvc->v4l2_dev);
 error:

-- 
2.34.1