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 perex@perex.cz Cc: Takashi Iwai tiwai@suse.com Cc: "Gustavo A. R. Silva" gustavoars@kernel.org Cc: Eli Billauer eli.billauer@gmail.com Cc: Emiliano Ingrassia ingrassia@epigenesys.com Cc: Alan Stern stern@rowland.harvard.edu Cc: Alexander Tsoy alexander@tsoy.me Cc: "Geoffrey D. Bennett" g@b4.vu Cc: Jussi Laako jussi@sonarnerd.net Cc: Nick Kossifidis mickflemm@gmail.com Cc: Dmitry Panchenko dmitry@d-systems.ee Cc: Chris Wulff crwulff@gmail.com Cc: Jesus Ramos jesus-ramos@live.com Cc: linux-usb@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
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