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
Greg Kroah-Hartman (10):
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()
drivers/bluetooth/ath3k.c | 90 +++++------------
drivers/usb/core/hub.c | 128 +++++++++---------------
drivers/usb/core/message.c | 171 ++++++++++++++++++++++++++++----
drivers/usb/core/urb.c | 29 ++++--
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, 345 insertions(+), 359 deletions(-)
--
2.28.0
snd_usb_pipe_sanity_check() is a great function, so let's move it into
the USB core so that other parts of the kernel, including the USB core,
can call it.
Name it usb_pipe_type_check() to match the existing
usb_urb_ep_type_check() call, which now uses this function.
Cc: Jaroslav Kysela <[email protected]>
Cc: Takashi Iwai <[email protected]>
Cc: "Gustavo A. R. Silva" <[email protected]>
Cc: Eli Billauer <[email protected]>
Cc: Emiliano Ingrassia <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Alexander Tsoy <[email protected]>
Cc: "Geoffrey D. Bennett" <[email protected]>
Cc: Jussi Laako <[email protected]>
Cc: Nick Kossifidis <[email protected]>
Cc: Dmitry Panchenko <[email protected]>
Cc: Chris Wulff <[email protected]>
Cc: Jesus Ramos <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/usb/core/urb.c | 29 ++++++++++++++++++++++-------
include/linux/usb.h | 1 +
sound/usb/helper.c | 16 +---------------
sound/usb/helper.h | 1 -
sound/usb/mixer_scarlett_gen2.c | 2 +-
sound/usb/quirks.c | 12 ++++++------
6 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 27e83e55a590..45bc2914c1ba 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -192,24 +192,39 @@ static const int pipetypes[4] = {
};
/**
- * usb_urb_ep_type_check - sanity check of endpoint in the given urb
- * @urb: urb to be checked
+ * usb_pipe_type_check - sanity check of a specific pipe for a usb device
+ * @dev: struct usb_device to be checked
+ * @pipe: pipe to check
*
* This performs a light-weight sanity check for the endpoint in the
- * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
- * a negative error code.
+ * given usb device. It returns 0 if the pipe is a valid for the specific usb
+ * device, otherwise a negative error code.
*/
-int usb_urb_ep_type_check(const struct urb *urb)
+int usb_pipe_type_check(struct usb_device *dev, unsigned int pipe)
{
const struct usb_host_endpoint *ep;
- ep = usb_pipe_endpoint(urb->dev, urb->pipe);
+ ep = usb_pipe_endpoint(dev, pipe);
if (!ep)
return -EINVAL;
- if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
+ if (usb_pipetype(pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
}
+EXPORT_SYMBOL_GPL(usb_pipe_type_check);
+
+/**
+ * usb_urb_ep_type_check - sanity check of endpoint in the given urb
+ * @urb: urb to be checked
+ *
+ * This performs a light-weight sanity check for the endpoint in the
+ * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
+ * a negative error code.
+ */
+int usb_urb_ep_type_check(const struct urb *urb)
+{
+ return usb_pipe_type_check(urb->dev, urb->pipe);
+}
EXPORT_SYMBOL_GPL(usb_urb_ep_type_check);
/**
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 20c555db4621..0b3963d7ec38 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1764,6 +1764,7 @@ static inline int usb_urb_dir_out(struct urb *urb)
return (urb->transfer_flags & URB_DIR_MASK) == URB_DIR_OUT;
}
+int usb_pipe_type_check(struct usb_device *dev, unsigned int pipe);
int usb_urb_ep_type_check(const struct urb *urb);
void *usb_alloc_coherent(struct usb_device *dev, size_t size,
diff --git a/sound/usb/helper.c b/sound/usb/helper.c
index 4c12cc5b53fd..cf92d7110773 100644
--- a/sound/usb/helper.c
+++ b/sound/usb/helper.c
@@ -63,20 +63,6 @@ void *snd_usb_find_csint_desc(void *buffer, int buflen, void *after, u8 dsubtype
return NULL;
}
-/* check the validity of pipe and EP types */
-int snd_usb_pipe_sanity_check(struct usb_device *dev, unsigned int pipe)
-{
- static const int pipetypes[4] = {
- PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
- };
- struct usb_host_endpoint *ep;
-
- ep = usb_pipe_endpoint(dev, pipe);
- if (!ep || usb_pipetype(pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
- return -EINVAL;
- return 0;
-}
-
/*
* Wrapper for usb_control_msg().
* Allocates a temp buffer to prevent dmaing from/to the stack.
@@ -89,7 +75,7 @@ int snd_usb_ctl_msg(struct usb_device *dev, unsigned int pipe, __u8 request,
void *buf = NULL;
int timeout;
- if (snd_usb_pipe_sanity_check(dev, pipe))
+ if (usb_pipe_type_check(dev, pipe))
return -EINVAL;
if (size > 0) {
diff --git a/sound/usb/helper.h b/sound/usb/helper.h
index 5e8a18b4e7b9..f5b4c6647e4d 100644
--- a/sound/usb/helper.h
+++ b/sound/usb/helper.h
@@ -7,7 +7,6 @@ unsigned int snd_usb_combine_bytes(unsigned char *bytes, int size);
void *snd_usb_find_desc(void *descstart, int desclen, void *after, u8 dtype);
void *snd_usb_find_csint_desc(void *descstart, int desclen, void *after, u8 dsubtype);
-int snd_usb_pipe_sanity_check(struct usb_device *dev, unsigned int pipe);
int snd_usb_ctl_msg(struct usb_device *dev, unsigned int pipe,
__u8 request, __u8 requesttype, __u16 value, __u16 index,
void *data, __u16 size);
diff --git a/sound/usb/mixer_scarlett_gen2.c b/sound/usb/mixer_scarlett_gen2.c
index 0ffff7640892..9609c6d9655c 100644
--- a/sound/usb/mixer_scarlett_gen2.c
+++ b/sound/usb/mixer_scarlett_gen2.c
@@ -1978,7 +1978,7 @@ static int scarlett2_mixer_status_create(struct usb_mixer_interface *mixer)
return 0;
}
- if (snd_usb_pipe_sanity_check(dev, pipe))
+ if (usb_pipe_type_check(dev, pipe))
return -EINVAL;
mixer->urb = usb_alloc_urb(0, GFP_KERNEL);
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index abf99b814a0f..fc3aab04a0bc 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -846,7 +846,7 @@ static int snd_usb_accessmusic_boot_quirk(struct usb_device *dev)
static const u8 seq[] = { 0x4e, 0x73, 0x52, 0x01 };
void *buf;
- if (snd_usb_pipe_sanity_check(dev, usb_sndintpipe(dev, 0x05)))
+ if (usb_pipe_type_check(dev, usb_sndintpipe(dev, 0x05)))
return -EINVAL;
buf = kmemdup(seq, ARRAY_SIZE(seq), GFP_KERNEL);
if (!buf)
@@ -875,7 +875,7 @@ static int snd_usb_nativeinstruments_boot_quirk(struct usb_device *dev)
{
int ret;
- if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
+ if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
return -EINVAL;
ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
0xaf, USB_TYPE_VENDOR | USB_RECIP_DEVICE,
@@ -984,7 +984,7 @@ static int snd_usb_axefx3_boot_quirk(struct usb_device *dev)
dev_dbg(&dev->dev, "Waiting for Axe-Fx III to boot up...\n");
- if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
+ if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
return -EINVAL;
/* If the Axe-Fx III has not fully booted, it will timeout when trying
* to enable the audio streaming interface. A more generous timeout is
@@ -1018,7 +1018,7 @@ static int snd_usb_motu_microbookii_communicate(struct usb_device *dev, u8 *buf,
{
int err, actual_length;
- if (snd_usb_pipe_sanity_check(dev, usb_sndintpipe(dev, 0x01)))
+ if (usb_pipe_type_check(dev, usb_sndintpipe(dev, 0x01)))
return -EINVAL;
err = usb_interrupt_msg(dev, usb_sndintpipe(dev, 0x01), buf, *length,
&actual_length, 1000);
@@ -1030,7 +1030,7 @@ static int snd_usb_motu_microbookii_communicate(struct usb_device *dev, u8 *buf,
memset(buf, 0, buf_size);
- if (snd_usb_pipe_sanity_check(dev, usb_rcvintpipe(dev, 0x82)))
+ if (usb_pipe_type_check(dev, usb_rcvintpipe(dev, 0x82)))
return -EINVAL;
err = usb_interrupt_msg(dev, usb_rcvintpipe(dev, 0x82), buf, buf_size,
&actual_length, 1000);
@@ -1117,7 +1117,7 @@ static int snd_usb_motu_m_series_boot_quirk(struct usb_device *dev)
{
int ret;
- if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
+ if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
return -EINVAL;
ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
1, USB_TYPE_VENDOR | USB_RECIP_DEVICE,
--
2.28.0
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
sound/usb/line6/driver.c | 69 +++++++++++++++-----------------------
sound/usb/line6/podhd.c | 17 ++++------
sound/usb/line6/toneport.c | 8 ++---
3 files changed, 37 insertions(+), 57 deletions(-)
diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index 60674ce4879b..601292c51491 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -337,23 +337,18 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
{
struct usb_device *usbdev = line6->usbdev;
int ret;
- unsigned char *len;
+ u8 len;
unsigned count;
if (address > 0xffff || datalen > 0xff)
return -EINVAL;
- len = kmalloc(1, GFP_KERNEL);
- if (!len)
- return -ENOMEM;
-
/* query the serial number: */
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
- (datalen << 8) | 0x21, address,
- NULL, 0, LINE6_TIMEOUT * HZ);
-
- if (ret < 0) {
+ ret = usb_control_msg_send(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ (datalen << 8) | 0x21, address, NULL, 0,
+ LINE6_TIMEOUT * HZ);
+ if (ret) {
dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
goto exit;
}
@@ -362,45 +357,41 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
mdelay(LINE6_READ_WRITE_STATUS_DELAY);
- ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE |
- USB_DIR_IN,
- 0x0012, 0x0000, len, 1,
- LINE6_TIMEOUT * HZ);
- if (ret < 0) {
+ ret = usb_control_msg_recv(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+ 0x0012, 0x0000, &len, 1,
+ LINE6_TIMEOUT * HZ);
+ if (ret) {
dev_err(line6->ifcdev,
"receive length failed (error %d)\n", ret);
goto exit;
}
- if (*len != 0xff)
+ if (len != 0xff)
break;
}
ret = -EIO;
- if (*len == 0xff) {
+ if (len == 0xff) {
dev_err(line6->ifcdev, "read failed after %d retries\n",
count);
goto exit;
- } else if (*len != datalen) {
+ } else if (len != datalen) {
/* should be equal or something went wrong */
dev_err(line6->ifcdev,
"length mismatch (expected %d, got %d)\n",
- (int)datalen, (int)*len);
+ (int)datalen, len);
goto exit;
}
/* receive the result: */
- ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
- 0x0013, 0x0000, data, datalen,
- LINE6_TIMEOUT * HZ);
-
- if (ret < 0)
+ ret = usb_control_msg_recv(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+ 0x0013, 0x0000, data, datalen, LINE6_TIMEOUT * HZ);
+ if (ret)
dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
exit:
- kfree(len);
return ret;
}
EXPORT_SYMBOL_GPL(line6_read_data);
@@ -423,12 +414,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
if (!status)
return -ENOMEM;
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
- 0x0022, address, data, datalen,
- LINE6_TIMEOUT * HZ);
-
- if (ret < 0) {
+ ret = usb_control_msg_send(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ 0x0022, address, data, datalen, LINE6_TIMEOUT * HZ);
+ if (ret) {
dev_err(line6->ifcdev,
"write request failed (error %d)\n", ret);
goto exit;
@@ -437,14 +426,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
mdelay(LINE6_READ_WRITE_STATUS_DELAY);
- ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
- 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE |
- USB_DIR_IN,
- 0x0012, 0x0000,
- status, 1, LINE6_TIMEOUT * HZ);
-
- if (ret < 0) {
+ ret = usb_control_msg_recv(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+ 0x0012, 0x0000, status, 1, LINE6_TIMEOUT * HZ);
+ if (ret) {
dev_err(line6->ifcdev,
"receiving status failed (error %d)\n", ret);
goto exit;
diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c
index eef45f7fef0d..a1261f55d62b 100644
--- a/sound/usb/line6/podhd.c
+++ b/sound/usb/line6/podhd.c
@@ -183,29 +183,25 @@ static const struct attribute_group podhd_dev_attr_group = {
static int podhd_dev_start(struct usb_line6_podhd *pod)
{
int ret;
- u8 *init_bytes;
+ u8 init_bytes[8];
int i;
struct usb_device *usbdev = pod->line6.usbdev;
- init_bytes = kmalloc(8, GFP_KERNEL);
- if (!init_bytes)
- return -ENOMEM;
-
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+ ret = usb_control_msg_send(usbdev, 0,
0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
0x11, 0,
NULL, 0, LINE6_TIMEOUT * HZ);
- if (ret < 0) {
+ if (ret) {
dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
goto exit;
}
/* NOTE: looks like some kind of ping message */
- ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
+ ret = usb_control_msg_recv(usbdev, 0, 0x67,
USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
0x11, 0x0,
init_bytes, 3, LINE6_TIMEOUT * HZ);
- if (ret < 0) {
+ if (ret) {
dev_err(pod->line6.ifcdev,
"receive length failed (error %d)\n", ret);
goto exit;
@@ -220,13 +216,12 @@ static int podhd_dev_start(struct usb_line6_podhd *pod)
goto exit;
}
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+ ret = usb_control_msg_send(usbdev, 0,
USB_REQ_SET_FEATURE,
USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
1, 0,
NULL, 0, LINE6_TIMEOUT * HZ);
exit:
- kfree(init_bytes);
return ret;
}
diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c
index 94dd5e7ab2e6..a9b56085b76a 100644
--- a/sound/usb/line6/toneport.c
+++ b/sound/usb/line6/toneport.c
@@ -126,11 +126,11 @@ static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2)
{
int ret;
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
- cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
+ ret = usb_control_msg_send(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
- if (ret < 0) {
+ if (ret) {
dev_err(&usbdev->dev, "send failed (error %d)\n", ret);
return ret;
}
--
2.28.0
The usb_control_msg_recv() function can handle data on the stack, as
well as properly detecting short reads, so move to use that function
instead of the older usb_control_msg() call. This ends up removing a
lot of extra lines in the driver.
Cc: Juergen Stuber <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/usb/misc/legousbtower.c | 60 +++++++++++----------------------
1 file changed, 19 insertions(+), 41 deletions(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index f922544056de..c3583df4c324 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -308,15 +308,9 @@ static int tower_open(struct inode *inode, struct file *file)
int subminor;
int retval = 0;
struct usb_interface *interface;
- struct tower_reset_reply *reset_reply;
+ struct tower_reset_reply reset_reply;
int result;
- reset_reply = kmalloc(sizeof(*reset_reply), GFP_KERNEL);
- if (!reset_reply) {
- retval = -ENOMEM;
- goto exit;
- }
-
nonseekable_open(inode, file);
subminor = iminor(inode);
@@ -347,15 +341,11 @@ static int tower_open(struct inode *inode, struct file *file)
}
/* reset the tower */
- result = usb_control_msg(dev->udev,
- usb_rcvctrlpipe(dev->udev, 0),
- LEGO_USB_TOWER_REQUEST_RESET,
- USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
- 0,
- 0,
- reset_reply,
- sizeof(*reset_reply),
- 1000);
+ result = usb_control_msg_recv(dev->udev, 0,
+ LEGO_USB_TOWER_REQUEST_RESET,
+ USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
+ 0, 0,
+ &reset_reply, sizeof(reset_reply), 1000);
if (result < 0) {
dev_err(&dev->udev->dev,
"LEGO USB Tower reset control request failed\n");
@@ -394,7 +384,6 @@ static int tower_open(struct inode *inode, struct file *file)
mutex_unlock(&dev->lock);
exit:
- kfree(reset_reply);
return retval;
}
@@ -753,7 +742,7 @@ static int tower_probe(struct usb_interface *interface, const struct usb_device_
struct device *idev = &interface->dev;
struct usb_device *udev = interface_to_usbdev(interface);
struct lego_usb_tower *dev;
- struct tower_get_version_reply *get_version_reply = NULL;
+ struct tower_get_version_reply get_version_reply;
int retval = -ENOMEM;
int result;
@@ -798,34 +787,25 @@ static int tower_probe(struct usb_interface *interface, const struct usb_device_
dev->interrupt_in_interval = interrupt_in_interval ? interrupt_in_interval : dev->interrupt_in_endpoint->bInterval;
dev->interrupt_out_interval = interrupt_out_interval ? interrupt_out_interval : dev->interrupt_out_endpoint->bInterval;
- get_version_reply = kmalloc(sizeof(*get_version_reply), GFP_KERNEL);
- if (!get_version_reply) {
- retval = -ENOMEM;
- goto error;
- }
-
/* get the firmware version and log it */
- result = usb_control_msg(udev,
- usb_rcvctrlpipe(udev, 0),
- LEGO_USB_TOWER_REQUEST_GET_VERSION,
- USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
- 0,
- 0,
- get_version_reply,
- sizeof(*get_version_reply),
- 1000);
- if (result != sizeof(*get_version_reply)) {
- if (result >= 0)
- result = -EIO;
+ result = usb_control_msg_recv(udev, 0,
+ LEGO_USB_TOWER_REQUEST_GET_VERSION,
+ USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
+ 0,
+ 0,
+ &get_version_reply,
+ sizeof(get_version_reply),
+ 1000);
+ if (!result) {
dev_err(idev, "get version request failed: %d\n", result);
retval = result;
goto error;
}
dev_info(&interface->dev,
"LEGO USB Tower firmware version is %d.%d build %d\n",
- get_version_reply->major,
- get_version_reply->minor,
- le16_to_cpu(get_version_reply->build_no));
+ get_version_reply.major,
+ get_version_reply.minor,
+ le16_to_cpu(get_version_reply.build_no));
/* we can register the device now, as it is ready */
usb_set_intfdata(interface, dev);
@@ -844,11 +824,9 @@ static int tower_probe(struct usb_interface *interface, const struct usb_device_
USB_MAJOR, dev->minor);
exit:
- kfree(get_version_reply);
return retval;
error:
- kfree(get_version_reply);
tower_delete(dev);
return retval;
}
--
2.28.0
There are a few calls to usb_control_msg() that can be converted to use
usb_control_msg_send() instead, so do that in order to make the error
checking a bit simpler and the code smaller.
Cc: Alan Stern <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/usb/core/hub.c | 128 +++++++++++++++--------------------------
1 file changed, 47 insertions(+), 81 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5b768b80d1ee..b7468517f336 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -410,8 +410,8 @@ static int get_hub_descriptor(struct usb_device *hdev,
*/
static int clear_hub_feature(struct usb_device *hdev, int feature)
{
- return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
- USB_REQ_CLEAR_FEATURE, USB_RT_HUB, feature, 0, NULL, 0, 1000);
+ return usb_control_msg_send(hdev, 0, USB_REQ_CLEAR_FEATURE, USB_RT_HUB,
+ feature, 0, NULL, 0, 1000);
}
/*
@@ -419,9 +419,8 @@ static int clear_hub_feature(struct usb_device *hdev, int feature)
*/
int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
{
- return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
- USB_REQ_CLEAR_FEATURE, USB_RT_PORT, feature, port1,
- NULL, 0, 1000);
+ return usb_control_msg_send(hdev, 0, USB_REQ_CLEAR_FEATURE, USB_RT_PORT,
+ feature, port1, NULL, 0, 1000);
}
/*
@@ -429,9 +428,8 @@ int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
*/
static int set_port_feature(struct usb_device *hdev, int port1, int feature)
{
- return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
- USB_REQ_SET_FEATURE, USB_RT_PORT, feature, port1,
- NULL, 0, 1000);
+ return usb_control_msg_send(hdev, 0, USB_REQ_SET_FEATURE, USB_RT_PORT,
+ feature, port1, NULL, 0, 1000);
}
static char *to_led_name(int selector)
@@ -755,15 +753,14 @@ hub_clear_tt_buffer(struct usb_device *hdev, u16 devinfo, u16 tt)
/* Need to clear both directions for control ep */
if (((devinfo >> 11) & USB_ENDPOINT_XFERTYPE_MASK) ==
USB_ENDPOINT_XFER_CONTROL) {
- int status = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
- HUB_CLEAR_TT_BUFFER, USB_RT_PORT,
- devinfo ^ 0x8000, tt, NULL, 0, 1000);
+ int status = usb_control_msg_send(hdev, 0,
+ HUB_CLEAR_TT_BUFFER, USB_RT_PORT,
+ devinfo ^ 0x8000, tt, NULL, 0, 1000);
if (status)
return status;
}
- return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
- HUB_CLEAR_TT_BUFFER, USB_RT_PORT, devinfo,
- tt, NULL, 0, 1000);
+ return usb_control_msg_send(hdev, 0, HUB_CLEAR_TT_BUFFER, USB_RT_PORT,
+ devinfo, tt, NULL, 0, 1000);
}
/*
@@ -1049,11 +1046,10 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
*/
if (type != HUB_RESUME) {
if (hdev->parent && hub_is_superspeed(hdev)) {
- ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
- HUB_SET_DEPTH, USB_RT_HUB,
- hdev->level - 1, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
- if (ret < 0)
+ ret = usb_control_msg_send(hdev, 0, HUB_SET_DEPTH, USB_RT_HUB,
+ hdev->level - 1, 0, NULL, 0,
+ USB_CTRL_SET_TIMEOUT);
+ if (ret)
dev_err(hub->intfdev,
"set hub depth failed\n");
}
@@ -2329,13 +2325,10 @@ static int usb_enumerate_device_otg(struct usb_device *udev)
/* enable HNP before suspend, it's simpler */
if (port1 == bus->otg_port) {
bus->b_hnp_enable = 1;
- err = usb_control_msg(udev,
- usb_sndctrlpipe(udev, 0),
- USB_REQ_SET_FEATURE, 0,
- USB_DEVICE_B_HNP_ENABLE,
- 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
- if (err < 0) {
+ err = usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, 0,
+ USB_DEVICE_B_HNP_ENABLE, 0,
+ NULL, 0, USB_CTRL_SET_TIMEOUT);
+ if (err) {
/*
* OTG MESSAGE: report errors here,
* customize to match your product.
@@ -2347,13 +2340,10 @@ static int usb_enumerate_device_otg(struct usb_device *udev)
} else if (desc->bLength == sizeof
(struct usb_otg_descriptor)) {
/* Set a_alt_hnp_support for legacy otg device */
- err = usb_control_msg(udev,
- usb_sndctrlpipe(udev, 0),
- USB_REQ_SET_FEATURE, 0,
- USB_DEVICE_A_ALT_HNP_SUPPORT,
- 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
- if (err < 0)
+ err = usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, 0,
+ USB_DEVICE_A_ALT_HNP_SUPPORT, 0,
+ NULL, 0, USB_CTRL_SET_TIMEOUT);
+ if (err)
dev_err(&udev->dev,
"set a_alt_hnp_support failed: %d\n",
err);
@@ -3121,10 +3111,8 @@ int usb_disable_ltm(struct usb_device *udev)
if (!udev->actconfig)
return 0;
- return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
- USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
- USB_DEVICE_LTM_ENABLE, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ return usb_control_msg_send(udev, 0, USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
+ USB_DEVICE_LTM_ENABLE, 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
}
EXPORT_SYMBOL_GPL(usb_disable_ltm);
@@ -3143,10 +3131,8 @@ void usb_enable_ltm(struct usb_device *udev)
if (!udev->actconfig)
return;
- usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
- USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
- USB_DEVICE_LTM_ENABLE, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
+ USB_DEVICE_LTM_ENABLE, 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
}
EXPORT_SYMBOL_GPL(usb_enable_ltm);
@@ -3163,17 +3149,14 @@ EXPORT_SYMBOL_GPL(usb_enable_ltm);
static int usb_enable_remote_wakeup(struct usb_device *udev)
{
if (udev->speed < USB_SPEED_SUPER)
- return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
- USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
- USB_DEVICE_REMOTE_WAKEUP, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ return usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
+ USB_DEVICE_REMOTE_WAKEUP, 0,
+ NULL, 0, USB_CTRL_SET_TIMEOUT);
else
- return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
- USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
- USB_INTRF_FUNC_SUSPEND,
- USB_INTRF_FUNC_SUSPEND_RW |
- USB_INTRF_FUNC_SUSPEND_LP,
- NULL, 0, USB_CTRL_SET_TIMEOUT);
+ return usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
+ USB_INTRF_FUNC_SUSPEND,
+ USB_INTRF_FUNC_SUSPEND_RW | USB_INTRF_FUNC_SUSPEND_LP,
+ NULL, 0, USB_CTRL_SET_TIMEOUT);
}
/*
@@ -3189,15 +3172,11 @@ static int usb_enable_remote_wakeup(struct usb_device *udev)
static int usb_disable_remote_wakeup(struct usb_device *udev)
{
if (udev->speed < USB_SPEED_SUPER)
- return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
- USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
- USB_DEVICE_REMOTE_WAKEUP, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ return usb_control_msg_send(udev, 0, USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
+ USB_DEVICE_REMOTE_WAKEUP, 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
else
- return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
- USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
- USB_INTRF_FUNC_SUSPEND, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ return usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
+ USB_INTRF_FUNC_SUSPEND, 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
}
/* Count of wakeup-enabled devices at or below udev */
@@ -3844,7 +3823,7 @@ static const char * const usb3_lpm_names[] = {
*/
static int usb_req_set_sel(struct usb_device *udev, enum usb3_link_state state)
{
- struct usb_set_sel_req *sel_values;
+ struct usb_set_sel_req sel_values;
unsigned long long u1_sel;
unsigned long long u1_pel;
unsigned long long u2_sel;
@@ -3896,27 +3875,14 @@ static int usb_req_set_sel(struct usb_device *udev, enum usb3_link_state state)
if (u2_pel > USB3_LPM_MAX_U2_SEL_PEL)
u2_pel = USB3_LPM_MAX_U2_SEL_PEL;
- /*
- * usb_enable_lpm() can be called as part of a failed device reset,
- * which may be initiated by an error path of a mass storage driver.
- * Therefore, use GFP_NOIO.
- */
- sel_values = kmalloc(sizeof *(sel_values), GFP_NOIO);
- if (!sel_values)
- return -ENOMEM;
-
- sel_values->u1_sel = u1_sel;
- sel_values->u1_pel = u1_pel;
- sel_values->u2_sel = cpu_to_le16(u2_sel);
- sel_values->u2_pel = cpu_to_le16(u2_pel);
+ sel_values.u1_sel = u1_sel;
+ sel_values.u1_pel = u1_pel;
+ sel_values.u2_sel = cpu_to_le16(u2_sel);
+ sel_values.u2_pel = cpu_to_le16(u2_pel);
- ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
- USB_REQ_SET_SEL,
- USB_RECIP_DEVICE,
- 0, 0,
- sel_values, sizeof *(sel_values),
- USB_CTRL_SET_TIMEOUT);
- kfree(sel_values);
+ ret = usb_control_msg_send(udev, 0, USB_REQ_SET_SEL, USB_RECIP_DEVICE,
+ 0, 0, &sel_values, sizeof(sel_values),
+ USB_CTRL_SET_TIMEOUT);
return ret;
}
@@ -4056,7 +4022,7 @@ static void usb_enable_link_state(struct usb_hcd *hcd, struct usb_device *udev,
* associated with the link state we're about to enable.
*/
ret = usb_req_set_sel(udev, state);
- if (ret < 0) {
+ if (ret) {
dev_warn(&udev->dev, "Set SEL for device-initiated %s failed.\n",
usb3_lpm_names[state]);
return;
--
2.28.0
The usb_control_msg_send() call can return an error if a "short" write
happens, so move the driver over to using that call instead.
Cc: Jaroslav Kysela <[email protected]>
Cc: Takashi Iwai <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
sound/usb/hiface/pcm.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c
index a148caa5f48e..f9c924e3964e 100644
--- a/sound/usb/hiface/pcm.c
+++ b/sound/usb/hiface/pcm.c
@@ -156,16 +156,14 @@ static int hiface_pcm_set_rate(struct pcm_runtime *rt, unsigned int rate)
* This control message doesn't have any ack from the
* other side
*/
- ret = usb_control_msg(device, usb_sndctrlpipe(device, 0),
- HIFACE_SET_RATE_REQUEST,
- USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
- rate_value, 0, NULL, 0, 100);
- if (ret < 0) {
+ ret = usb_control_msg_send(device, 0,
+ HIFACE_SET_RATE_REQUEST,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+ rate_value, 0, NULL, 0, 100);
+ if (ret)
dev_err(&device->dev, "Error setting samplerate %d.\n", rate);
- return ret;
- }
- return 0;
+ return ret;
}
static struct pcm_substream *hiface_pcm_get_substream(struct snd_pcm_substream
--
2.28.0
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
sound/usb/hiface/pcm.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c
index a148caa5f48e..feab3f4834c2 100644
--- a/sound/usb/hiface/pcm.c
+++ b/sound/usb/hiface/pcm.c
@@ -156,14 +156,12 @@ static int hiface_pcm_set_rate(struct pcm_runtime *rt, unsigned int rate)
* This control message doesn't have any ack from the
* other side
*/
- ret = usb_control_msg(device, usb_sndctrlpipe(device, 0),
- HIFACE_SET_RATE_REQUEST,
- USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
- rate_value, 0, NULL, 0, 100);
- if (ret < 0) {
+ ret = usb_control_msg_send(device, 0,
+ HIFACE_SET_RATE_REQUEST,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+ rate_value, 0, NULL, 0, 100);
+ if (ret)
dev_err(&device->dev, "Error setting samplerate %d.\n", rate);
- return ret;
- }
return 0;
}
--
2.28.0
The usb_control_msg_send() and usb_control_msg_recv() calls can return
an error if a "short" write/read happens, and they can handle data off
of the stack, so move the driver over to using those calls instead,
saving some logic when dynamically allocating memory.
Cc: Jaroslav Kysela <[email protected]>
Cc: Takashi Iwai <[email protected]>
Cc: Vasily Khoruzhick <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
sound/usb/line6/driver.c | 69 +++++++++++++++-----------------------
sound/usb/line6/podhd.c | 17 ++++------
sound/usb/line6/toneport.c | 8 ++---
3 files changed, 37 insertions(+), 57 deletions(-)
diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index 60674ce4879b..601292c51491 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -337,23 +337,18 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
{
struct usb_device *usbdev = line6->usbdev;
int ret;
- unsigned char *len;
+ u8 len;
unsigned count;
if (address > 0xffff || datalen > 0xff)
return -EINVAL;
- len = kmalloc(1, GFP_KERNEL);
- if (!len)
- return -ENOMEM;
-
/* query the serial number: */
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
- (datalen << 8) | 0x21, address,
- NULL, 0, LINE6_TIMEOUT * HZ);
-
- if (ret < 0) {
+ ret = usb_control_msg_send(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ (datalen << 8) | 0x21, address, NULL, 0,
+ LINE6_TIMEOUT * HZ);
+ if (ret) {
dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
goto exit;
}
@@ -362,45 +357,41 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
mdelay(LINE6_READ_WRITE_STATUS_DELAY);
- ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE |
- USB_DIR_IN,
- 0x0012, 0x0000, len, 1,
- LINE6_TIMEOUT * HZ);
- if (ret < 0) {
+ ret = usb_control_msg_recv(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+ 0x0012, 0x0000, &len, 1,
+ LINE6_TIMEOUT * HZ);
+ if (ret) {
dev_err(line6->ifcdev,
"receive length failed (error %d)\n", ret);
goto exit;
}
- if (*len != 0xff)
+ if (len != 0xff)
break;
}
ret = -EIO;
- if (*len == 0xff) {
+ if (len == 0xff) {
dev_err(line6->ifcdev, "read failed after %d retries\n",
count);
goto exit;
- } else if (*len != datalen) {
+ } else if (len != datalen) {
/* should be equal or something went wrong */
dev_err(line6->ifcdev,
"length mismatch (expected %d, got %d)\n",
- (int)datalen, (int)*len);
+ (int)datalen, len);
goto exit;
}
/* receive the result: */
- ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
- 0x0013, 0x0000, data, datalen,
- LINE6_TIMEOUT * HZ);
-
- if (ret < 0)
+ ret = usb_control_msg_recv(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+ 0x0013, 0x0000, data, datalen, LINE6_TIMEOUT * HZ);
+ if (ret)
dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
exit:
- kfree(len);
return ret;
}
EXPORT_SYMBOL_GPL(line6_read_data);
@@ -423,12 +414,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
if (!status)
return -ENOMEM;
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
- 0x0022, address, data, datalen,
- LINE6_TIMEOUT * HZ);
-
- if (ret < 0) {
+ ret = usb_control_msg_send(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ 0x0022, address, data, datalen, LINE6_TIMEOUT * HZ);
+ if (ret) {
dev_err(line6->ifcdev,
"write request failed (error %d)\n", ret);
goto exit;
@@ -437,14 +426,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
mdelay(LINE6_READ_WRITE_STATUS_DELAY);
- ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
- 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE |
- USB_DIR_IN,
- 0x0012, 0x0000,
- status, 1, LINE6_TIMEOUT * HZ);
-
- if (ret < 0) {
+ ret = usb_control_msg_recv(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+ 0x0012, 0x0000, status, 1, LINE6_TIMEOUT * HZ);
+ if (ret) {
dev_err(line6->ifcdev,
"receiving status failed (error %d)\n", ret);
goto exit;
diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c
index eef45f7fef0d..a1261f55d62b 100644
--- a/sound/usb/line6/podhd.c
+++ b/sound/usb/line6/podhd.c
@@ -183,29 +183,25 @@ static const struct attribute_group podhd_dev_attr_group = {
static int podhd_dev_start(struct usb_line6_podhd *pod)
{
int ret;
- u8 *init_bytes;
+ u8 init_bytes[8];
int i;
struct usb_device *usbdev = pod->line6.usbdev;
- init_bytes = kmalloc(8, GFP_KERNEL);
- if (!init_bytes)
- return -ENOMEM;
-
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+ ret = usb_control_msg_send(usbdev, 0,
0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
0x11, 0,
NULL, 0, LINE6_TIMEOUT * HZ);
- if (ret < 0) {
+ if (ret) {
dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
goto exit;
}
/* NOTE: looks like some kind of ping message */
- ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
+ ret = usb_control_msg_recv(usbdev, 0, 0x67,
USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
0x11, 0x0,
init_bytes, 3, LINE6_TIMEOUT * HZ);
- if (ret < 0) {
+ if (ret) {
dev_err(pod->line6.ifcdev,
"receive length failed (error %d)\n", ret);
goto exit;
@@ -220,13 +216,12 @@ static int podhd_dev_start(struct usb_line6_podhd *pod)
goto exit;
}
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+ ret = usb_control_msg_send(usbdev, 0,
USB_REQ_SET_FEATURE,
USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
1, 0,
NULL, 0, LINE6_TIMEOUT * HZ);
exit:
- kfree(init_bytes);
return ret;
}
diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c
index 94dd5e7ab2e6..a9b56085b76a 100644
--- a/sound/usb/line6/toneport.c
+++ b/sound/usb/line6/toneport.c
@@ -126,11 +126,11 @@ static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2)
{
int ret;
- ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
- cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
+ ret = usb_control_msg_send(usbdev, 0, 0x67,
+ USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
- if (ret < 0) {
+ if (ret) {
dev_err(&usbdev->dev, "send failed (error %d)\n", ret);
return ret;
}
--
2.28.0
On Wed, 02 Sep 2020 13:01:03 +0200,
Greg Kroah-Hartman wrote:
>
> snd_usb_pipe_sanity_check() is a great function, so let's move it into
> the USB core so that other parts of the kernel, including the USB core,
> can call it.
>
> Name it usb_pipe_type_check() to match the existing
> usb_urb_ep_type_check() call, which now uses this function.
>
> Cc: Jaroslav Kysela <[email protected]>
> Cc: Takashi Iwai <[email protected]>
> Cc: "Gustavo A. R. Silva" <[email protected]>
> Cc: Eli Billauer <[email protected]>
> Cc: Emiliano Ingrassia <[email protected]>
> Cc: Alan Stern <[email protected]>
> Cc: Alexander Tsoy <[email protected]>
> Cc: "Geoffrey D. Bennett" <[email protected]>
> Cc: Jussi Laako <[email protected]>
> Cc: Nick Kossifidis <[email protected]>
> Cc: Dmitry Panchenko <[email protected]>
> Cc: Chris Wulff <[email protected]>
> Cc: Jesus Ramos <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Takashi Iwai <[email protected]>
thanks,
Takashi
> ---
> drivers/usb/core/urb.c | 29 ++++++++++++++++++++++-------
> include/linux/usb.h | 1 +
> sound/usb/helper.c | 16 +---------------
> sound/usb/helper.h | 1 -
> sound/usb/mixer_scarlett_gen2.c | 2 +-
> sound/usb/quirks.c | 12 ++++++------
> 6 files changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
> index 27e83e55a590..45bc2914c1ba 100644
> --- a/drivers/usb/core/urb.c
> +++ b/drivers/usb/core/urb.c
> @@ -192,24 +192,39 @@ static const int pipetypes[4] = {
> };
>
> /**
> - * usb_urb_ep_type_check - sanity check of endpoint in the given urb
> - * @urb: urb to be checked
> + * usb_pipe_type_check - sanity check of a specific pipe for a usb device
> + * @dev: struct usb_device to be checked
> + * @pipe: pipe to check
> *
> * This performs a light-weight sanity check for the endpoint in the
> - * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
> - * a negative error code.
> + * given usb device. It returns 0 if the pipe is a valid for the specific usb
> + * device, otherwise a negative error code.
> */
> -int usb_urb_ep_type_check(const struct urb *urb)
> +int usb_pipe_type_check(struct usb_device *dev, unsigned int pipe)
> {
> const struct usb_host_endpoint *ep;
>
> - ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> + ep = usb_pipe_endpoint(dev, pipe);
> if (!ep)
> return -EINVAL;
> - if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
> + if (usb_pipetype(pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
> return -EINVAL;
> return 0;
> }
> +EXPORT_SYMBOL_GPL(usb_pipe_type_check);
> +
> +/**
> + * usb_urb_ep_type_check - sanity check of endpoint in the given urb
> + * @urb: urb to be checked
> + *
> + * This performs a light-weight sanity check for the endpoint in the
> + * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
> + * a negative error code.
> + */
> +int usb_urb_ep_type_check(const struct urb *urb)
> +{
> + return usb_pipe_type_check(urb->dev, urb->pipe);
> +}
> EXPORT_SYMBOL_GPL(usb_urb_ep_type_check);
>
> /**
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 20c555db4621..0b3963d7ec38 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -1764,6 +1764,7 @@ static inline int usb_urb_dir_out(struct urb *urb)
> return (urb->transfer_flags & URB_DIR_MASK) == URB_DIR_OUT;
> }
>
> +int usb_pipe_type_check(struct usb_device *dev, unsigned int pipe);
> int usb_urb_ep_type_check(const struct urb *urb);
>
> void *usb_alloc_coherent(struct usb_device *dev, size_t size,
> diff --git a/sound/usb/helper.c b/sound/usb/helper.c
> index 4c12cc5b53fd..cf92d7110773 100644
> --- a/sound/usb/helper.c
> +++ b/sound/usb/helper.c
> @@ -63,20 +63,6 @@ void *snd_usb_find_csint_desc(void *buffer, int buflen, void *after, u8 dsubtype
> return NULL;
> }
>
> -/* check the validity of pipe and EP types */
> -int snd_usb_pipe_sanity_check(struct usb_device *dev, unsigned int pipe)
> -{
> - static const int pipetypes[4] = {
> - PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
> - };
> - struct usb_host_endpoint *ep;
> -
> - ep = usb_pipe_endpoint(dev, pipe);
> - if (!ep || usb_pipetype(pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
> - return -EINVAL;
> - return 0;
> -}
> -
> /*
> * Wrapper for usb_control_msg().
> * Allocates a temp buffer to prevent dmaing from/to the stack.
> @@ -89,7 +75,7 @@ int snd_usb_ctl_msg(struct usb_device *dev, unsigned int pipe, __u8 request,
> void *buf = NULL;
> int timeout;
>
> - if (snd_usb_pipe_sanity_check(dev, pipe))
> + if (usb_pipe_type_check(dev, pipe))
> return -EINVAL;
>
> if (size > 0) {
> diff --git a/sound/usb/helper.h b/sound/usb/helper.h
> index 5e8a18b4e7b9..f5b4c6647e4d 100644
> --- a/sound/usb/helper.h
> +++ b/sound/usb/helper.h
> @@ -7,7 +7,6 @@ unsigned int snd_usb_combine_bytes(unsigned char *bytes, int size);
> void *snd_usb_find_desc(void *descstart, int desclen, void *after, u8 dtype);
> void *snd_usb_find_csint_desc(void *descstart, int desclen, void *after, u8 dsubtype);
>
> -int snd_usb_pipe_sanity_check(struct usb_device *dev, unsigned int pipe);
> int snd_usb_ctl_msg(struct usb_device *dev, unsigned int pipe,
> __u8 request, __u8 requesttype, __u16 value, __u16 index,
> void *data, __u16 size);
> diff --git a/sound/usb/mixer_scarlett_gen2.c b/sound/usb/mixer_scarlett_gen2.c
> index 0ffff7640892..9609c6d9655c 100644
> --- a/sound/usb/mixer_scarlett_gen2.c
> +++ b/sound/usb/mixer_scarlett_gen2.c
> @@ -1978,7 +1978,7 @@ static int scarlett2_mixer_status_create(struct usb_mixer_interface *mixer)
> return 0;
> }
>
> - if (snd_usb_pipe_sanity_check(dev, pipe))
> + if (usb_pipe_type_check(dev, pipe))
> return -EINVAL;
>
> mixer->urb = usb_alloc_urb(0, GFP_KERNEL);
> diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
> index abf99b814a0f..fc3aab04a0bc 100644
> --- a/sound/usb/quirks.c
> +++ b/sound/usb/quirks.c
> @@ -846,7 +846,7 @@ static int snd_usb_accessmusic_boot_quirk(struct usb_device *dev)
> static const u8 seq[] = { 0x4e, 0x73, 0x52, 0x01 };
> void *buf;
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndintpipe(dev, 0x05)))
> + if (usb_pipe_type_check(dev, usb_sndintpipe(dev, 0x05)))
> return -EINVAL;
> buf = kmemdup(seq, ARRAY_SIZE(seq), GFP_KERNEL);
> if (!buf)
> @@ -875,7 +875,7 @@ static int snd_usb_nativeinstruments_boot_quirk(struct usb_device *dev)
> {
> int ret;
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
> + if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
> return -EINVAL;
> ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> 0xaf, USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> @@ -984,7 +984,7 @@ static int snd_usb_axefx3_boot_quirk(struct usb_device *dev)
>
> dev_dbg(&dev->dev, "Waiting for Axe-Fx III to boot up...\n");
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
> + if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
> return -EINVAL;
> /* If the Axe-Fx III has not fully booted, it will timeout when trying
> * to enable the audio streaming interface. A more generous timeout is
> @@ -1018,7 +1018,7 @@ static int snd_usb_motu_microbookii_communicate(struct usb_device *dev, u8 *buf,
> {
> int err, actual_length;
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndintpipe(dev, 0x01)))
> + if (usb_pipe_type_check(dev, usb_sndintpipe(dev, 0x01)))
> return -EINVAL;
> err = usb_interrupt_msg(dev, usb_sndintpipe(dev, 0x01), buf, *length,
> &actual_length, 1000);
> @@ -1030,7 +1030,7 @@ static int snd_usb_motu_microbookii_communicate(struct usb_device *dev, u8 *buf,
>
> memset(buf, 0, buf_size);
>
> - if (snd_usb_pipe_sanity_check(dev, usb_rcvintpipe(dev, 0x82)))
> + if (usb_pipe_type_check(dev, usb_rcvintpipe(dev, 0x82)))
> return -EINVAL;
> err = usb_interrupt_msg(dev, usb_rcvintpipe(dev, 0x82), buf, buf_size,
> &actual_length, 1000);
> @@ -1117,7 +1117,7 @@ static int snd_usb_motu_m_series_boot_quirk(struct usb_device *dev)
> {
> int ret;
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
> + if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
> return -EINVAL;
> ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> 1, USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> --
> 2.28.0
>
On Wed, 02 Sep 2020 13:01:12 +0200,
Greg Kroah-Hartman wrote:
>
> The usb_control_msg_send() and usb_control_msg_recv() calls can return
> an error if a "short" write/read happens, and they can handle data off
> of the stack, so move the driver over to using those calls instead,
> saving some logic when dynamically allocating memory.
>
> Cc: Jaroslav Kysela <[email protected]>
> Cc: Takashi Iwai <[email protected]>
> Cc: Vasily Khoruzhick <[email protected]>
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Takashi Iwai <[email protected]>
thanks,
Takashi
> ---
> sound/usb/line6/driver.c | 69 +++++++++++++++-----------------------
> sound/usb/line6/podhd.c | 17 ++++------
> sound/usb/line6/toneport.c | 8 ++---
> 3 files changed, 37 insertions(+), 57 deletions(-)
>
> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
> index 60674ce4879b..601292c51491 100644
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -337,23 +337,18 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
> {
> struct usb_device *usbdev = line6->usbdev;
> int ret;
> - unsigned char *len;
> + u8 len;
> unsigned count;
>
> if (address > 0xffff || datalen > 0xff)
> return -EINVAL;
>
> - len = kmalloc(1, GFP_KERNEL);
> - if (!len)
> - return -ENOMEM;
> -
> /* query the serial number: */
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> - (datalen << 8) | 0x21, address,
> - NULL, 0, LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0) {
> + ret = usb_control_msg_send(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + (datalen << 8) | 0x21, address, NULL, 0,
> + LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
> goto exit;
> }
> @@ -362,45 +357,41 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
> for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
> mdelay(LINE6_READ_WRITE_STATUS_DELAY);
>
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE |
> - USB_DIR_IN,
> - 0x0012, 0x0000, len, 1,
> - LINE6_TIMEOUT * HZ);
> - if (ret < 0) {
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> + 0x0012, 0x0000, &len, 1,
> + LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev,
> "receive length failed (error %d)\n", ret);
> goto exit;
> }
>
> - if (*len != 0xff)
> + if (len != 0xff)
> break;
> }
>
> ret = -EIO;
> - if (*len == 0xff) {
> + if (len == 0xff) {
> dev_err(line6->ifcdev, "read failed after %d retries\n",
> count);
> goto exit;
> - } else if (*len != datalen) {
> + } else if (len != datalen) {
> /* should be equal or something went wrong */
> dev_err(line6->ifcdev,
> "length mismatch (expected %d, got %d)\n",
> - (int)datalen, (int)*len);
> + (int)datalen, len);
> goto exit;
> }
>
> /* receive the result: */
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> - 0x0013, 0x0000, data, datalen,
> - LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0)
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> + 0x0013, 0x0000, data, datalen, LINE6_TIMEOUT * HZ);
> + if (ret)
> dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
>
> exit:
> - kfree(len);
> return ret;
> }
> EXPORT_SYMBOL_GPL(line6_read_data);
> @@ -423,12 +414,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
> if (!status)
> return -ENOMEM;
>
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> - 0x0022, address, data, datalen,
> - LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0) {
> + ret = usb_control_msg_send(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + 0x0022, address, data, datalen, LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev,
> "write request failed (error %d)\n", ret);
> goto exit;
> @@ -437,14 +426,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
> for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
> mdelay(LINE6_READ_WRITE_STATUS_DELAY);
>
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
> - 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE |
> - USB_DIR_IN,
> - 0x0012, 0x0000,
> - status, 1, LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0) {
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> + 0x0012, 0x0000, status, 1, LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev,
> "receiving status failed (error %d)\n", ret);
> goto exit;
> diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c
> index eef45f7fef0d..a1261f55d62b 100644
> --- a/sound/usb/line6/podhd.c
> +++ b/sound/usb/line6/podhd.c
> @@ -183,29 +183,25 @@ static const struct attribute_group podhd_dev_attr_group = {
> static int podhd_dev_start(struct usb_line6_podhd *pod)
> {
> int ret;
> - u8 *init_bytes;
> + u8 init_bytes[8];
> int i;
> struct usb_device *usbdev = pod->line6.usbdev;
>
> - init_bytes = kmalloc(8, GFP_KERNEL);
> - if (!init_bytes)
> - return -ENOMEM;
> -
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> + ret = usb_control_msg_send(usbdev, 0,
> 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> 0x11, 0,
> NULL, 0, LINE6_TIMEOUT * HZ);
> - if (ret < 0) {
> + if (ret) {
> dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
> goto exit;
> }
>
> /* NOTE: looks like some kind of ping message */
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> 0x11, 0x0,
> init_bytes, 3, LINE6_TIMEOUT * HZ);
> - if (ret < 0) {
> + if (ret) {
> dev_err(pod->line6.ifcdev,
> "receive length failed (error %d)\n", ret);
> goto exit;
> @@ -220,13 +216,12 @@ static int podhd_dev_start(struct usb_line6_podhd *pod)
> goto exit;
> }
>
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> + ret = usb_control_msg_send(usbdev, 0,
> USB_REQ_SET_FEATURE,
> USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
> 1, 0,
> NULL, 0, LINE6_TIMEOUT * HZ);
> exit:
> - kfree(init_bytes);
> return ret;
> }
>
> diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c
> index 94dd5e7ab2e6..a9b56085b76a 100644
> --- a/sound/usb/line6/toneport.c
> +++ b/sound/usb/line6/toneport.c
> @@ -126,11 +126,11 @@ static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2)
> {
> int ret;
>
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> - cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
> + ret = usb_control_msg_send(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
>
> - if (ret < 0) {
> + if (ret) {
> dev_err(&usbdev->dev, "send failed (error %d)\n", ret);
> return ret;
> }
> --
> 2.28.0
>
On Wed, 02 Sep 2020 13:01:14 +0200,
Greg Kroah-Hartman wrote:
>
> The usb_control_msg_send() call can return an error if a "short" write
> happens, so move the driver over to using that call instead.
>
> Cc: Jaroslav Kysela <[email protected]>
> Cc: Takashi Iwai <[email protected]>
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Takashi Iwai <[email protected]>
thanks,
Takashi
> ---
> sound/usb/hiface/pcm.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c
> index a148caa5f48e..f9c924e3964e 100644
> --- a/sound/usb/hiface/pcm.c
> +++ b/sound/usb/hiface/pcm.c
> @@ -156,16 +156,14 @@ static int hiface_pcm_set_rate(struct pcm_runtime *rt, unsigned int rate)
> * This control message doesn't have any ack from the
> * other side
> */
> - ret = usb_control_msg(device, usb_sndctrlpipe(device, 0),
> - HIFACE_SET_RATE_REQUEST,
> - USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> - rate_value, 0, NULL, 0, 100);
> - if (ret < 0) {
> + ret = usb_control_msg_send(device, 0,
> + HIFACE_SET_RATE_REQUEST,
> + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> + rate_value, 0, NULL, 0, 100);
> + if (ret)
> dev_err(&device->dev, "Error setting samplerate %d.\n", rate);
> - return ret;
> - }
>
> - return 0;
> + return ret;
> }
>
> static struct pcm_substream *hiface_pcm_get_substream(struct snd_pcm_substream
> --
> 2.28.0
>
On Wed, 02 Sep 2020 13:01:10 +0200,
Greg Kroah-Hartman wrote:
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
I guess this and a few others with (x/9) are stale patches, right?
thanks,
Takashi
> ---
> sound/usb/line6/driver.c | 69 +++++++++++++++-----------------------
> sound/usb/line6/podhd.c | 17 ++++------
> sound/usb/line6/toneport.c | 8 ++---
> 3 files changed, 37 insertions(+), 57 deletions(-)
>
> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
> index 60674ce4879b..601292c51491 100644
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -337,23 +337,18 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
> {
> struct usb_device *usbdev = line6->usbdev;
> int ret;
> - unsigned char *len;
> + u8 len;
> unsigned count;
>
> if (address > 0xffff || datalen > 0xff)
> return -EINVAL;
>
> - len = kmalloc(1, GFP_KERNEL);
> - if (!len)
> - return -ENOMEM;
> -
> /* query the serial number: */
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> - (datalen << 8) | 0x21, address,
> - NULL, 0, LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0) {
> + ret = usb_control_msg_send(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + (datalen << 8) | 0x21, address, NULL, 0,
> + LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
> goto exit;
> }
> @@ -362,45 +357,41 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
> for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
> mdelay(LINE6_READ_WRITE_STATUS_DELAY);
>
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE |
> - USB_DIR_IN,
> - 0x0012, 0x0000, len, 1,
> - LINE6_TIMEOUT * HZ);
> - if (ret < 0) {
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> + 0x0012, 0x0000, &len, 1,
> + LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev,
> "receive length failed (error %d)\n", ret);
> goto exit;
> }
>
> - if (*len != 0xff)
> + if (len != 0xff)
> break;
> }
>
> ret = -EIO;
> - if (*len == 0xff) {
> + if (len == 0xff) {
> dev_err(line6->ifcdev, "read failed after %d retries\n",
> count);
> goto exit;
> - } else if (*len != datalen) {
> + } else if (len != datalen) {
> /* should be equal or something went wrong */
> dev_err(line6->ifcdev,
> "length mismatch (expected %d, got %d)\n",
> - (int)datalen, (int)*len);
> + (int)datalen, len);
> goto exit;
> }
>
> /* receive the result: */
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> - 0x0013, 0x0000, data, datalen,
> - LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0)
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> + 0x0013, 0x0000, data, datalen, LINE6_TIMEOUT * HZ);
> + if (ret)
> dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
>
> exit:
> - kfree(len);
> return ret;
> }
> EXPORT_SYMBOL_GPL(line6_read_data);
> @@ -423,12 +414,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
> if (!status)
> return -ENOMEM;
>
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> - 0x0022, address, data, datalen,
> - LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0) {
> + ret = usb_control_msg_send(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + 0x0022, address, data, datalen, LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev,
> "write request failed (error %d)\n", ret);
> goto exit;
> @@ -437,14 +426,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
> for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
> mdelay(LINE6_READ_WRITE_STATUS_DELAY);
>
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
> - 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE |
> - USB_DIR_IN,
> - 0x0012, 0x0000,
> - status, 1, LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0) {
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> + 0x0012, 0x0000, status, 1, LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev,
> "receiving status failed (error %d)\n", ret);
> goto exit;
> diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c
> index eef45f7fef0d..a1261f55d62b 100644
> --- a/sound/usb/line6/podhd.c
> +++ b/sound/usb/line6/podhd.c
> @@ -183,29 +183,25 @@ static const struct attribute_group podhd_dev_attr_group = {
> static int podhd_dev_start(struct usb_line6_podhd *pod)
> {
> int ret;
> - u8 *init_bytes;
> + u8 init_bytes[8];
> int i;
> struct usb_device *usbdev = pod->line6.usbdev;
>
> - init_bytes = kmalloc(8, GFP_KERNEL);
> - if (!init_bytes)
> - return -ENOMEM;
> -
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> + ret = usb_control_msg_send(usbdev, 0,
> 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> 0x11, 0,
> NULL, 0, LINE6_TIMEOUT * HZ);
> - if (ret < 0) {
> + if (ret) {
> dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
> goto exit;
> }
>
> /* NOTE: looks like some kind of ping message */
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> 0x11, 0x0,
> init_bytes, 3, LINE6_TIMEOUT * HZ);
> - if (ret < 0) {
> + if (ret) {
> dev_err(pod->line6.ifcdev,
> "receive length failed (error %d)\n", ret);
> goto exit;
> @@ -220,13 +216,12 @@ static int podhd_dev_start(struct usb_line6_podhd *pod)
> goto exit;
> }
>
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> + ret = usb_control_msg_send(usbdev, 0,
> USB_REQ_SET_FEATURE,
> USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
> 1, 0,
> NULL, 0, LINE6_TIMEOUT * HZ);
> exit:
> - kfree(init_bytes);
> return ret;
> }
>
> diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c
> index 94dd5e7ab2e6..a9b56085b76a 100644
> --- a/sound/usb/line6/toneport.c
> +++ b/sound/usb/line6/toneport.c
> @@ -126,11 +126,11 @@ static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2)
> {
> int ret;
>
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> - cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
> + ret = usb_control_msg_send(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
>
> - if (ret < 0) {
> + if (ret) {
> dev_err(&usbdev->dev, "send failed (error %d)\n", ret);
> return ret;
> }
> --
> 2.28.0
>
On Wed, Sep 02, 2020 at 01:01:03PM +0200, Greg Kroah-Hartman wrote:
> snd_usb_pipe_sanity_check() is a great function, so let's move it into
> the USB core so that other parts of the kernel, including the USB core,
> can call it.
>
> Name it usb_pipe_type_check() to match the existing
> usb_urb_ep_type_check() call, which now uses this function.
>
> Cc: Jaroslav Kysela <[email protected]>
> Cc: Takashi Iwai <[email protected]>
> Cc: "Gustavo A. R. Silva" <[email protected]>
> Cc: Eli Billauer <[email protected]>
> Cc: Emiliano Ingrassia <[email protected]>
> Cc: Alan Stern <[email protected]>
> Cc: Alexander Tsoy <[email protected]>
> Cc: "Geoffrey D. Bennett" <[email protected]>
> Cc: Jussi Laako <[email protected]>
> Cc: Nick Kossifidis <[email protected]>
> Cc: Dmitry Panchenko <[email protected]>
> Cc: Chris Wulff <[email protected]>
> Cc: Jesus Ramos <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
> index 27e83e55a590..45bc2914c1ba 100644
> --- a/drivers/usb/core/urb.c
> +++ b/drivers/usb/core/urb.c
> @@ -192,24 +192,39 @@ static const int pipetypes[4] = {
> };
>
> /**
> - * usb_urb_ep_type_check - sanity check of endpoint in the given urb
> - * @urb: urb to be checked
> + * usb_pipe_type_check - sanity check of a specific pipe for a usb device
> + * @dev: struct usb_device to be checked
> + * @pipe: pipe to check
> *
> * This performs a light-weight sanity check for the endpoint in the
> - * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
> - * a negative error code.
> + * given usb device. It returns 0 if the pipe is a valid for the specific usb
-----------------------------------------------------^
Typo.
> + * device, otherwise a negative error code.
> */
> -int usb_urb_ep_type_check(const struct urb *urb)
> +int usb_pipe_type_check(struct usb_device *dev, unsigned int pipe)
> {
> const struct usb_host_endpoint *ep;
>
> - ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> + ep = usb_pipe_endpoint(dev, pipe);
> if (!ep)
> return -EINVAL;
> - if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
> + if (usb_pipetype(pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
> return -EINVAL;
> return 0;
> }
> +EXPORT_SYMBOL_GPL(usb_pipe_type_check);
> +
> +/**
> + * usb_urb_ep_type_check - sanity check of endpoint in the given urb
> + * @urb: urb to be checked
> + *
> + * This performs a light-weight sanity check for the endpoint in the
> + * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
> + * a negative error code.
> + */
> +int usb_urb_ep_type_check(const struct urb *urb)
> +{
> + return usb_pipe_type_check(urb->dev, urb->pipe);
> +}
> EXPORT_SYMBOL_GPL(usb_urb_ep_type_check);
Since this routine is used in only one place in the entire kernel, you
might as well inline the code there and get rid of the function
entirely.
> diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
> index abf99b814a0f..fc3aab04a0bc 100644
> --- a/sound/usb/quirks.c
> +++ b/sound/usb/quirks.c
> @@ -846,7 +846,7 @@ static int snd_usb_accessmusic_boot_quirk(struct usb_device *dev)
> static const u8 seq[] = { 0x4e, 0x73, 0x52, 0x01 };
> void *buf;
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndintpipe(dev, 0x05)))
> + if (usb_pipe_type_check(dev, usb_sndintpipe(dev, 0x05)))
> return -EINVAL;
> buf = kmemdup(seq, ARRAY_SIZE(seq), GFP_KERNEL);
> if (!buf)
> @@ -875,7 +875,7 @@ static int snd_usb_nativeinstruments_boot_quirk(struct usb_device *dev)
> {
> int ret;
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
> + if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
> return -EINVAL;
In a few places here this check is completely unnecessary. All it does
is verify that the device does have an endpoint 0 and the the type of
the endpoint matches the type of the pipe. Well, every USB device
always has an endpoint 0, and it is always a bidirectional control
endpoint. Therefore a simple static check is all you need: There's no
point calling usb_pipe_type_check() when the pipe is of the form
usb_{snd|rcv}ctrlpipe(dev, 0).
In short, this check should be removed completely; it does nothing.
> ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> 0xaf, USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> @@ -984,7 +984,7 @@ static int snd_usb_axefx3_boot_quirk(struct usb_device *dev)
>
> dev_dbg(&dev->dev, "Waiting for Axe-Fx III to boot up...\n");
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
> + if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
Same for this check.
> return -EINVAL;
> /* If the Axe-Fx III has not fully booted, it will timeout when trying
> * to enable the audio streaming interface. A more generous timeout is
> @@ -1018,7 +1018,7 @@ static int snd_usb_motu_microbookii_communicate(struct usb_device *dev, u8 *buf,
> {
> int err, actual_length;
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndintpipe(dev, 0x01)))
> + if (usb_pipe_type_check(dev, usb_sndintpipe(dev, 0x01)))
> return -EINVAL;
> err = usb_interrupt_msg(dev, usb_sndintpipe(dev, 0x01), buf, *length,
> &actual_length, 1000);
> @@ -1030,7 +1030,7 @@ static int snd_usb_motu_microbookii_communicate(struct usb_device *dev, u8 *buf,
>
> memset(buf, 0, buf_size);
>
> - if (snd_usb_pipe_sanity_check(dev, usb_rcvintpipe(dev, 0x82)))
> + if (usb_pipe_type_check(dev, usb_rcvintpipe(dev, 0x82)))
> return -EINVAL;
> err = usb_interrupt_msg(dev, usb_rcvintpipe(dev, 0x82), buf, buf_size,
> &actual_length, 1000);
> @@ -1117,7 +1117,7 @@ static int snd_usb_motu_m_series_boot_quirk(struct usb_device *dev)
> {
> int ret;
>
> - if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
> + if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
And this one.
> return -EINVAL;
> ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> 1, USB_TYPE_VENDOR | USB_RECIP_DEVICE,
Alan Stern
On Wed, Sep 02, 2020 at 08:45:53PM -0400, Alan Stern wrote:
> On Wed, Sep 02, 2020 at 01:01:03PM +0200, Greg Kroah-Hartman wrote:
> > snd_usb_pipe_sanity_check() is a great function, so let's move it into
> > the USB core so that other parts of the kernel, including the USB core,
> > can call it.
> >
> > Name it usb_pipe_type_check() to match the existing
> > usb_urb_ep_type_check() call, which now uses this function.
> >
> > Cc: Jaroslav Kysela <[email protected]>
> > Cc: Takashi Iwai <[email protected]>
> > Cc: "Gustavo A. R. Silva" <[email protected]>
> > Cc: Eli Billauer <[email protected]>
> > Cc: Emiliano Ingrassia <[email protected]>
> > Cc: Alan Stern <[email protected]>
> > Cc: Alexander Tsoy <[email protected]>
> > Cc: "Geoffrey D. Bennett" <[email protected]>
> > Cc: Jussi Laako <[email protected]>
> > Cc: Nick Kossifidis <[email protected]>
> > Cc: Dmitry Panchenko <[email protected]>
> > Cc: Chris Wulff <[email protected]>
> > Cc: Jesus Ramos <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > ---
>
> > diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
> > index 27e83e55a590..45bc2914c1ba 100644
> > --- a/drivers/usb/core/urb.c
> > +++ b/drivers/usb/core/urb.c
> > @@ -192,24 +192,39 @@ static const int pipetypes[4] = {
> > };
> >
> > /**
> > - * usb_urb_ep_type_check - sanity check of endpoint in the given urb
> > - * @urb: urb to be checked
> > + * usb_pipe_type_check - sanity check of a specific pipe for a usb device
> > + * @dev: struct usb_device to be checked
> > + * @pipe: pipe to check
> > *
> > * This performs a light-weight sanity check for the endpoint in the
> > - * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
> > - * a negative error code.
> > + * given usb device. It returns 0 if the pipe is a valid for the specific usb
> -----------------------------------------------------^
> Typo.
Oops, will fix, thanks.
>
> > + * device, otherwise a negative error code.
> > */
> > -int usb_urb_ep_type_check(const struct urb *urb)
> > +int usb_pipe_type_check(struct usb_device *dev, unsigned int pipe)
> > {
> > const struct usb_host_endpoint *ep;
> >
> > - ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > + ep = usb_pipe_endpoint(dev, pipe);
> > if (!ep)
> > return -EINVAL;
> > - if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
> > + if (usb_pipetype(pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
> > return -EINVAL;
> > return 0;
> > }
> > +EXPORT_SYMBOL_GPL(usb_pipe_type_check);
> > +
> > +/**
> > + * usb_urb_ep_type_check - sanity check of endpoint in the given urb
> > + * @urb: urb to be checked
> > + *
> > + * This performs a light-weight sanity check for the endpoint in the
> > + * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
> > + * a negative error code.
> > + */
> > +int usb_urb_ep_type_check(const struct urb *urb)
> > +{
> > + return usb_pipe_type_check(urb->dev, urb->pipe);
> > +}
> > EXPORT_SYMBOL_GPL(usb_urb_ep_type_check);
>
> Since this routine is used in only one place in the entire kernel, you
> might as well inline the code there and get rid of the function
> entirely.
Good idea, will do.
> > diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
> > index abf99b814a0f..fc3aab04a0bc 100644
> > --- a/sound/usb/quirks.c
> > +++ b/sound/usb/quirks.c
> > @@ -846,7 +846,7 @@ static int snd_usb_accessmusic_boot_quirk(struct usb_device *dev)
> > static const u8 seq[] = { 0x4e, 0x73, 0x52, 0x01 };
> > void *buf;
> >
> > - if (snd_usb_pipe_sanity_check(dev, usb_sndintpipe(dev, 0x05)))
> > + if (usb_pipe_type_check(dev, usb_sndintpipe(dev, 0x05)))
> > return -EINVAL;
> > buf = kmemdup(seq, ARRAY_SIZE(seq), GFP_KERNEL);
> > if (!buf)
> > @@ -875,7 +875,7 @@ static int snd_usb_nativeinstruments_boot_quirk(struct usb_device *dev)
> > {
> > int ret;
> >
> > - if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0)))
> > + if (usb_pipe_type_check(dev, usb_sndctrlpipe(dev, 0)))
> > return -EINVAL;
>
> In a few places here this check is completely unnecessary. All it does
> is verify that the device does have an endpoint 0 and the the type of
> the endpoint matches the type of the pipe. Well, every USB device
> always has an endpoint 0, and it is always a bidirectional control
> endpoint.
I think this was probably added to handle syzbot issues. As long as the
USB core does ensure that a USB device has endpoint 0, I agree, these
can be removed. I'll go check that and add a follow-on patch to the
series to do this, thanks.
thanks,
greg k-h
On Thu, Sep 03, 2020 at 09:32:30AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 02, 2020 at 08:45:53PM -0400, Alan Stern wrote:
> > On Wed, Sep 02, 2020 at 01:01:03PM +0200, Greg Kroah-Hartman wrote:
> > > snd_usb_pipe_sanity_check() is a great function, so let's move it into
> > > the USB core so that other parts of the kernel, including the USB core,
> > > can call it.
> > >
> > > Name it usb_pipe_type_check() to match the existing
> > > usb_urb_ep_type_check() call, which now uses this function.
> > >
> > > Cc: Jaroslav Kysela <[email protected]>
> > > Cc: Takashi Iwai <[email protected]>
> > > Cc: "Gustavo A. R. Silva" <[email protected]>
> > > Cc: Eli Billauer <[email protected]>
> > > Cc: Emiliano Ingrassia <[email protected]>
> > > Cc: Alan Stern <[email protected]>
> > > Cc: Alexander Tsoy <[email protected]>
> > > Cc: "Geoffrey D. Bennett" <[email protected]>
> > > Cc: Jussi Laako <[email protected]>
> > > Cc: Nick Kossifidis <[email protected]>
> > > Cc: Dmitry Panchenko <[email protected]>
> > > Cc: Chris Wulff <[email protected]>
> > > Cc: Jesus Ramos <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > > ---
> >
> > > diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
> > > index 27e83e55a590..45bc2914c1ba 100644
> > > --- a/drivers/usb/core/urb.c
> > > +++ b/drivers/usb/core/urb.c
> > > @@ -192,24 +192,39 @@ static const int pipetypes[4] = {
> > > };
> > >
> > > /**
> > > - * usb_urb_ep_type_check - sanity check of endpoint in the given urb
> > > - * @urb: urb to be checked
> > > + * usb_pipe_type_check - sanity check of a specific pipe for a usb device
> > > + * @dev: struct usb_device to be checked
> > > + * @pipe: pipe to check
> > > *
> > > * This performs a light-weight sanity check for the endpoint in the
> > > - * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
> > > - * a negative error code.
> > > + * given usb device. It returns 0 if the pipe is a valid for the specific usb
> > -----------------------------------------------------^
> > Typo.
>
> Oops, will fix, thanks.
>
>
> >
> > > + * device, otherwise a negative error code.
> > > */
> > > -int usb_urb_ep_type_check(const struct urb *urb)
> > > +int usb_pipe_type_check(struct usb_device *dev, unsigned int pipe)
> > > {
> > > const struct usb_host_endpoint *ep;
> > >
> > > - ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > > + ep = usb_pipe_endpoint(dev, pipe);
> > > if (!ep)
> > > return -EINVAL;
> > > - if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
> > > + if (usb_pipetype(pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
> > > return -EINVAL;
> > > return 0;
> > > }
> > > +EXPORT_SYMBOL_GPL(usb_pipe_type_check);
> > > +
> > > +/**
> > > + * usb_urb_ep_type_check - sanity check of endpoint in the given urb
> > > + * @urb: urb to be checked
> > > + *
> > > + * This performs a light-weight sanity check for the endpoint in the
> > > + * given urb. It returns 0 if the urb contains a valid endpoint, otherwise
> > > + * a negative error code.
> > > + */
> > > +int usb_urb_ep_type_check(const struct urb *urb)
> > > +{
> > > + return usb_pipe_type_check(urb->dev, urb->pipe);
> > > +}
> > > EXPORT_SYMBOL_GPL(usb_urb_ep_type_check);
> >
> > Since this routine is used in only one place in the entire kernel, you
> > might as well inline the code there and get rid of the function
> > entirely.
>
> Good idea, will do.
No, wait, the USB sound drivers call it a lot, so it needs to stick
around for now until we clean that up.
thanks,
greg k-h
On Wed, Sep 02, 2020 at 04:35:33PM +0200, Takashi Iwai wrote:
> On Wed, 02 Sep 2020 13:01:03 +0200,
> Greg Kroah-Hartman wrote:
> >
> > snd_usb_pipe_sanity_check() is a great function, so let's move it into
> > the USB core so that other parts of the kernel, including the USB core,
> > can call it.
> >
> > Name it usb_pipe_type_check() to match the existing
> > usb_urb_ep_type_check() call, which now uses this function.
> >
> > Cc: Jaroslav Kysela <[email protected]>
> > Cc: Takashi Iwai <[email protected]>
> > Cc: "Gustavo A. R. Silva" <[email protected]>
> > Cc: Eli Billauer <[email protected]>
> > Cc: Emiliano Ingrassia <[email protected]>
> > Cc: Alan Stern <[email protected]>
> > Cc: Alexander Tsoy <[email protected]>
> > Cc: "Geoffrey D. Bennett" <[email protected]>
> > Cc: Jussi Laako <[email protected]>
> > Cc: Nick Kossifidis <[email protected]>
> > Cc: Dmitry Panchenko <[email protected]>
> > Cc: Chris Wulff <[email protected]>
> > Cc: Jesus Ramos <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> Reviewed-by: Takashi Iwai <[email protected]>
Thanks for the reviews of all of these.
greg k-h
On Mon, Sep 07, 2020 at 04:16:34PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 03, 2020 at 09:32:30AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 02, 2020 at 08:45:53PM -0400, Alan Stern wrote:
> > > Since this routine is used in only one place in the entire kernel, you
> > > might as well inline the code there and get rid of the function
> > > entirely.
> >
> > Good idea, will do.
>
> No, wait, the USB sound drivers call it a lot, so it needs to stick
> around for now until we clean that up.
Argh. I must have run "git grep" from within drivers/usb/core. My
mistake.
Alan Stern