linux-kernel - [PATCH v3 0/4] usb: gadget: uvc: fix racing between uvc_function_set

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]

Message-Id: <20190109071039.27702-1-paul.elder@ideasonboard.com>
Date:   Wed,  9 Jan 2019 02:10:35 -0500
From:   Paul Elder <paul.elder@...asonboard.com>
To:     laurent.pinchart@...asonboard.com, kieran.bingham@...asonboard.com
Cc:     Paul Elder <paul.elder@...asonboard.com>, rogerq@...com,
        balbi@...nel.org, gregkh@...uxfoundation.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH v3 0/4] usb: gadget: uvc: fix racing between uvc_function_set_alt and streamon/off

Down the call stack from the ioctl handler for VIDIOC_STREAMON,
uvc_video_alloc_requests contains a BUG_ON, which in the high level,
triggers when VIDIOC_STREAMON ioctl is issued without VIDIOC_STREAMOFF
being issued previously.

This can happen in a few ways, such as if the userspace uvc gadget
application simply doesn't issue VIDIOC_STREAMOFF. Another way is if
uvc_function_set_alt with alt 0 is called after it is called with 1 but
before VIDIOC_STREAMON is called; in this case, UVC_EVENT_STREAMOFF will
not be queued to userspace, and therefore userspace will never call
VIDIOC_STREAMOFF.

To fix this, add two more uvc states: starting and stopping. The
starting state is entered when uvc_function_set_alt 1 is called, and is
exited in uvc_v4l2_streamon, when the state is changed to streaming. The
stopping state is entered when uvc_function_set_alt 0 is called, and is
exited in uvc_v4l2_streamoff, when the state is changed to connected.

The status phase of the SET_INTERFACE request doesn't need to be delayed
by the uvc gadget driver, so that is removed.

Finally, there is another way to trigger the aforementioned BUG: start
streaming and (physically) disconnect usb. To fix this, call
uvcg_video_enable 0 in uvc_function_disable.

Changes in v3:

- add state guard to uvc_function_set_alt 1
- add documentation for newly added uvc states
- reorder uvc states to more or less follow the flow diagram
- add more state guards to ioctl handlers for streamon and streamoff
- added interrupt-safe uvcg_video_cancel and used instead of the
  non-interrupt-save uvcg_video_enable 0 in uvc_function_disable

Changes in v2:
	1. Remove delay usb status phase

Paul Elder (4):
  usb: gadget: uvc: synchronize streamon/off with uvc_function_set_alt
  usb: gadget: uvc: don't delay the status phase of non-zero
    SET_INTERFACE requests
  usb: gadget: uvc: disable stream when disconnected
  usb: gadget: uvc: remove unused/duplicate function prototypes from
    uvc.h

 drivers/usb/gadget/function/f_uvc.c     | 23 ++++++++----
 drivers/usb/gadget/function/uvc.h       | 47 +++++++++++++++++++------
 drivers/usb/gadget/function/uvc_v4l2.c  | 28 +++++++++++----
 drivers/usb/gadget/function/uvc_video.c | 13 +++++++
 drivers/usb/gadget/function/uvc_video.h |  2 ++
 5 files changed, 91 insertions(+), 22 deletions(-)

-- 
2.20.1