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

Rustam Kovhaev
2020-07-19T18:12:06Z

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@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=c2a1fa67c02faa0de723
> > Signed-off-by: Rustam Kovhaev <rkovhaev@gmail.com>
> > ---
> > 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