linux-kernel - [PATCH 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request

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-next>] [day] [month] [year] [list]

Date:   Tue,  9 Oct 2018 22:48:57 -0400
From:   Paul Elder <paul.elder@...asonboard.com>
To:     laurent.pinchart@...asonboard.com, kieran.bingham@...asonboard.com
Cc:     Paul Elder <paul.elder@...asonboard.com>, b-liu@...com,
        gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, balbi@...nel.org,
        stern@...land.harvard.edu, rogerq@...com
Subject: [PATCH 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request

This patch series adds a mechanism to allow asynchronously validating
the data stage of a control out request, and for stalling or suceeding
the request accordingly. This mechanism is implemented for MUSB, and is
used by UVC. At the same time, UVC packages the setup stage and data
stage data together to send to userspace to save on a pair of context
switches per control out request.

This patch series does change the userspace API. We however believe that
it is justified because the current API is broken, and because it isn't
being used (because it's broken).

The current API is broken such that it is subject to race conditions
that cause fatal errors with a high frequency. This is actually what
motivated this patch series in the first place. In the current API, not
only is there no way to asynchronously validate the data stage of a
control OUT request, but an empty buffer is expected to be provided to
hold the data stage data -- which is more likely than not to be late.
There is even a warning in musb_g_ep0_queue:

/* else for sequence #2 (OUT), caller provides a buffer
 * before the next packet arrives.  deferred responses
 * (after SETUP is acked) are racey.
 */

This problem has never been reported in years, which is a sign that the
API isn't used. Furthermore, the vendor kernels that we have seen using
the UVC gadget driver (such as QC and Huawei) are heavily patched with
local changes to the API. This corroborates the suspicion that the
current mainline API is not being used.

Additionally, this API isn't meant to be used by generic applications,
but by a dedicated userspace helper. uvc-gadget is one such example, but
it has bitrotten and isn't compatible with the current kernel API. The
fact that nobody has submitted patches nor complained for a long time
again shows that it isn't being used.

The conclusion is that since the API hasn't been used for a long time,
it is safe to fix it.

Paul Elder (6):
  usb: uvc: include videodev2.h in g_uvc.h
  usb: gadget: uvc: enqueue usb request in setup handler for control OUT
  usb: gadget: uvc: package setup and data for control OUT requests
  usb: gadget: add functions to signal udc driver to delay status stage
  usb: musb: gadget: implement send_response
  usb: gadget: uvc: allow ioctl to send response in status stage

 drivers/usb/gadget/function/f_uvc.c    | 33 ++++++++++++++++-----
 drivers/usb/gadget/function/uvc.h      |  1 +
 drivers/usb/gadget/function/uvc_v4l2.c | 21 +++++++++++++
 drivers/usb/gadget/udc/core.c          | 40 +++++++++++++++++++++++++
 drivers/usb/musb/musb_gadget_ep0.c     | 41 ++++++++++++++++++++++++++
 include/linux/usb/gadget.h             | 11 +++++++
 include/uapi/linux/usb/g_uvc.h         |  4 ++-
 7 files changed, 142 insertions(+), 9 deletions(-)

-- 
2.18.0