Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

usb: gadget: uvc: prevent use of disabled endpoint

Currently the set_alt callback immediately disables the endpoint and queues
the v4l2 streamoff event. However, as the streamoff event is processed
asynchronously, it is possible that the video_pump thread attempts to queue
requests to an already disabled endpoint.

This change moves disabling usb endpoint to the end of streamoff event
callback. As the endpoint's state can no longer be used, video_pump is
now guarded by uvc->state as well. To be consistent with the actual
streaming state, uvc->state is now toggled between CONNECTED and STREAMING
from the v4l2 event callback only.

Link: https://lore.kernel.org/20230615171558.GK741@pendragon.ideasonboard.com/
Link: https://lore.kernel.org/20230531085544.253363-1-dan.scally@ideasonboard.com/
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Tested-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Avichal Rakesh <arakesh@google.com>
Link: https://lore.kernel.org/r/20231109004104.3467968-1-arakesh@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Avichal Rakesh and committed by
Greg Kroah-Hartman
991544dc 5a1ccf0c

+26 -12
+5 -6
drivers/usb/gadget/function/f_uvc.c
··· 263 263 return 0; 264 264 } 265 265 266 - void uvc_function_setup_continue(struct uvc_device *uvc) 266 + void uvc_function_setup_continue(struct uvc_device *uvc, int disable_ep) 267 267 { 268 268 struct usb_composite_dev *cdev = uvc->func.config->cdev; 269 + 270 + if (disable_ep && uvc->video.ep) 271 + usb_ep_disable(uvc->video.ep); 269 272 270 273 usb_composite_setup_continue(cdev); 271 274 } ··· 340 337 if (uvc->state != UVC_STATE_STREAMING) 341 338 return 0; 342 339 343 - if (uvc->video.ep) 344 - usb_ep_disable(uvc->video.ep); 345 - 346 340 memset(&v4l2_event, 0, sizeof(v4l2_event)); 347 341 v4l2_event.type = UVC_EVENT_STREAMOFF; 348 342 v4l2_event_queue(&uvc->vdev, &v4l2_event); 349 343 350 - uvc->state = UVC_STATE_CONNECTED; 351 - return 0; 344 + return USB_GADGET_DELAYED_STATUS; 352 345 353 346 case 1: 354 347 if (uvc->state != UVC_STATE_CONNECTED)
+1 -1
drivers/usb/gadget/function/f_uvc.h
··· 11 11 12 12 struct uvc_device; 13 13 14 - void uvc_function_setup_continue(struct uvc_device *uvc); 14 + void uvc_function_setup_continue(struct uvc_device *uvc, int disable_ep); 15 15 16 16 void uvc_function_connect(struct uvc_device *uvc); 17 17
+1 -1
drivers/usb/gadget/function/uvc.h
··· 177 177 * Functions 178 178 */ 179 179 180 - extern void uvc_function_setup_continue(struct uvc_device *uvc); 180 + extern void uvc_function_setup_continue(struct uvc_device *uvc, int disable_ep); 181 181 extern void uvc_function_connect(struct uvc_device *uvc); 182 182 extern void uvc_function_disconnect(struct uvc_device *uvc); 183 183
+17 -3
drivers/usb/gadget/function/uvc_v4l2.c
··· 451 451 * Complete the alternate setting selection setup phase now that 452 452 * userspace is ready to provide video frames. 453 453 */ 454 - uvc_function_setup_continue(uvc); 454 + uvc_function_setup_continue(uvc, 0); 455 455 uvc->state = UVC_STATE_STREAMING; 456 456 457 457 return 0; ··· 463 463 struct video_device *vdev = video_devdata(file); 464 464 struct uvc_device *uvc = video_get_drvdata(vdev); 465 465 struct uvc_video *video = &uvc->video; 466 + int ret = 0; 466 467 467 468 if (type != video->queue.queue.type) 468 469 return -EINVAL; 469 470 470 - return uvcg_video_enable(video, 0); 471 + uvc->state = UVC_STATE_CONNECTED; 472 + ret = uvcg_video_enable(video, 0); 473 + if (ret < 0) 474 + return ret; 475 + 476 + uvc_function_setup_continue(uvc, 1); 477 + return 0; 471 478 } 472 479 473 480 static int ··· 507 500 static void uvc_v4l2_disable(struct uvc_device *uvc) 508 501 { 509 502 uvc_function_disconnect(uvc); 503 + /* 504 + * Drop uvc->state to CONNECTED if it was streaming before. 505 + * This ensures that the usb_requests are no longer queued 506 + * to the controller. 507 + */ 508 + if (uvc->state == UVC_STATE_STREAMING) 509 + uvc->state = UVC_STATE_CONNECTED; 510 + 510 511 uvcg_video_enable(&uvc->video, 0); 511 512 uvcg_free_buffers(&uvc->video.queue); 512 513 uvc->func_connected = false; ··· 662 647 .get_unmapped_area = uvcg_v4l2_get_unmapped_area, 663 648 #endif 664 649 }; 665 -
+2 -1
drivers/usb/gadget/function/uvc_video.c
··· 384 384 struct uvc_video_queue *queue = &video->queue; 385 385 /* video->max_payload_size is only set when using bulk transfer */ 386 386 bool is_bulk = video->max_payload_size; 387 + struct uvc_device *uvc = video->uvc; 387 388 struct usb_request *req = NULL; 388 389 struct uvc_buffer *buf; 389 390 unsigned long flags; 390 391 bool buf_done; 391 392 int ret; 392 393 393 - while (video->ep->enabled) { 394 + while (uvc->state == UVC_STATE_STREAMING && video->ep->enabled) { 394 395 /* 395 396 * Retrieve the first available USB request, protected by the 396 397 * request lock.