| Message ID | 20221228224008.146343-1-pchelkin@ispras.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | wifi: ath9k: hif_usb: clean up skbs if ath9k_hif_usb_rx_stream() fails | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Fedor Pchelkin <pchelkin@ispras.ru> writes: > Syzkaller detected a memory leak of skbs in ath9k_hif_usb_rx_stream(). > While processing skbs in ath9k_hif_usb_rx_stream(), the already allocated > skbs in skb_pool are not freed if ath9k_hif_usb_rx_stream() fails. If we > have an incorrect pkt_len or pkt_tag, the skb is dropped and all the > associated skb_pool buffers should be cleaned, too. > > Found by Linux Verification Center (linuxtesting.org) with Syzkaller. > > Fixes: 6ce708f54cc8 ("ath9k: Fix out-of-bound memcpy in ath9k_hif_usb_rx_stream") > Fixes: 44b23b488d44 ("ath9k: hif_usb: Reduce indent 1 column") > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> Is this the same issue reported in https://lore.kernel.org/r/000000000000f3e5f805f133d3f7@google.com ? If so, could you please tag the patch appropriately? -Toke
> Is this the same issue reported in > https://lore.kernel.org/r/000000000000f3e5f805f133d3f7@google.com ? Actually, this issue is fixed by another patch I've sent you recently: > [PATCH] wifi: ath9k: htc_hst: free skb in ath9k_htc_rx_msg() if there is > no callback function I've added the relevant Reported-by tags to both patches and resent them.
Fedor Pchelkin <pchelkin@ispras.ru> writes: >> Is this the same issue reported in >> https://lore.kernel.org/r/000000000000f3e5f805f133d3f7@google.com ? > > Actually, this issue is fixed by another patch I've sent you recently: Ah, great! >> [PATCH] wifi: ath9k: htc_hst: free skb in ath9k_htc_rx_msg() if there is >> no callback function > > I've added the relevant Reported-by tags to both patches and resent them. Awesome, thank you :) -Toke
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c index 1a2e0c7eeb02..d02cec114280 100644 --- a/drivers/net/wireless/ath/ath9k/hif_usb.c +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c @@ -586,14 +586,14 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev, if (pkt_tag != ATH_USB_RX_STREAM_MODE_TAG) { RX_STAT_INC(hif_dev, skb_dropped); - return; + goto invalid_pkt; } if (pkt_len > 2 * MAX_RX_BUF_SIZE) { dev_err(&hif_dev->udev->dev, "ath9k_htc: invalid pkt_len (%x)\n", pkt_len); RX_STAT_INC(hif_dev, skb_dropped); - return; + goto invalid_pkt; } pad_len = 4 - (pkt_len & 0x3); @@ -654,6 +654,11 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev, skb_pool[i]->len, USB_WLAN_RX_PIPE); RX_STAT_INC(hif_dev, skb_completed); } + return; +invalid_pkt: + for (i = 0; i < pool_index; i++) + kfree_skb(skb_pool[i]); + return; } static void ath9k_hif_usb_rx_cb(struct urb *urb)
Syzkaller detected a memory leak of skbs in ath9k_hif_usb_rx_stream(). While processing skbs in ath9k_hif_usb_rx_stream(), the already allocated skbs in skb_pool are not freed if ath9k_hif_usb_rx_stream() fails. If we have an incorrect pkt_len or pkt_tag, the skb is dropped and all the associated skb_pool buffers should be cleaned, too. Found by Linux Verification Center (linuxtesting.org) with Syzkaller. Fixes: 6ce708f54cc8 ("ath9k: Fix out-of-bound memcpy in ath9k_hif_usb_rx_stream") Fixes: 44b23b488d44 ("ath9k: hif_usb: Reduce indent 1 column") Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> --- drivers/net/wireless/ath/ath9k/hif_usb.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)