[PATCH] staging: rtl8712: handle firmware load failure
when firmware fails to load we should not call unregister_netdev()
this patch fixes a race condition between rtl871x_load_fw_cb() and
r871xu_dev_remove() and fixes the bug reported by syzbot
Reported-by: syzbot+80899a8a8efe8968c...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=80899a8a8efe8968cde7
Signed-off-by: Rustam Kovhaev
---
drivers/staging/rtl8712/hal_init.c | 3 ++-
drivers/staging/rtl8712/usb_intf.c | 11 ---
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8712/hal_init.c
b/drivers/staging/rtl8712/hal_init.c
index 40145c0338e4..42c0a3c947f1 100644
--- a/drivers/staging/rtl8712/hal_init.c
+++ b/drivers/staging/rtl8712/hal_init.c
@@ -33,7 +33,6 @@ static void rtl871x_load_fw_cb(const struct firmware
*firmware, void *context)
{
struct _adapter *adapter = context;
- complete(&adapter->rtl8712_fw_ready);
if (!firmware) {
struct usb_device *udev = adapter->dvobjpriv.pusbdev;
struct usb_interface *usb_intf = adapter->pusb_intf;
@@ -41,11 +40,13 @@ static void rtl871x_load_fw_cb(const struct firmware
*firmware, void *context)
dev_err(&udev->dev, "r8712u: Firmware request failed\n");
usb_put_dev(udev);
usb_set_intfdata(usb_intf, NULL);
+ complete(&adapter->rtl8712_fw_ready);
return;
}
adapter->fw = firmware;
/* firmware available - start netdev */
register_netdev(adapter->pnetdev);
+ complete(&adapter->rtl8712_fw_ready);
}
static const char firmware_file[] = "rtlwifi/rtl8712u.bin";
diff --git a/drivers/staging/rtl8712/usb_intf.c
b/drivers/staging/rtl8712/usb_intf.c
index a87562f632a7..2fcd65260f4c 100644
--- a/drivers/staging/rtl8712/usb_intf.c
+++ b/drivers/staging/rtl8712/usb_intf.c
@@ -595,13 +595,17 @@ static void r871xu_dev_remove(struct usb_interface
*pusb_intf)
if (pnetdev) {
struct _adapter *padapter = netdev_priv(pnetdev);
- usb_set_intfdata(pusb_intf, NULL);
- release_firmware(padapter->fw);
/* never exit with a firmware callback pending */
wait_for_completion(&padapter->rtl8712_fw_ready);
+ pnetdev = usb_get_intfdata(pusb_intf);
+ usb_set_intfdata(pusb_intf, NULL);
+ if (!pnetdev)
+ goto firmware_load_fail;
+ release_firmware(padapter->fw);
if (drvpriv.drv_registered)
padapter->surprise_removed = true;
- unregister_netdev(pnetdev); /* will call netdev_close() */
+ if (pnetdev->reg_state != NETREG_UNINITIALIZED)
+ unregister_netdev(pnetdev); /* will call netdev_close()
*/
flush_scheduled_work();
udelay(1);
/* Stop driver mlme relation timer */
@@ -614,6 +618,7 @@ static void r871xu_dev_remove(struct usb_interface
*pusb_intf)
*/
usb_put_dev(udev);
}
+firmware_load_fail:
/* If we didn't unplug usb dongle and remove/insert module, driver
* fails on sitesurvey for the first time when device is up.
* Reset usb port for sitesurvey fail issue.
--
2.27.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wlan-ng: properly check endpoint types
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+c2a1fa67c02faa0de...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=c2a1fa67c02faa0de723
Signed-off-by: Rustam Kovhaev
---
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;
+ }
+
result = -ENOLINK;
if (!hw->wlandev->hwremoved &&
!test_bit(WORK_RX_HALT, &hw->usb_flags)) {
@@ -354,6 +360,7 @@ static int submit_rx_urb(struct hfa384x *hw, gfp_t memflags)
}
}
+cleanup:
/* Don't leak memory if anything should go wrong */
if (result != 0) {
dev_kfree_skb(skb);
@@ -388,6 +395,12 @@ static int submit_tx_urb(struct hfa384x *hw, struct urb
*tx_urb, gfp_t memflags)
struct net_device *netdev = hw->wlandev->netdev;
int result;
+ result = usb_urb_ep_type_check(&hw->tx_urb);
+ if (result) {
+ netdev_warn(hw->wlandev->netdev, "invalid tx endpoint");
+ goto done;
+ }
+
result = -ENOLINK;
if (netif_running(netdev)) {
if (!hw->wlandev->hwremoved &&
@@ -407,6 +420,7 @@ static int submit_tx_urb(struct hfa384x *hw, struct urb
*tx_urb, gfp_t memflags)
}
}
+done:
return result;
}
--
2.27.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wlan-ng: properly check endpoint types
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+c2a1fa67c02faa0de...@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=c2a1fa67c02faa0de723
> > Signed-off-by: Rustam Kovhaev
> > ---
> > 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wlan-ng: properly check endpoint types
As syzkaller detected, wlan-ng driver does not do sanity check of
endpoints in prism2sta_probe_usb(), add check for xfer direction and type
Reported-and-tested-by: syzbot+c2a1fa67c02faa0de...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=c2a1fa67c02faa0de723
Signed-off-by: Rustam Kovhaev
---
drivers/staging/wlan-ng/prism2usb.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/wlan-ng/prism2usb.c
b/drivers/staging/wlan-ng/prism2usb.c
index 4689b2170e4f..456603fd26c0 100644
--- a/drivers/staging/wlan-ng/prism2usb.c
+++ b/drivers/staging/wlan-ng/prism2usb.c
@@ -61,11 +61,25 @@ static int prism2sta_probe_usb(struct usb_interface
*interface,
const struct usb_device_id *id)
{
struct usb_device *dev;
-
+ const struct usb_endpoint_descriptor *epd;
+ const struct usb_host_interface *iface_desc = interface->cur_altsetting;
struct wlandevice *wlandev = NULL;
struct hfa384x *hw = NULL;
int result = 0;
+ if (iface_desc->desc.bNumEndpoints != 2) {
+ result = -ENODEV;
+ goto failed;
+ }
+
+ result = -EINVAL;
+ epd = &iface_desc->endpoint[1].desc;
+ if (!usb_endpoint_is_bulk_in(epd))
+ goto failed;
+ epd = &iface_desc->endpoint[2].desc;
+ if (!usb_endpoint_is_bulk_out(epd))
+ goto failed;
+
dev = interface_to_usbdev(interface);
wlandev = create_wlan();
if (!wlandev) {
--
2.27.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wlan-ng: fix out of bounds read in prism2sta_probe_usb()
let's use usb_find_common_endpoints() to discover endpoints, it does all
necessary checks for type and xfer direction
remove memset() in hfa384x_create(), because we now assign endpoints in
prism2sta_probe_usb() and because create_wlan() uses kzalloc() to
allocate hfa384x struct before calling hfa384x_create()
Fixes: faaff9765664 ("staging: wlan-ng: properly check endpoint types")
Reported-and-tested-by: syzbot+22794221ab96b0bab...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=22794221ab96b0bab53a
Signed-off-by: Rustam Kovhaev
---
drivers/staging/wlan-ng/hfa384x_usb.c | 5 -
drivers/staging/wlan-ng/prism2usb.c | 19 ++-
2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c
b/drivers/staging/wlan-ng/hfa384x_usb.c
index fa1bf8b069fd..2720f7319a3d 100644
--- a/drivers/staging/wlan-ng/hfa384x_usb.c
+++ b/drivers/staging/wlan-ng/hfa384x_usb.c
@@ -524,13 +524,8 @@ static void hfa384x_usb_defer(struct work_struct *data)
*/
void hfa384x_create(struct hfa384x *hw, struct usb_device *usb)
{
- memset(hw, 0, sizeof(*hw));
hw->usb = usb;
- /* set up the endpoints */
- hw->endp_in = usb_rcvbulkpipe(usb, 1);
- hw->endp_out = usb_sndbulkpipe(usb, 2);
-
/* Set up the waitq */
init_waitqueue_head(&hw->cmdq);
diff --git a/drivers/staging/wlan-ng/prism2usb.c
b/drivers/staging/wlan-ng/prism2usb.c
index 456603fd26c0..4b08dc1da4f9 100644
--- a/drivers/staging/wlan-ng/prism2usb.c
+++ b/drivers/staging/wlan-ng/prism2usb.c
@@ -61,23 +61,14 @@ static int prism2sta_probe_usb(struct usb_interface
*interface,
const struct usb_device_id *id)
{
struct usb_device *dev;
- const struct usb_endpoint_descriptor *epd;
- const struct usb_host_interface *iface_desc = interface->cur_altsetting;
+ struct usb_endpoint_descriptor *bulk_in, *bulk_out;
+ struct usb_host_interface *iface_desc = interface->cur_altsetting;
struct wlandevice *wlandev = NULL;
struct hfa384x *hw = NULL;
int result = 0;
- if (iface_desc->desc.bNumEndpoints != 2) {
- result = -ENODEV;
- goto failed;
- }
-
- result = -EINVAL;
- epd = &iface_desc->endpoint[1].desc;
- if (!usb_endpoint_is_bulk_in(epd))
- goto failed;
- epd = &iface_desc->endpoint[2].desc;
- if (!usb_endpoint_is_bulk_out(epd))
+ result = usb_find_common_endpoints(iface_desc, &bulk_in, &bulk_out,
NULL, NULL);
+ if (result)
goto failed;
dev = interface_to_usbdev(interface);
@@ -96,6 +87,8 @@ static int prism2sta_probe_usb(struct usb_interface
*interface,
}
/* Initialize the hw data */
+ hw->endp_in = usb_rcvbulkpipe(dev, bulk_in->bEndpointAddress);
+ hw->endp_out = usb_sndbulkpipe(dev, bulk_out->bEndpointAddress);
hfa384x_create(hw, dev);
hw->wlandev = wlandev;
--
2.27.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
