Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: himadrispandya@gmail.com, dvyukov@google.com, linux-usb@vger.kernel.org
Cc: alsa-devel@alsa-project.org, johan.hedberg@gmail.com,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	marcel@holtmann.org, linux-kernel@vger.kernel.org,
	tiwai@suse.com, stern@rowland.harvard.ed,
	linux-bluetooth@vger.kernel.org
Subject: [PATCH v3 00/11] USB: new USB control message helper functions
Date: Mon, 14 Sep 2020 17:37:45 +0200
Message-ID: <20200914153756.3412156-1-gregkh@linuxfoundation.org> (raw)

In a recent discussion about a USB networking bug found by syzbot, and
fixed by Himadri Pandya, the design of the existing usb_control_msg()
call was brought up as not being the "nicest" thing to use by Dmitry
Vyukov:
        https://lore.kernel.org/r/CACT4Y+YbDODLRFn8M5QcY4CazhpeCaunJnP_udXtAs0rYoASSg@mail.gmail.com

The function makes it hard to get right, in that it will return the
number of bytes sent/received, but almost no one checks to see if a
short read/write happens.  With a malicious, or broken, USB device, this
can cause drivers to act on data that they did not anticipate, and
sometimes copy internal kernel data out to userspace.

So let's fix this up by creating two new functions,
usb_control_msg_send() and usb_control_msg_recv().  These functions
either complete the full transation, or they return an error, a short
send/recv is now an error.

They also accept data off of the stack, saving individual drivers the
pain of having to constantly allocate memory on their own for tiny
messages, thereby saving overall kernel code space.

The api also does not require a raw USB "pipe" to be sent to the
function, as we know the direction, so just pass in the endpoint number
instead, again making it easier on the USB driver author to use.

This series first takes a helper function out of the sound core for
verifying USB endpoints to be able to use internally, and then adds the
new functions, converts over some internal USB code to use them, and
then starts to clean up some drivers using these new functions, as an
example of the savings that can happen by using these functions.

Thanks to Dmitry and Himadri for the idea on how to do all of this.

greg k-h

-----

Changes from v2:
	- add Andy's reviewed-by: to patch 3
	- remove unneeded change in usb_enable_link_state() in hub.c
	  in patch 4

Changes from v1:
        - added acks from Takashi Iwai
        - dropped changes to one function in patch 04 thanks to review
          from Alan Stern
        - typo fix in comment in patch 01
        - added new patch 11 to remove some unneeded checks in the sound
          drivers for endpoint statuses that would always be true.



Greg Kroah-Hartman (11):
  USB: move snd_usb_pipe_sanity_check into the USB core
  USB: add usb_control_msg_send() and usb_control_msg_recv()
  USB: core: message.c: use usb_control_msg_send() in a few places
  USB: core: hub.c: use usb_control_msg_send() in a few places
  USB: legousbtower: use usb_control_msg_recv()
  sound: usx2y: move to use usb_control_msg_send()
  sound: 6fire: move to use usb_control_msg_send() and
    usb_control_msg_recv()
  sound: line6: move to use usb_control_msg_send() and
    usb_control_msg_recv()
  sound: hiface: move to use usb_control_msg_send()
  Bluetooth: ath3k: use usb_control_msg_send() and
    usb_control_msg_recv()
  ALSA: remove calls to usb_pipe_type_check for control endpoints

 drivers/bluetooth/ath3k.c       |  90 +++++------------
 drivers/usb/core/hub.c          |  99 ++++++++----------
 drivers/usb/core/message.c      | 171 ++++++++++++++++++++++++++++----
 drivers/usb/core/urb.c          |  31 ++++--
 drivers/usb/misc/legousbtower.c |  60 ++++-------
 include/linux/usb.h             |   7 ++
 sound/usb/6fire/firmware.c      |  38 +++----
 sound/usb/helper.c              |  16 +--
 sound/usb/helper.h              |   1 -
 sound/usb/hiface/pcm.c          |  14 ++-
 sound/usb/line6/driver.c        |  69 +++++--------
 sound/usb/line6/podhd.c         |  17 ++--
 sound/usb/line6/toneport.c      |   8 +-
 sound/usb/mixer_scarlett_gen2.c |   2 +-
 sound/usb/quirks.c              |  12 +--
 sound/usb/usx2y/us122l.c        |  42 ++------
 16 files changed, 336 insertions(+), 341 deletions(-)

-- 
2.28.0


             reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-14 15:37 Greg Kroah-Hartman [this message]
2020-09-14 15:37 ` [PATCH v3 01/11] USB: move snd_usb_pipe_sanity_check into the USB core Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 02/11] USB: add usb_control_msg_send() and usb_control_msg_recv() Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 03/11] USB: core: message.c: use usb_control_msg_send() in a few places Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 04/11] USB: core: hub.c: " Greg Kroah-Hartman
2020-09-14 18:06   ` Alan Stern
2020-09-16  9:06     ` Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 05/11] USB: legousbtower: use usb_control_msg_recv() Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 06/11] sound: usx2y: move to use usb_control_msg_send() Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 07/11] sound: 6fire: move to use usb_control_msg_send() and usb_control_msg_recv() Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 08/11] sound: line6: " Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 09/11] sound: hiface: move to use usb_control_msg_send() Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 10/11] Bluetooth: ath3k: use usb_control_msg_send() and usb_control_msg_recv() Greg Kroah-Hartman
2020-09-14 15:37 ` [PATCH v3 11/11] ALSA: remove calls to usb_pipe_type_check for control endpoints Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200914153756.3412156-1-gregkh@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=dvyukov@google.com \
    --cc=himadrispandya@gmail.com \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=stern@rowland.harvard.ed \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git