Re: [PATCH] staging: wlan-ng: properly check endpoint types

[Rustam Kovhaev]

On Sun, Jul 19, 2020 at 11:23:38AM +0200, Greg KH wrote:
> On Sat, Jul 18, 2020 at 08:58:36AM -0700, Rustam Kovhaev wrote:
> > As syzkaller detected, wlan-ng driver submits bulk urb without checking
> > that the endpoint type is actually bulk, add usb_urb_ep_type_check()
> > 
> > Reported-and-tested-by: syzbot+c2a1fa67c02faa0de723@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Link: https://syzkaller.appspot.com/bug?extid=c2a1fa67c02faa0de723
> > Signed-off-by: Rustam Kovhaev <rkovhaev@xxxxxxxxx>
> > ---
> >  drivers/staging/wlan-ng/hfa384x_usb.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c
> > index fa1bf8b069fd..7cde60ea68a2 100644
> > --- a/drivers/staging/wlan-ng/hfa384x_usb.c
> > +++ b/drivers/staging/wlan-ng/hfa384x_usb.c
> > @@ -339,6 +339,12 @@ static int submit_rx_urb(struct hfa384x *hw, gfp_t memflags)
> >  
> >  	hw->rx_urb_skb = skb;
> >  
> > +	result = usb_urb_ep_type_check(&hw->rx_urb);
> > +	if (result) {
> > +	       netdev_warn(hw->wlandev->netdev, "invalid rx endpoint");
> > +	       goto cleanup;
> > +	}
> 
> In looking at this again, can you just make these checks in the probe
> function, and abort binding the driver to the device at that point in
> time?  This feels really late in the init sequence.
> 
> The real problem here is in the hfa384x_create() function, where it
> blindly takes the 1 and 2 endpoints and assumes that those are the
> "correct type".  How about checking the types there, and if they are
> incorrect, returning an error from that function and have the caller
> return the error as well.
> 
> That should keep anything else in the driver from being initialized and
> set up, and stop bad devices from being bound to the driver at a much
> earlier point in time.
> 
> Note, just checking for the valid type/direction of those endpoints
> should be sufficient.
> 
tyvm for the feedback! makes perfect sense, i'll send a new patch