From patchwork Wed Oct 18 19:46:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Avichal Rakesh X-Patchwork-Id: 155106 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2908:b0:403:3b70:6f57 with SMTP id ib8csp5028529vqb; Wed, 18 Oct 2023 12:47:01 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGS8mViobbrXgdUKsOb5R+dbFXmgvl0lQoHyBBIMS5++q+0jZtY66HcMXGlfGPMEhlAc5Np X-Received: by 2002:a05:6870:4589:b0:1d0:f5bd:6e9 with SMTP id y9-20020a056870458900b001d0f5bd06e9mr462058oao.22.1697658421451; Wed, 18 Oct 2023 12:47:01 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697658421; cv=none; d=google.com; s=arc-20160816; b=iJaeDqqUvT347QAmmxfUNlFAU6mmUaLsZPJr/KD2BO7usxVf58cBZVvuTm4085pFxG 7r4l4ONt+cLhRTnEeLsxgstIF2ZVu/HXBtZzPek9nC8gykz7oN92i40mGNjQBDcNiBMG umf8LJ3W9rISUb/vryCIavXUxzA1/gpar51dY04pETsZIR2fS+mV/vtKY6fIu7vpXPlO MALmdD6R+RgmpsCUXgInOsQjdrd3Kt1MHcMcH5J0XWakGyQ01xqqpgqIi65OrsH9G79o n1yQ+H5A8QDBB51FSAFRaLk6xXhuiK6Axo8dEFJgxOs+PG5SnShr3nZia6BArBMWypW6 C3pg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:dkim-signature; bh=wazq6oxL63t4WK65yHIQsChGr02y7yPq4mkDkJPS1lI=; fh=E5X6/3uWCMdGSKLB7vuNn1z8ZeRDzUIfiV2I8BhU+4Q=; b=0iAVi0f70DEIwD5wgn+2TvKgXg1p7O2xKemfsdQKLS5mLHI8xOayaqQzyN+/rOoe8t oaYPWwQUZAVJ2hrVB33VKj/gYJ7xcQD+MIcrtM0GC9POICSeHlwIsLcG9/3qKdirMn0u B5htMWdSlhNjpKzQdMAWvSYeeWXh325JOQ9V9KEPbKlHOBgq2fP1+DvUeOKomhRe3obX 3VI+9LHSzL2WrDwobh0Td92DcPkHf4FuP7Mpd5syLo7canUR0o0PYUMh40S7a2DjlpEO Tnxq34HFJH2yhEz4nuUA5ciuZKIauIskBUT/oRlYnerjRKUE3VWGb/iuLW/b3hmuJIg/ bJbg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=yBuI7vqG; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id b69-20020a633448000000b00577ae8b603fsi2847736pga.314.2023.10.18.12.47.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Oct 2023 12:47:01 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=yBuI7vqG; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id 101698244E63; Wed, 18 Oct 2023 12:46:56 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231182AbjJRTqr (ORCPT + 24 others); Wed, 18 Oct 2023 15:46:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58870 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229963AbjJRTqq (ORCPT ); Wed, 18 Oct 2023 15:46:46 -0400 Received: from mail-yw1-x114a.google.com (mail-yw1-x114a.google.com [IPv6:2607:f8b0:4864:20::114a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4EF5B112 for ; Wed, 18 Oct 2023 12:46:44 -0700 (PDT) Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-5a88f9a1cf7so49525257b3.3 for ; Wed, 18 Oct 2023 12:46:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1697658403; x=1698263203; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=wazq6oxL63t4WK65yHIQsChGr02y7yPq4mkDkJPS1lI=; b=yBuI7vqGYV937YH8bRXnsQBr7GgLNVJW87gdSZ5VZVSHNuVzCwV8O41Y5BId7ZlzlN JICtdvUfmA56jjsdN5is6jtqaEiPBCv/M6JJaIYCKXxcif1rQXSBnWqILNgNHUaI6iH0 I4pS+C0TL7pKGDuUkmsF05dNws6zYjqEiTsc+GTqHfmoJ6Dy4aPx5IjzFpilG5C9xDX4 zLiOKWH7//7DLGBRhfwpXrJ/Cpts869a/8aQoandLd9fcimoHaCv7SkS+1B/iwDdHMq/ UQx1ZCX+lnheQNqLJybJmt8UvkDzxVBwLtE/WkQxV0gN2M9ntmCQy6kdgcaWyWsboUlJ NbEA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697658403; x=1698263203; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=wazq6oxL63t4WK65yHIQsChGr02y7yPq4mkDkJPS1lI=; b=qXv+7uoXg9EBEFijAFkCgVVVSzWhCcgVD4NxggX2DGeaPPo7uWZAQZdmgULX4t3Kwe VXl/QjEU0ndEE06Rt9DoCbQFcq6M9SKVL/EH8o6AHwP+SmIzgf+X0OGR+P/+2feDW+Q0 BhNhbmddCb/IP8gS5gwXZCD4c+DoaIfyA67V+mQfd+q0nqfroWhl7BGcklbV7FlQon/H afUkurB1Df2ea6vPXeuF7cR9kFJjHemeW+aH/IA5/LkxPN2iozxECc3b+yaBKsv5GoFs 5HFVIm9wj3bJ1VSGZybrpMaLPkjKn78h2L6hn0uCr7vIXgA++z2e5572iIX4WxWuDfBI meHw== X-Gm-Message-State: AOJu0Yx71bKpHau+iKU7vZoy4M91mHd1/fsex17FeUh3hl/7lR0ZsaDf N6ceZ8MwXasq1wceZ6y5DKOFfbjj9xUz X-Received: from hi-h2o-specialist.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:3cef]) (user=arakesh job=sendgmr) by 2002:a05:6902:52:b0:d9b:f88c:ca2b with SMTP id m18-20020a056902005200b00d9bf88cca2bmr8249ybh.11.1697658403560; Wed, 18 Oct 2023 12:46:43 -0700 (PDT) Date: Wed, 18 Oct 2023 12:46:36 -0700 In-Reply-To: <20230930184821.310143-1-arakesh@google.com> Mime-Version: 1.0 References: <20230930184821.310143-1-arakesh@google.com> X-Mailer: git-send-email 2.42.0.758.gaed0368e0e-goog Message-ID: <20231018194638.2446575-1-arakesh@google.com> Subject: [PATCH v5 1/3] usb: gadget: uvc: prevent use of disabled endpoint From: Avichal Rakesh To: arakesh@google.com, dan.scally@ideasonboard.com, gregkh@linuxfoundation.org, laurent.pinchart@ideasonboard.com, m.grzeschik@pengutronix.de Cc: etalvala@google.com, jchowdhary@google.com, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org X-Spam-Status: No, score=-8.4 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Wed, 18 Oct 2023 12:46:56 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1778501398075540037 X-GMAIL-MSGID: 1780123877109842783 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: Michael Grzeschik Tested-by: Michael Grzeschik Signed-off-by: Avichal Rakesh --- v1 -> v2: Rebased to ToT and reworded commit message. v2 -> v3: Fix email threading goof-up v3 -> v4: Address review comments & re-rebase to ToT v4 -> v5: Add Reviewed-by & Tested-by drivers/usb/gadget/function/f_uvc.c | 11 +++++------ drivers/usb/gadget/function/f_uvc.h | 2 +- drivers/usb/gadget/function/uvc.h | 2 +- drivers/usb/gadget/function/uvc_v4l2.c | 20 +++++++++++++++++--- drivers/usb/gadget/function/uvc_video.c | 3 ++- 5 files changed, 26 insertions(+), 12 deletions(-) -- 2.42.0.758.gaed0368e0e-goog diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index faa398109431..ae08341961eb 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -263,10 +263,13 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl) return 0; } -void uvc_function_setup_continue(struct uvc_device *uvc) +void uvc_function_setup_continue(struct uvc_device *uvc, int disable_ep) { struct usb_composite_dev *cdev = uvc->func.config->cdev; + if (disable_ep && uvc->video.ep) + usb_ep_disable(uvc->video.ep); + usb_composite_setup_continue(cdev); } @@ -337,15 +340,11 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt) if (uvc->state != UVC_STATE_STREAMING) return 0; - if (uvc->video.ep) - usb_ep_disable(uvc->video.ep); - memset(&v4l2_event, 0, sizeof(v4l2_event)); v4l2_event.type = UVC_EVENT_STREAMOFF; v4l2_event_queue(&uvc->vdev, &v4l2_event); - uvc->state = UVC_STATE_CONNECTED; - return 0; + return USB_GADGET_DELAYED_STATUS; case 1: if (uvc->state != UVC_STATE_CONNECTED) diff --git a/drivers/usb/gadget/function/f_uvc.h b/drivers/usb/gadget/function/f_uvc.h index 1db972d4beeb..e7f9f13f14dc 100644 --- a/drivers/usb/gadget/function/f_uvc.h +++ b/drivers/usb/gadget/function/f_uvc.h @@ -11,7 +11,7 @@ struct uvc_device; -void uvc_function_setup_continue(struct uvc_device *uvc); +void uvc_function_setup_continue(struct uvc_device *uvc, int disale_ep); void uvc_function_connect(struct uvc_device *uvc); diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 6751de8b63ad..989bc6b4e93d 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -177,7 +177,7 @@ struct uvc_file_handle { * Functions */ -extern void uvc_function_setup_continue(struct uvc_device *uvc); +extern void uvc_function_setup_continue(struct uvc_device *uvc, int disable_ep); extern void uvc_function_connect(struct uvc_device *uvc); extern void uvc_function_disconnect(struct uvc_device *uvc); diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index 3f0a9795c0d4..7cb8d027ff0c 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -451,7 +451,7 @@ uvc_v4l2_streamon(struct file *file, void *fh, enum v4l2_buf_type type) * Complete the alternate setting selection setup phase now that * userspace is ready to provide video frames. */ - uvc_function_setup_continue(uvc); + uvc_function_setup_continue(uvc, 0); uvc->state = UVC_STATE_STREAMING; return 0; @@ -463,11 +463,18 @@ uvc_v4l2_streamoff(struct file *file, void *fh, enum v4l2_buf_type type) struct video_device *vdev = video_devdata(file); struct uvc_device *uvc = video_get_drvdata(vdev); struct uvc_video *video = &uvc->video; + int ret = 0; if (type != video->queue.queue.type) return -EINVAL; - return uvcg_video_enable(video, 0); + uvc->state = UVC_STATE_CONNECTED; + ret = uvcg_video_enable(video, 0); + if (ret < 0) + return ret; + + uvc_function_setup_continue(uvc, 1); + return 0; } static int @@ -500,6 +507,14 @@ uvc_v4l2_subscribe_event(struct v4l2_fh *fh, static void uvc_v4l2_disable(struct uvc_device *uvc) { uvc_function_disconnect(uvc); + /* + * Drop uvc->state to CONNECTED if it was streaming before. + * This ensures that the usb_requests are no longer queued + * to the controller. + */ + if (uvc->state == UVC_STATE_STREAMING) + uvc->state = UVC_STATE_CONNECTED; + uvcg_video_enable(&uvc->video, 0); uvcg_free_buffers(&uvc->video.queue); uvc->func_connected = false; @@ -647,4 +662,3 @@ const struct v4l2_file_operations uvc_v4l2_fops = { .get_unmapped_area = uvcg_v4l2_get_unmapped_area, #endif }; - diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index 91af3b1ef0d4..c334802ac0a4 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -384,13 +384,14 @@ static void uvcg_video_pump(struct work_struct *work) struct uvc_video_queue *queue = &video->queue; /* video->max_payload_size is only set when using bulk transfer */ bool is_bulk = video->max_payload_size; + struct uvc_device *uvc = video->uvc; struct usb_request *req = NULL; struct uvc_buffer *buf; unsigned long flags; bool buf_done; int ret; - while (video->ep->enabled) { + while (uvc->state == UVC_STATE_STREAMING && video->ep->enabled) { /* * Retrieve the first available USB request, protected by the * request lock. From patchwork Wed Oct 18 19:46:37 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Avichal Rakesh X-Patchwork-Id: 155107 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2908:b0:403:3b70:6f57 with SMTP id ib8csp5028528vqb; Wed, 18 Oct 2023 12:47:01 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFLCp/RPn3qF/pA1+k41Fft+8wJnpTzVlq65NQtP+whsIsSXGo8Qx20Z4pZAbfFJq908uqa X-Received: by 2002:aa7:88d0:0:b0:6bd:71e3:b647 with SMTP id k16-20020aa788d0000000b006bd71e3b647mr80052pff.19.1697658421250; Wed, 18 Oct 2023 12:47:01 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697658421; cv=none; d=google.com; s=arc-20160816; b=PKCdFz3wzsclnNOVaN2ZsojJL24tj3qf4GO/QAyvAnBbZBLXN369veqOvTF21FchTA b1cF8apXbPI53QkIt3yjyIS6S7S2rJHE8guvPmIedRDaAup171b/bCiQyWK9Wldj/N1U MLKjuTo5d2zchjqlxhi9/CJuywLB0yjlTsNZdPpiRDwtmUocEQllq8cgm9nO9Uowkbd9 iClQ8qLZgQtYH9o5EYJolboPTr2ZQW9BSBT/QL4iCZCht7cLM5j6p0sR3YWlO8WNSrTj umXUqIcrLbzRHl6ktxGUoaro8TgWTI7o3oJC5EFkMPtjvfEaO8ve373hsnWPO2mwqzeg 2pyg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:dkim-signature; bh=I3ifVLPf//gGBLo42V9Etc/XVJjRwYx2PCWlQzF4JOM=; fh=E5X6/3uWCMdGSKLB7vuNn1z8ZeRDzUIfiV2I8BhU+4Q=; b=bxaRLeziDJkgrSbJ0lVrycVkT8R6xsTFBFBxYsrfytglVuy0AX0eYz8yt8lr7G2gVu wygRICr2DoRxmYocKIZNskrBYrpFUV4vI3VQNWsYFQ5SaV1cHbsa/jp4olS2tCCho6oE b8QrFw83Dvu/QGxQwp73Fk2KV/baqLxLev8lgzmWMdxKorQ7oF4k7yLWVW/YGSblNBby e74w0pUrD+XNcJIlX6aXlPMSwnbppsDlJSO+ZVZXsk5c95l7KOBf5ElnnHyuPPE3PRvV LkKnaMRG+/SvH5MdN7ClYGiMi9BhNKq6ZARdpZfj1wXVTvBF5elykxSy7y8vmBA1+FA5 XTtg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=hE33QzmO; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id r4-20020a632b04000000b005a073e0cca1si2848052pgr.512.2023.10.18.12.47.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Oct 2023 12:47:01 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=hE33QzmO; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 9984681BDF34; Wed, 18 Oct 2023 12:47:00 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231435AbjJRTqx (ORCPT + 24 others); Wed, 18 Oct 2023 15:46:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34398 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231390AbjJRTqv (ORCPT ); Wed, 18 Oct 2023 15:46:51 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B9846112 for ; Wed, 18 Oct 2023 12:46:47 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id 3f1490d57ef6-d8486b5e780so9027919276.0 for ; Wed, 18 Oct 2023 12:46:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1697658407; x=1698263207; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=I3ifVLPf//gGBLo42V9Etc/XVJjRwYx2PCWlQzF4JOM=; b=hE33QzmOMhoMKgkXMr5EGWm5ZcEX8lo7iWR7lAMkBZvw5R7TscQriPho7NiKXJ68uR qYVz6XZIQKqL6g5Bn/NboUD6AZXGqS3ufXy5iuZrIoEOs1b8lgtkZOvWtSAsuYQ97zRE 3eUammWjhrJI+z1GOAS849xCHDM9ROE0hotPznRJ2+i4uncA6isT/Hg1W5N17gY6B3EF ASkbkHFhO3Zmb2HTQlfqv4XFfwlbK6YqsA9y9p6v2UQ8lKZWS8idJP4S4WK7+0K0G4X3 ukaZjf7p9SxNKtuWMLxeAIOG286jjsl3i+eFkgnM6mA/rQ92TAZ3nJ9ucr8YFC0SjoFH GU2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697658407; x=1698263207; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=I3ifVLPf//gGBLo42V9Etc/XVJjRwYx2PCWlQzF4JOM=; b=NQBaae6B7wZQMkgoEoD075GlpX3zua/bl0Dv5wLW0nJteaMAi5T5oPF4Lmo4dWdVn9 9FCcEbXnDMucF8ZvGMeGBXTs8b2wMDeBfz6BZBwh8x4ZpTKiOLb6j9unB4GBF4ZpxdYr cwkhI9D5ph7k9Oq1SQQFu3tFUNKAD/sQJjFy7MElpfoKGej0o+DRyQzwJIlT6MZ2rgVz E0oANHVcv4AwrqXas2EBfFynelZXaqCJBmNPtDeApJHyNwes+2WgGRBStPMwrwPcF3t+ c24nPWzleDGUupIEEGhh/dPcUdz2Ex6YKtRb4y22gQI+/UIuJnn8X+EtgOPhaXrnHvWZ QIaw== X-Gm-Message-State: AOJu0YznkoH2mNpNXIYDwmVz54+4kxQfTFIGXSUMzghLBHe0zEtOZDGB EJh4rrSjOlZVkFEsre3QJhprjQFOdqrD X-Received: from hi-h2o-specialist.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:3cef]) (user=arakesh job=sendgmr) by 2002:a25:9287:0:b0:d74:93a1:70a2 with SMTP id y7-20020a259287000000b00d7493a170a2mr8588ybl.5.1697658406978; Wed, 18 Oct 2023 12:46:46 -0700 (PDT) Date: Wed, 18 Oct 2023 12:46:37 -0700 In-Reply-To: <20231018194638.2446575-1-arakesh@google.com> Mime-Version: 1.0 References: <20230930184821.310143-1-arakesh@google.com> <20231018194638.2446575-1-arakesh@google.com> X-Mailer: git-send-email 2.42.0.758.gaed0368e0e-goog Message-ID: <20231018194638.2446575-2-arakesh@google.com> Subject: [PATCH v5 2/3] usb: gadget: uvc: Allocate uvc_requests one at a time From: Avichal Rakesh To: arakesh@google.com, dan.scally@ideasonboard.com, gregkh@linuxfoundation.org, laurent.pinchart@ideasonboard.com, m.grzeschik@pengutronix.de Cc: etalvala@google.com, jchowdhary@google.com, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 18 Oct 2023 12:47:00 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1778501399086666798 X-GMAIL-MSGID: 1780123876545399288 Currently, the uvc gadget driver allocates all uvc_requests as one array and deallocates them all when the video stream stops. This includes de-allocating all the usb_requests associated with those uvc_requests. This can lead to use-after-free issues if any of those de-allocated usb_requests were still owned by the usb controller. This patch is 1 of 2 patches addressing the use-after-free issue. Instead of bulk allocating all uvc_requests as an array, this patch allocates uvc_requests one at a time, which should allows for similar granularity when deallocating the uvc_requests. This patch has no functional changes other than allocating each uvc_request separately, and similarly freeing each of them separately. Link: https://lore.kernel.org/7cd81649-2795-45b6-8c10-b7df1055020d@google.com Suggested-by: Michael Grzeschik Reviewed-by: Michael Grzeschik Tested-by: Michael Grzeschik Signed-off-by: Avichal Rakesh --- v1 -> v2: Rebased to ToT v2 -> v3: Fix email threading goof-up v3 -> v4: Address review comments & re-rebase to ToT v4 -> v5: Address more review comments. Add Reviewed-by & Tested-by. drivers/usb/gadget/function/uvc.h | 3 +- drivers/usb/gadget/function/uvc_video.c | 89 ++++++++++++++----------- 2 files changed, 52 insertions(+), 40 deletions(-) -- 2.42.0.758.gaed0368e0e-goog diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 989bc6b4e93d..993694da0bbc 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -81,6 +81,7 @@ struct uvc_request { struct sg_table sgt; u8 header[UVCG_REQUEST_HEADER_LEN]; struct uvc_buffer *last_buf; + struct list_head list; }; struct uvc_video { @@ -102,7 +103,7 @@ struct uvc_video { /* Requests */ unsigned int req_size; - struct uvc_request *ureq; + struct list_head ureqs; /* all uvc_requests allocated by uvc_video */ struct list_head req_free; spinlock_t req_lock; diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index c334802ac0a4..c180866c8e34 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -227,6 +227,24 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video, * Request handling */ +static void +uvc_video_free_request(struct uvc_request *ureq, struct usb_ep *ep) +{ + sg_free_table(&ureq->sgt); + if (ureq->req && ep) { + usb_ep_free_request(ep, ureq->req); + ureq->req = NULL; + } + + kfree(ureq->req_buffer); + ureq->req_buffer = NULL; + + if (!list_empty(&ureq->list)) + list_del_init(&ureq->list); + + kfree(ureq); +} + static int uvcg_video_ep_queue(struct uvc_video *video, struct usb_request *req) { int ret; @@ -293,27 +311,12 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) static int uvc_video_free_requests(struct uvc_video *video) { - unsigned int i; - - if (video->ureq) { - for (i = 0; i < video->uvc_num_requests; ++i) { - sg_free_table(&video->ureq[i].sgt); + struct uvc_request *ureq, *temp; - if (video->ureq[i].req) { - usb_ep_free_request(video->ep, video->ureq[i].req); - video->ureq[i].req = NULL; - } - - if (video->ureq[i].req_buffer) { - kfree(video->ureq[i].req_buffer); - video->ureq[i].req_buffer = NULL; - } - } - - kfree(video->ureq); - video->ureq = NULL; - } + list_for_each_entry_safe(ureq, temp, &video->ureqs, list) + uvc_video_free_request(ureq, video->ep); + INIT_LIST_HEAD(&video->ureqs); INIT_LIST_HEAD(&video->req_free); video->req_size = 0; return 0; @@ -322,6 +325,7 @@ uvc_video_free_requests(struct uvc_video *video) static int uvc_video_alloc_requests(struct uvc_video *video) { + struct uvc_request *ureq; unsigned int req_size; unsigned int i; int ret = -ENOMEM; @@ -332,29 +336,34 @@ uvc_video_alloc_requests(struct uvc_video *video) * max_t(unsigned int, video->ep->maxburst, 1) * (video->ep->mult); - video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL); - if (video->ureq == NULL) - return -ENOMEM; + INIT_LIST_HEAD(&video->ureqs); + for (i = 0; i < video->uvc_num_requests; i++) { + ureq = kzalloc(sizeof(struct uvc_request), GFP_KERNEL); + if (ureq == NULL) + goto error; + + INIT_LIST_HEAD(&ureq->list); + + list_add_tail(&ureq->list, &video->ureqs); - for (i = 0; i < video->uvc_num_requests; ++i) { - video->ureq[i].req_buffer = kmalloc(req_size, GFP_KERNEL); - if (video->ureq[i].req_buffer == NULL) + ureq->req_buffer = kmalloc(req_size, GFP_KERNEL); + if (ureq->req_buffer == NULL) goto error; - video->ureq[i].req = usb_ep_alloc_request(video->ep, GFP_KERNEL); - if (video->ureq[i].req == NULL) + ureq->req = usb_ep_alloc_request(video->ep, GFP_KERNEL); + if (ureq->req == NULL) goto error; - video->ureq[i].req->buf = video->ureq[i].req_buffer; - video->ureq[i].req->length = 0; - video->ureq[i].req->complete = uvc_video_complete; - video->ureq[i].req->context = &video->ureq[i]; - video->ureq[i].video = video; - video->ureq[i].last_buf = NULL; + ureq->req->buf = ureq->req_buffer; + ureq->req->length = 0; + ureq->req->complete = uvc_video_complete; + ureq->req->context = ureq; + ureq->video = video; + ureq->last_buf = NULL; - list_add_tail(&video->ureq[i].req->list, &video->req_free); + list_add_tail(&ureq->req->list, &video->req_free); /* req_size/PAGE_SIZE + 1 for overruns and + 1 for header */ - sg_alloc_table(&video->ureq[i].sgt, + sg_alloc_table(&ureq->sgt, DIV_ROUND_UP(req_size - UVCG_REQUEST_HEADER_LEN, PAGE_SIZE) + 2, GFP_KERNEL); } @@ -489,8 +498,8 @@ static void uvcg_video_pump(struct work_struct *work) */ int uvcg_video_enable(struct uvc_video *video, int enable) { - unsigned int i; int ret; + struct uvc_request *ureq; if (video->ep == NULL) { uvcg_info(&video->uvc->func, @@ -502,9 +511,10 @@ int uvcg_video_enable(struct uvc_video *video, int enable) cancel_work_sync(&video->pump); uvcg_queue_cancel(&video->queue, 0); - for (i = 0; i < video->uvc_num_requests; ++i) - if (video->ureq && video->ureq[i].req) - usb_ep_dequeue(video->ep, video->ureq[i].req); + list_for_each_entry(ureq, &video->ureqs, list) { + if (ureq->req) + usb_ep_dequeue(video->ep, ureq->req); + } uvc_video_free_requests(video); uvcg_queue_enable(&video->queue, 0); @@ -536,6 +546,7 @@ int uvcg_video_enable(struct uvc_video *video, int enable) */ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc) { + INIT_LIST_HEAD(&video->ureqs); INIT_LIST_HEAD(&video->req_free); spin_lock_init(&video->req_lock); INIT_WORK(&video->pump, uvcg_video_pump); From patchwork Wed Oct 18 19:46:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Avichal Rakesh X-Patchwork-Id: 155108 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2908:b0:403:3b70:6f57 with SMTP id ib8csp5028558vqb; Wed, 18 Oct 2023 12:47:03 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHLezMgR7w/5aBgpGCsZmvmsk/8bZS5U47k0HXl6Q1rHUdZj1IATFz5Z3xg+Wj3ah/OOSNw X-Received: by 2002:a17:90b:3b8f:b0:27d:12e1:7e2c with SMTP id pc15-20020a17090b3b8f00b0027d12e17e2cmr500846pjb.19.1697658423654; Wed, 18 Oct 2023 12:47:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697658423; cv=none; d=google.com; s=arc-20160816; b=Ge+ege5WDeDJKY8bSOsUYXGmQekalymt0qWgiNqhE8TIQ99S08KK4Vo6gyMUBkmMcL uMn+mnO8Fj9ISaqGHwzV15bNuvSiLmgdBEpgqNjC/mrN9tG46aPJwHcFUrPMGXeNjK3P zv/h8OetTg79bx+S2Q0Qn2Mz1MfUgbJDlNfXIZog5WN44U5S/uYQwtiSuDXFMK502gZP mN9dOnecsWyfoFpu/DbMEO3YyLEQrrvFASVysNu7kw0fBZHVOhFKe7PcfcUJjffOXpwE dhiiT5KpykfEwnajvSFJWf1/JF/Yx3z/DqSMGMG8HTK83VCC9ow5O7/5J8m2fDdM80lV zPzQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:dkim-signature; bh=bmULkbCQppb59vBxF7/p7Z1i+j8YHvsPaF2eqdRrrN4=; fh=E5X6/3uWCMdGSKLB7vuNn1z8ZeRDzUIfiV2I8BhU+4Q=; b=zpaNAPsb5q7Y4Tv5DMf9RYsGlc70v4eAXblEjlh6l7/XCDz5rcK7qfxjNvWRyM7y06 KKsER1oaNcAaLB8tf3O5Qw0XX7/+GX3w1jcPm7TAEqY2AcIuq2PiTigmiTx7+0YtJHR7 Zk10Bp2T73bDNc1Aoi0P7t1KROnjEU/fOwQ0LIYczPhVxmS5yp5yikkMm5QPrI1DzBfC d5DZVCof/Nw0pjdzWoPykcfqK/JkARe3FySE1x4XES3imPe3HH8nzqhGyvDBB0QKgMUl 2dRlI+GKWMU/PD+Rhcw8g7kMl5TJd6cpU+YkQ8/SlD6u9Z9EQGAE3VBfRDkzMGwDe2Nj rTJw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=F2wlT3QA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id qe3-20020a17090b4f8300b00278f81e54cdsi655111pjb.19.2023.10.18.12.47.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Oct 2023 12:47:03 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=F2wlT3QA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 9A45081BBA1E; Wed, 18 Oct 2023 12:47:02 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231713AbjJRTq5 (ORCPT + 24 others); Wed, 18 Oct 2023 15:46:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34420 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231527AbjJRTqy (ORCPT ); Wed, 18 Oct 2023 15:46:54 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E3AE8114 for ; Wed, 18 Oct 2023 12:46:50 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id 3f1490d57ef6-d9ab7badadeso10007444276.1 for ; Wed, 18 Oct 2023 12:46:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1697658410; x=1698263210; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=bmULkbCQppb59vBxF7/p7Z1i+j8YHvsPaF2eqdRrrN4=; b=F2wlT3QA6q0Zmw+MwaXwmId28vmmpR/1qgDltJdC99ASMCrfDSwsf0nkqP0QopqjEn Dvjt1+DsEEQ61ZBTCxYYGLuIIvgROIgLlKltgey8uq5ExOnut8FmhfTyzMNgyjdlSy1c Ekb8SrkKqDmuHYPuPUZWlV/9dmjhuw7hjhDANC5Sf8/bru2bCG+nYkYKonYHm4zWMhce mXh7HwdcHUNnNsQog+D0Rx0r5C4zS7CryyolZZHzq5Jko8jF23d9n02HCAE47c2xgSf3 stmSMs9u/MYWggzwJuSZ+h42Tq0R3dSgA+yWCHq0CiCWIJeSGcVVFgkpJeV6yo6XgmfG pW4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697658410; x=1698263210; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bmULkbCQppb59vBxF7/p7Z1i+j8YHvsPaF2eqdRrrN4=; b=Sk/BKvtMCkHbODWEClEp6ZVcDPOcMpuQw/1yW39WOp7BVBUAjyLWzDSQc96INbBXgz fMzpSfLGaxvKDHTgUQqVmL+fGstsyctg9jnorV2phrWXUcshHkGDUvHJiMJTtDjktXJO nQWQmlKQTXisRy95raHlqHeIHac3OLxzVzFG//mwE87Cc5nHBIR70SoOQtj2I26Br4Ta 1xnC5SRREcolKyVulU9HYGjFpOU+67NUssK8v5sKQkwrK2l+GGD10padPEg1acvGXqRg ov9OXwD4r8tvBHs+0PgU4lLGPmgJXzmzOzFBHMCEDkMG7//iRbEj925EUs1Xgp4yq7dc puzw== X-Gm-Message-State: AOJu0Yyp++GeQzLn3WjxlOElUaI61Nh8ePtk9sVqLfelMyhI+uHM/cnM DarJgAtNOQNByyzPWvg85//Iqt5eSPru X-Received: from hi-h2o-specialist.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:3cef]) (user=arakesh job=sendgmr) by 2002:a25:5ca:0:b0:d77:f7c3:37db with SMTP id 193-20020a2505ca000000b00d77f7c337dbmr7926ybf.8.1697658410148; Wed, 18 Oct 2023 12:46:50 -0700 (PDT) Date: Wed, 18 Oct 2023 12:46:38 -0700 In-Reply-To: <20231018194638.2446575-1-arakesh@google.com> Mime-Version: 1.0 References: <20230930184821.310143-1-arakesh@google.com> <20231018194638.2446575-1-arakesh@google.com> X-Mailer: git-send-email 2.42.0.758.gaed0368e0e-goog Message-ID: <20231018194638.2446575-3-arakesh@google.com> Subject: [PATCH v5 3/3] usb: gadget: uvc: Fix use-after-free for inflight usb_requests From: Avichal Rakesh To: arakesh@google.com, dan.scally@ideasonboard.com, gregkh@linuxfoundation.org, laurent.pinchart@ideasonboard.com, m.grzeschik@pengutronix.de Cc: etalvala@google.com, jchowdhary@google.com, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 18 Oct 2023 12:47:02 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1778517869169821913 X-GMAIL-MSGID: 1780123879259992529 Currently, the uvc gadget driver allocates all uvc_requests as one array and deallocates them all when the video stream stops. This includes de-allocating all the usb_requests associated with those uvc_requests. This can lead to use-after-free issues if any of those de-allocated usb_requests were still owned by the usb controller. This is patch 2 of 2 in fixing the use-after-free issue. It adds a new flag to uvc_video to track when frames and requests should be flowing. When disabling the video stream, the flag is tripped and, instead of de-allocating all uvc_requests and usb_requests, the gadget driver only de-allocates those usb_requests that are currently owned by it (as present in req_free). Other usb_requests are left untouched until their completion handler is called which takes care of freeing the usb_request and its corresponding uvc_request. Now that uvc_video does not depends on uvc->state, this patch removes unnecessary upates to uvc->state that were made to accommodate uvc_video logic. This should ensure that uvc gadget driver never accidentally de-allocates a usb_request that it doesn't own. Link: https://lore.kernel.org/7cd81649-2795-45b6-8c10-b7df1055020d@google.com Suggested-by: Michael Grzeschik Reviewed-by: Michael Grzeschik Tested-by: Michael Grzeschik Signed-off-by: Avichal Rakesh --- v1 -> v2: Rebased to ToT, and fixed deadlock reported in https://lore.kernel.org/all/ZRv2UnKztgyqk2pt@pengutronix.de/ v2 -> v3: Fix email threading goof-up v3 -> v4: re-rebase to ToT & moved to a uvc_video level lock as discussed in https://lore.kernel.org/b14b296f-2e08-4edf-aeea-1c5b621e2d0c@google.com/ v4 -> v5: Address review comments. Add Reviewed-by & Tested-by. drivers/usb/gadget/function/uvc.h | 1 + drivers/usb/gadget/function/uvc_v4l2.c | 12 +- drivers/usb/gadget/function/uvc_video.c | 154 +++++++++++++++++++----- 3 files changed, 128 insertions(+), 39 deletions(-) -- 2.42.0.758.gaed0368e0e-goog diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 993694da0bbc..be0d012aa244 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -102,6 +102,7 @@ struct uvc_video { unsigned int uvc_num_requests; /* Requests */ + bool is_enabled; /* tracks whether video stream is enabled */ unsigned int req_size; struct list_head ureqs; /* all uvc_requests allocated by uvc_video */ struct list_head req_free; diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index 7cb8d027ff0c..f4d2e24835d4 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -451,8 +451,8 @@ uvc_v4l2_streamon(struct file *file, void *fh, enum v4l2_buf_type type) * Complete the alternate setting selection setup phase now that * userspace is ready to provide video frames. */ - uvc_function_setup_continue(uvc, 0); uvc->state = UVC_STATE_STREAMING; + uvc_function_setup_continue(uvc, 0); return 0; } @@ -468,11 +468,11 @@ uvc_v4l2_streamoff(struct file *file, void *fh, enum v4l2_buf_type type) if (type != video->queue.queue.type) return -EINVAL; - uvc->state = UVC_STATE_CONNECTED; ret = uvcg_video_enable(video, 0); if (ret < 0) return ret; + uvc->state = UVC_STATE_CONNECTED; uvc_function_setup_continue(uvc, 1); return 0; } @@ -507,14 +507,6 @@ uvc_v4l2_subscribe_event(struct v4l2_fh *fh, static void uvc_v4l2_disable(struct uvc_device *uvc) { uvc_function_disconnect(uvc); - /* - * Drop uvc->state to CONNECTED if it was streaming before. - * This ensures that the usb_requests are no longer queued - * to the controller. - */ - if (uvc->state == UVC_STATE_STREAMING) - uvc->state = UVC_STATE_CONNECTED; - uvcg_video_enable(&uvc->video, 0); uvcg_free_buffers(&uvc->video.queue); uvc->func_connected = false; diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index c180866c8e34..3a5192f10f95 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -227,6 +227,9 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video, * Request handling */ +/** + * Must be called with req_lock held as it modifies the list ureq is held in + */ static void uvc_video_free_request(struct uvc_request *ureq, struct usb_ep *ep) { @@ -271,9 +274,25 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) struct uvc_request *ureq = req->context; struct uvc_video *video = ureq->video; struct uvc_video_queue *queue = &video->queue; - struct uvc_device *uvc = video->uvc; + struct uvc_buffer *last_buf = NULL; unsigned long flags; + spin_lock_irqsave(&video->req_lock, flags); + if (!video->is_enabled) { + /* + * When is_enabled is false, uvc_video_disable ensures that + * in-flight uvc_buffers are returned, so we can safely + * call free_request without worrying about last_buf. + */ + uvc_video_free_request(ureq, ep); + spin_unlock_irqrestore(&video->req_lock, flags); + return; + } + + last_buf = ureq->last_buf; + ureq->last_buf = NULL; + spin_unlock_irqrestore(&video->req_lock, flags); + switch (req->status) { case 0: break; @@ -295,17 +314,26 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) uvcg_queue_cancel(queue, 0); } - if (ureq->last_buf) { - uvcg_complete_buffer(&video->queue, ureq->last_buf); - ureq->last_buf = NULL; + if (last_buf) { + spin_lock_irqsave(&queue->irqlock, flags); + uvcg_complete_buffer(&video->queue, last_buf); + spin_unlock_irqrestore(&queue->irqlock, flags); } spin_lock_irqsave(&video->req_lock, flags); - list_add_tail(&req->list, &video->req_free); - spin_unlock_irqrestore(&video->req_lock, flags); - - if (uvc->state == UVC_STATE_STREAMING) + /* + * Video stream might have been disabled while we were + * processing the current usb_request. So make sure + * we're still streaming before queueing the usb_request + * back to req_free + */ + if (video->is_enabled) { + list_add_tail(&req->list, &video->req_free); queue_work(video->async_wq, &video->pump); + } else { + uvc_video_free_request(ureq, ep); + } + spin_unlock_irqrestore(&video->req_lock, flags); } static int @@ -393,20 +421,22 @@ static void uvcg_video_pump(struct work_struct *work) struct uvc_video_queue *queue = &video->queue; /* video->max_payload_size is only set when using bulk transfer */ bool is_bulk = video->max_payload_size; - struct uvc_device *uvc = video->uvc; struct usb_request *req = NULL; struct uvc_buffer *buf; unsigned long flags; bool buf_done; int ret; - while (uvc->state == UVC_STATE_STREAMING && video->ep->enabled) { + while (true) { + if (!video->ep->enabled) + return; + /* - * Retrieve the first available USB request, protected by the - * request lock. + * Check is_enabled and retrieve the first available USB + * request, protected by the request lock. */ spin_lock_irqsave(&video->req_lock, flags); - if (list_empty(&video->req_free)) { + if (!video->is_enabled || list_empty(&video->req_free)) { spin_unlock_irqrestore(&video->req_lock, flags); return; } @@ -488,9 +518,79 @@ static void uvcg_video_pump(struct work_struct *work) return; spin_lock_irqsave(&video->req_lock, flags); - list_add_tail(&req->list, &video->req_free); + if (video->is_enabled) + list_add_tail(&req->list, &video->req_free); + else + uvc_video_free_request(req->context, video->ep); spin_unlock_irqrestore(&video->req_lock, flags); - return; +} + +/* + * Disable video stream + */ +static int +uvcg_video_disable(struct uvc_video *video) +{ + unsigned long flags; + struct list_head inflight_bufs; + struct usb_request *req, *temp; + struct uvc_buffer *buf, *btemp; + struct uvc_request *ureq, *utemp; + + INIT_LIST_HEAD(&inflight_bufs); + spin_lock_irqsave(&video->req_lock, flags); + video->is_enabled = false; + + /* + * Remove any in-flight buffers from the uvc_requests + * because we want to return them before cancelling the + * queue. This ensures that we aren't stuck waiting for + * all complete callbacks to come through before disabling + * vb2 queue. + */ + list_for_each_entry(ureq, &video->ureqs, list) { + if (ureq->last_buf) { + list_add_tail(&ureq->last_buf->queue, &inflight_bufs); + ureq->last_buf = NULL; + } + } + spin_unlock_irqrestore(&video->req_lock, flags); + + cancel_work_sync(&video->pump); + uvcg_queue_cancel(&video->queue, 0); + + spin_lock_irqsave(&video->req_lock, flags); + /* + * Remove all uvc_reqeusts from ureqs with list_del_init + * This lets uvc_video_free_request correctly identify + * if the uvc_request is attached to a list or not when freeing + * memory. + */ + list_for_each_entry_safe(ureq, utemp, &video->ureqs, list) + list_del_init(&ureq->list); + + list_for_each_entry_safe(req, temp, &video->req_free, list) { + list_del(&req->list); + uvc_video_free_request(req->context, video->ep); + } + + INIT_LIST_HEAD(&video->ureqs); + INIT_LIST_HEAD(&video->req_free); + video->req_size = 0; + spin_unlock_irqrestore(&video->req_lock, flags); + + /* + * Return all the video buffers before disabling the queue. + */ + spin_lock_irqsave(&video->queue.irqlock, flags); + list_for_each_entry_safe(buf, btemp, &inflight_bufs, queue) { + list_del(&buf->queue); + uvcg_complete_buffer(&video->queue, buf); + } + spin_unlock_irqrestore(&video->queue.irqlock, flags); + + uvcg_queue_enable(&video->queue, 0); + return 0; } /* @@ -499,27 +599,22 @@ static void uvcg_video_pump(struct work_struct *work) int uvcg_video_enable(struct uvc_video *video, int enable) { int ret; - struct uvc_request *ureq; if (video->ep == NULL) { uvcg_info(&video->uvc->func, "Video enable failed, device is uninitialized.\n"); return -ENODEV; } + if (!enable) + return uvcg_video_disable(video); - if (!enable) { - cancel_work_sync(&video->pump); - uvcg_queue_cancel(&video->queue, 0); - - list_for_each_entry(ureq, &video->ureqs, list) { - if (ureq->req) - usb_ep_dequeue(video->ep, ureq->req); - } - - uvc_video_free_requests(video); - uvcg_queue_enable(&video->queue, 0); - return 0; - } + /* + * Safe to access request related fields without req_lock because + * this is the only thread currently active, and no other + * request handling thread will become active until this function + * returns. + */ + video->is_enabled = true; if ((ret = uvcg_queue_enable(&video->queue, 1)) < 0) return ret; @@ -546,6 +641,7 @@ int uvcg_video_enable(struct uvc_video *video, int enable) */ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc) { + video->is_enabled = false; INIT_LIST_HEAD(&video->ureqs); INIT_LIST_HEAD(&video->req_free); spin_lock_init(&video->req_lock);