linux-kernel - Re: [PATCH 1/3] usb: gadget: clarify usage of USB_GADGET_DELAYED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 28 Aug 2023 10:44:09 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     andrey.konovalov@...ux.dev
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andrey Konovalov <andreyknvl@...il.com>,
        Felipe Balbi <balbi@...nel.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Pawel Laszczak <pawell@...ence.com>,
        Chunfeng Yun <chunfeng.yun@...iatek.com>,
        Minas Harutyunyan <hminas@...opsys.com>,
        Justin Chen <justin.chen@...adcom.com>,
        Al Cooper <alcooperx@...il.com>,
        Herve Codina <herve.codina@...tlin.com>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] usb: gadget: clarify usage of
 USB_GADGET_DELAYED_STATUS

On Mon, Aug 28, 2023 at 04:10:30AM +0200, andrey.konovalov@...ux.dev wrote:
> From: Andrey Konovalov <andreyknvl@...il.com>
> 
> USB_GADGET_DELAYED_STATUS was introduced in commit 1b9ba000177e ("usb:
> gadget: composite: Allow function drivers to pause control transfers").
> It was initially intended for the composite framework to allow delaying
> completing the status stage of a SET_CONFIGURATION request until all
> functions are ready.
> 
> Unfortunately, that commit had an unintended side-effect of returning
> USB_GADGET_DELAYED_STATUS from the ->setup() call of the composite
> framework gadget driver.
> 
> As a result of this and the incomplete documentation, some UDC drivers
> started relying on USB_GADGET_DELAYED_STATUS to decide when to avoid
> autocompleting the status stage for 0-length control transfers. dwc3 was
> the first in commit 5bdb1dcc6330 ("usb: dwc3: ep0: handle delayed_status
> again"). And a number of other UDC drivers followed later, probably
> relying on the dwc3 behavior as a reference.
> 
> Unfortunately, this violated the interface between the UDC and the
> gadget driver for 0-length control transfers: the UDC driver must only
> proceed with the status stage for a 0-length control transfer once the
> gadget driver queued a response to EP0.
> 
> As a result, a few gadget drivers are partially broken when used with
> a UDC that only delays the status stage for 0-length transfers when
> USB_GADGET_DELAYED_STATUS is returned from the setup() callback.
> 
> This includes Raw Gadget and GadgetFS. For FunctionFS, a workaround was
> added in commit 946ef68ad4e4 ("usb: gadget: ffs: Let setup() return
> USB_GADGET_DELAYED_STATUS") and commit 4d644abf2569 ("usb: gadget: f_fs:
> Only return delayed status when len is 0").
> 
> The proper solution to this issue would be to contain
> USB_GADGET_DELAYED_STATUS within the composite framework and make all
> UDC drivers to not complete the status stage for 0-length requests on
> their own.
> 
> Unfortunately, there is quite a few UDC drivers that need to get fixed
> and the required changes for some of them are not trivial.
> 
> For now, update the comments to clarify that USB_GADGET_DELAYED_STATUS
> must not be used by the UDC drivers.
> 
> The following two commits also add workarounds to Raw Gadget and GadgetFS
> to make them compatible with the broken UDC drivers until they are fixed.
> 
> Signed-off-by: Andrey Konovalov <andreyknvl@...il.com>
> ---

Acked-by: Alan Stern <stern@...land.harvard.edu>