* [PATCH 0/5] rsi: fix use-after-free, memleak and sleep-while-atomic
@ 2019-11-28 17:21 Johan Hovold
2019-11-28 17:22 ` [PATCH 1/5] rsi: fix use-after-free on failed probe and unbind Johan Hovold
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Johan Hovold @ 2019-11-28 17:21 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, linux-usb, Johan Hovold
The syzbot fuzzer has reported two separate use-after-free issues,
which are fixed by the first two patches.
Turns out there were more gems in this driver and the next two patches
fixes a memory leak and a potential sleep-while-atomic found through
inspection.
The last one tightens the seemingly broken endpoint sanity check which
would have the driver try to submit a bulk URB to the default pipe (and
fail).
Tested using a mockup device.
Johan
Johan Hovold (5):
rsi: fix use-after-free on failed probe and unbind
rsi: fix use-after-free on probe errors
rsi: fix memory leak on failed URB submission
rsi: fix non-atomic allocation in completion handler
rsi: add missing endpoint sanity checks
drivers/net/wireless/rsi/rsi_91x_hal.c | 12 +++----
drivers/net/wireless/rsi/rsi_91x_usb.c | 47 ++++++++++++++++++++------
2 files changed, 43 insertions(+), 16 deletions(-)
--
2.24.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/5] rsi: fix use-after-free on failed probe and unbind
2019-11-28 17:21 [PATCH 0/5] rsi: fix use-after-free, memleak and sleep-while-atomic Johan Hovold
@ 2019-11-28 17:22 ` Johan Hovold
2019-12-18 18:57 ` Kalle Valo
2019-11-28 17:22 ` [PATCH 2/5] rsi: fix use-after-free on probe errors Johan Hovold
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Johan Hovold @ 2019-11-28 17:22 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, linux-usb, Johan Hovold,
syzbot+b563b7f8dbe8223a51e8, stable, Siva Rebbagondla,
Prameela Rani Garnepudi, Amitkumar Karwar, Fariya Fatima
Make sure to stop both URBs before returning after failed probe as well
as on disconnect to avoid use-after-free in the completion handler.
Reported-by: syzbot+b563b7f8dbe8223a51e8@syzkaller.appspotmail.com
Fixes: a4302bff28e2 ("rsi: add bluetooth rx endpoint")
Fixes: dad0d04fa7ba ("rsi: Add RS9113 wireless driver")
Cc: stable <stable@vger.kernel.org> # 3.15
Cc: Siva Rebbagondla <siva.rebbagondla@redpinesignals.com>
Cc: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
Cc: Amitkumar Karwar <amit.karwar@redpinesignals.com>
Cc: Fariya Fatima <fariyaf@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/rsi/rsi_91x_usb.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index 53f41fc2cadf..30bed719486e 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -292,6 +292,15 @@ static void rsi_rx_done_handler(struct urb *urb)
dev_kfree_skb(rx_cb->rx_skb);
}
+static void rsi_rx_urb_kill(struct rsi_hw *adapter, u8 ep_num)
+{
+ struct rsi_91x_usbdev *dev = (struct rsi_91x_usbdev *)adapter->rsi_dev;
+ struct rx_usb_ctrl_block *rx_cb = &dev->rx_cb[ep_num - 1];
+ struct urb *urb = rx_cb->rx_urb;
+
+ usb_kill_urb(urb);
+}
+
/**
* rsi_rx_urb_submit() - This function submits the given URB to the USB stack.
* @adapter: Pointer to the adapter structure.
@@ -823,10 +832,13 @@ static int rsi_probe(struct usb_interface *pfunction,
if (adapter->priv->coex_mode > 1) {
status = rsi_rx_urb_submit(adapter, BT_EP);
if (status)
- goto err1;
+ goto err_kill_wlan_urb;
}
return 0;
+
+err_kill_wlan_urb:
+ rsi_rx_urb_kill(adapter, WLAN_EP);
err1:
rsi_deinit_usb_interface(adapter);
err:
@@ -857,6 +869,10 @@ static void rsi_disconnect(struct usb_interface *pfunction)
adapter->priv->bt_adapter = NULL;
}
+ if (adapter->priv->coex_mode > 1)
+ rsi_rx_urb_kill(adapter, BT_EP);
+ rsi_rx_urb_kill(adapter, WLAN_EP);
+
rsi_reset_card(adapter);
rsi_deinit_usb_interface(adapter);
rsi_91x_deinit(adapter);
--
2.24.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/5] rsi: fix use-after-free on probe errors
2019-11-28 17:21 [PATCH 0/5] rsi: fix use-after-free, memleak and sleep-while-atomic Johan Hovold
2019-11-28 17:22 ` [PATCH 1/5] rsi: fix use-after-free on failed probe and unbind Johan Hovold
@ 2019-11-28 17:22 ` Johan Hovold
2019-11-28 17:22 ` [PATCH 3/5] rsi: fix memory leak on failed URB submission Johan Hovold
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2019-11-28 17:22 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, linux-usb, Johan Hovold,
syzbot+1d1597a5aa3679c65b9f, stable, Prameela Rani Garnepudi,
Amitkumar Karwar
The driver would fail to stop the command timer in most error paths,
something which specifically could lead to the timer being freed while
still active on I/O errors during probe.
Fix this by making sure that each function starting the timer also stops
it in all relevant error paths.
Reported-by: syzbot+1d1597a5aa3679c65b9f@syzkaller.appspotmail.com
Fixes: b78e91bcfb33 ("rsi: Add new firmware loading method")
Cc: stable <stable@vger.kernel.org> # 4.12
Cc: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
Cc: Amitkumar Karwar <amit.karwar@redpinesignals.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/rsi/rsi_91x_hal.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_hal.c b/drivers/net/wireless/rsi/rsi_91x_hal.c
index f84250bdb8cf..6f8d5f9a9f7e 100644
--- a/drivers/net/wireless/rsi/rsi_91x_hal.c
+++ b/drivers/net/wireless/rsi/rsi_91x_hal.c
@@ -622,6 +622,7 @@ static int bl_cmd(struct rsi_hw *adapter, u8 cmd, u8 exp_resp, char *str)
bl_start_cmd_timer(adapter, timeout);
status = bl_write_cmd(adapter, cmd, exp_resp, ®out_val);
if (status < 0) {
+ bl_stop_cmd_timer(adapter);
rsi_dbg(ERR_ZONE,
"%s: Command %s (%0x) writing failed..\n",
__func__, str, cmd);
@@ -737,10 +738,9 @@ static int ping_pong_write(struct rsi_hw *adapter, u8 cmd, u8 *addr, u32 size)
}
status = bl_cmd(adapter, cmd_req, cmd_resp, str);
- if (status) {
- bl_stop_cmd_timer(adapter);
+ if (status)
return status;
- }
+
return 0;
}
@@ -828,10 +828,9 @@ static int auto_fw_upgrade(struct rsi_hw *adapter, u8 *flash_content,
status = bl_cmd(adapter, EOF_REACHED, FW_LOADING_SUCCESSFUL,
"EOF_REACHED");
- if (status) {
- bl_stop_cmd_timer(adapter);
+ if (status)
return status;
- }
+
rsi_dbg(INFO_ZONE, "FW loading is done and FW is running..\n");
return 0;
}
@@ -849,6 +848,7 @@ static int rsi_hal_prepare_fwload(struct rsi_hw *adapter)
®out_val,
RSI_COMMON_REG_SIZE);
if (status < 0) {
+ bl_stop_cmd_timer(adapter);
rsi_dbg(ERR_ZONE,
"%s: REGOUT read failed\n", __func__);
return status;
--
2.24.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/5] rsi: fix memory leak on failed URB submission
2019-11-28 17:21 [PATCH 0/5] rsi: fix use-after-free, memleak and sleep-while-atomic Johan Hovold
2019-11-28 17:22 ` [PATCH 1/5] rsi: fix use-after-free on failed probe and unbind Johan Hovold
2019-11-28 17:22 ` [PATCH 2/5] rsi: fix use-after-free on probe errors Johan Hovold
@ 2019-11-28 17:22 ` Johan Hovold
2019-11-28 17:22 ` [PATCH 4/5] rsi: fix non-atomic allocation in completion handler Johan Hovold
2019-11-28 17:22 ` [PATCH 5/5] rsi: add missing endpoint sanity checks Johan Hovold
4 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2019-11-28 17:22 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, linux-usb, Johan Hovold,
stable, Prameela Rani Garnepudi
Make sure to free the skb on failed receive-URB submission (e.g. on
disconnect or currently also due to a missing endpoint).
Fixes: a1854fae1414 ("rsi: improve RX packet handling in USB interface")
Cc: stable <stable@vger.kernel.org> # 4.17
Cc: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/rsi/rsi_91x_usb.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index 30bed719486e..2c869df1c62e 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -338,8 +338,10 @@ static int rsi_rx_urb_submit(struct rsi_hw *adapter, u8 ep_num)
rx_cb);
status = usb_submit_urb(urb, GFP_KERNEL);
- if (status)
+ if (status) {
rsi_dbg(ERR_ZONE, "%s: Failed in urb submission\n", __func__);
+ dev_kfree_skb(skb);
+ }
return status;
}
--
2.24.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] rsi: fix non-atomic allocation in completion handler
2019-11-28 17:21 [PATCH 0/5] rsi: fix use-after-free, memleak and sleep-while-atomic Johan Hovold
` (2 preceding siblings ...)
2019-11-28 17:22 ` [PATCH 3/5] rsi: fix memory leak on failed URB submission Johan Hovold
@ 2019-11-28 17:22 ` Johan Hovold
2019-11-28 17:22 ` [PATCH 5/5] rsi: add missing endpoint sanity checks Johan Hovold
4 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2019-11-28 17:22 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, linux-usb, Johan Hovold,
stable, Prameela Rani Garnepudi
USB completion handlers are called in atomic context and must
specifically not allocate memory using GFP_KERNEL.
Fixes: a1854fae1414 ("rsi: improve RX packet handling in USB interface")
Cc: stable <stable@vger.kernel.org> # 4.17
Cc: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/rsi/rsi_91x_usb.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index 2c869df1c62e..ead75574e10a 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -16,6 +16,7 @@
*/
#include <linux/module.h>
+#include <linux/types.h>
#include <net/rsi_91x.h>
#include "rsi_usb.h"
#include "rsi_hal.h"
@@ -29,7 +30,7 @@ MODULE_PARM_DESC(dev_oper_mode,
"9[Wi-Fi STA + BT LE], 13[Wi-Fi STA + BT classic + BT LE]\n"
"6[AP + BT classic], 14[AP + BT classic + BT LE]");
-static int rsi_rx_urb_submit(struct rsi_hw *adapter, u8 ep_num);
+static int rsi_rx_urb_submit(struct rsi_hw *adapter, u8 ep_num, gfp_t flags);
/**
* rsi_usb_card_write() - This function writes to the USB Card.
@@ -285,7 +286,7 @@ static void rsi_rx_done_handler(struct urb *urb)
status = 0;
out:
- if (rsi_rx_urb_submit(dev->priv, rx_cb->ep_num))
+ if (rsi_rx_urb_submit(dev->priv, rx_cb->ep_num, GFP_ATOMIC))
rsi_dbg(ERR_ZONE, "%s: Failed in urb submission", __func__);
if (status)
@@ -307,7 +308,7 @@ static void rsi_rx_urb_kill(struct rsi_hw *adapter, u8 ep_num)
*
* Return: 0 on success, a negative error code on failure.
*/
-static int rsi_rx_urb_submit(struct rsi_hw *adapter, u8 ep_num)
+static int rsi_rx_urb_submit(struct rsi_hw *adapter, u8 ep_num, gfp_t mem_flags)
{
struct rsi_91x_usbdev *dev = (struct rsi_91x_usbdev *)adapter->rsi_dev;
struct rx_usb_ctrl_block *rx_cb = &dev->rx_cb[ep_num - 1];
@@ -337,7 +338,7 @@ static int rsi_rx_urb_submit(struct rsi_hw *adapter, u8 ep_num)
rsi_rx_done_handler,
rx_cb);
- status = usb_submit_urb(urb, GFP_KERNEL);
+ status = usb_submit_urb(urb, mem_flags);
if (status) {
rsi_dbg(ERR_ZONE, "%s: Failed in urb submission\n", __func__);
dev_kfree_skb(skb);
@@ -827,12 +828,12 @@ static int rsi_probe(struct usb_interface *pfunction,
rsi_dbg(INIT_ZONE, "%s: Device Init Done\n", __func__);
}
- status = rsi_rx_urb_submit(adapter, WLAN_EP);
+ status = rsi_rx_urb_submit(adapter, WLAN_EP, GFP_KERNEL);
if (status)
goto err1;
if (adapter->priv->coex_mode > 1) {
- status = rsi_rx_urb_submit(adapter, BT_EP);
+ status = rsi_rx_urb_submit(adapter, BT_EP, GFP_KERNEL);
if (status)
goto err_kill_wlan_urb;
}
--
2.24.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/5] rsi: add missing endpoint sanity checks
2019-11-28 17:21 [PATCH 0/5] rsi: fix use-after-free, memleak and sleep-while-atomic Johan Hovold
` (3 preceding siblings ...)
2019-11-28 17:22 ` [PATCH 4/5] rsi: fix non-atomic allocation in completion handler Johan Hovold
@ 2019-11-28 17:22 ` Johan Hovold
4 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2019-11-28 17:22 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, linux-usb, Johan Hovold
The driver expects at least one bulk-in endpoint when in "wifi-alone"
operating mode and two bulk-in endpoints otherwise, and would otherwise
fail to to submit the corresponding bulk URB to the default pipe during
probe with a somewhat cryptic message:
rsi_91x: rsi_rx_urb_submit: Failed in urb submission
rsi_91x: rsi_probe: Failed in probe...Exiting
RSI-USB WLAN: probe of 2-2.4:1.0 failed with error -8
The current endpoint sanity check looks broken and would only bail out
early if there was no bulk-in endpoint but at least one bulk-out
endpoint.
Tighten this check to always require at least one bulk-in and one
bulk-out endpoint, and add the missing sanity check for a Bluetooth
bulk-in endpoint when in a BT operating mode. Also make sure to log an
informative error message when the expected endpoints are missing.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/rsi/rsi_91x_usb.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index ead75574e10a..396b9b81c1cd 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -149,9 +149,17 @@ static int rsi_find_bulk_in_and_out_endpoints(struct usb_interface *interface,
break;
}
- if (!(dev->bulkin_endpoint_addr[0]) &&
- dev->bulkout_endpoint_addr[0])
+ if (!(dev->bulkin_endpoint_addr[0] && dev->bulkout_endpoint_addr[0])) {
+ dev_err(&interface->dev, "missing wlan bulk endpoints\n");
return -EINVAL;
+ }
+
+ if (adapter->priv->coex_mode > 1) {
+ if (!dev->bulkin_endpoint_addr[1]) {
+ dev_err(&interface->dev, "missing bt bulk-in endpoint\n");
+ return -EINVAL;
+ }
+ }
return 0;
}
--
2.24.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] rsi: fix use-after-free on failed probe and unbind
2019-11-28 17:22 ` [PATCH 1/5] rsi: fix use-after-free on failed probe and unbind Johan Hovold
@ 2019-12-18 18:57 ` Kalle Valo
0 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-12-18 18:57 UTC (permalink / raw)
To: Johan Hovold
Cc: Amitkumar Karwar, Siva Rebbagondla, linux-wireless, netdev,
linux-kernel, linux-usb, Johan Hovold,
syzbot+b563b7f8dbe8223a51e8, stable, Siva Rebbagondla,
Prameela Rani Garnepudi, Amitkumar Karwar, Fariya Fatima
Johan Hovold <johan@kernel.org> wrote:
> Make sure to stop both URBs before returning after failed probe as well
> as on disconnect to avoid use-after-free in the completion handler.
>
> Reported-by: syzbot+b563b7f8dbe8223a51e8@syzkaller.appspotmail.com
> Fixes: a4302bff28e2 ("rsi: add bluetooth rx endpoint")
> Fixes: dad0d04fa7ba ("rsi: Add RS9113 wireless driver")
> Cc: stable <stable@vger.kernel.org> # 3.15
> Cc: Siva Rebbagondla <siva.rebbagondla@redpinesignals.com>
> Cc: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
> Cc: Amitkumar Karwar <amit.karwar@redpinesignals.com>
> Cc: Fariya Fatima <fariyaf@gmail.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
5 patches applied to wireless-drivers-next.git, thanks.
e93cd35101b6 rsi: fix use-after-free on failed probe and unbind
92aafe77123a rsi: fix use-after-free on probe errors
477682974811 rsi: fix memory leak on failed URB submission
b9b9f9fea218 rsi: fix non-atomic allocation in completion handler
960da557f435 rsi: add missing endpoint sanity checks
--
https://patchwork.kernel.org/patch/11266455/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-12-18 18:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-28 17:21 [PATCH 0/5] rsi: fix use-after-free, memleak and sleep-while-atomic Johan Hovold
2019-11-28 17:22 ` [PATCH 1/5] rsi: fix use-after-free on failed probe and unbind Johan Hovold
2019-12-18 18:57 ` Kalle Valo
2019-11-28 17:22 ` [PATCH 2/5] rsi: fix use-after-free on probe errors Johan Hovold
2019-11-28 17:22 ` [PATCH 3/5] rsi: fix memory leak on failed URB submission Johan Hovold
2019-11-28 17:22 ` [PATCH 4/5] rsi: fix non-atomic allocation in completion handler Johan Hovold
2019-11-28 17:22 ` [PATCH 5/5] rsi: add missing endpoint sanity checks Johan Hovold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).