[alsa-devel] [PATCH v2 9/9] ALSA: line6: Add yet more sanity checks for invalid EPs

[Takashi Iwai]

On Wed, 11 Oct 2017 16:39:52 +0200,
Johan Hovold wrote:
> 
> On Wed, Oct 11, 2017 at 12:36:46PM +0200, Takashi Iwai wrote:
> > There are a few other places calling usb_submit_urb() with the URB
> > composed from the fixed endpoint without validation.  For avoiding the
> > spurious kernel warnings, add the sanity checks to appropriate
> > places.
> > 
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > ---
> >  sound/usb/line6/driver.c | 23 +++++++++++++++--------
> >  sound/usb/line6/midi.c   | 17 +++++++++++------
> >  2 files changed, 26 insertions(+), 14 deletions(-)
> > 
> > diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
> > index 0da6f68761e3..7c682b219584 100644
> > --- a/sound/usb/line6/driver.c
> > +++ b/sound/usb/line6/driver.c
> > @@ -175,17 +175,24 @@ static int line6_send_raw_message_async_part(struct message *msg,
> >  	}
> >  
> >  	msg->done += bytes;
> > -	retval = usb_submit_urb(urb, GFP_ATOMIC);
> >  
> > -	if (retval < 0) {
> > -		dev_err(line6->ifcdev, "%s: usb_submit_urb failed (%d)\n",
> > -			__func__, retval);
> > -		usb_free_urb(urb);
> > -		kfree(msg);
> > -		return retval;
> > -	}
> > +	/* sanity checks of EP before actually submitting */
> > +	retval = usb_urb_ep_type_check(urb);
> > +	if (retval < 0)
> > +		goto error;
> 
> This also seems like something which should have been done once at urb
> allocation time, rather than on every submission (just like the URB
> initialisation should have been).

Yes, ideally this would be done in the init time.  But the code
allocates a URB temporarily and fills it depending on the board
configuration, so it's not too trivial to do that without a big code
refactoring.  So the current patch is merely hardening a bit.
For obtaining the ideal code flow, we'd need the whole code
restructuring in anyway for this driver; currently it's too messy.


thanks,

Takashi