| Message ID | 20230103143202.274163-1-pchelkin@ispras.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] wifi: ath9k: htc_hst: free skb in ath9k_htc_rx_msg() if there is no callback function | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Fedor Pchelkin <pchelkin@ispras.ru> writes: > It is stated that ath9k_htc_rx_msg() either frees the provided skb or > passes its management to another callback function. However, the skb is > not freed in case there is no another callback function, and Syzkaller was > able to cause a memory leak. Also minor comment fix. > > Found by Linux Verification Center (linuxtesting.org) with Syzkaller. > > Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.") > Reported-by: syzbot+e008dccab31bd3647609@syzkaller.appspotmail.com > Reported-by: syzbot+6692c72009680f7c4eb2@syzkaller.appspotmail.com > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> > --- > v1->v2: added Reported-by tag > > drivers/net/wireless/ath/ath9k/htc_hst.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c > index ca05b07a45e6..7d5041eb5f29 100644 > --- a/drivers/net/wireless/ath/ath9k/htc_hst.c > +++ b/drivers/net/wireless/ath/ath9k/htc_hst.c > @@ -391,7 +391,7 @@ static void ath9k_htc_fw_panic_report(struct htc_target *htc_handle, > * HTC Messages are handled directly here and the obtained SKB > * is freed. > * > - * Service messages (Data, WMI) passed to the corresponding > + * Service messages (Data, WMI) are passed to the corresponding > * endpoint RX handlers, which have to free the SKB. > */ > void ath9k_htc_rx_msg(struct htc_target *htc_handle, > @@ -478,6 +478,8 @@ void ath9k_htc_rx_msg(struct htc_target *htc_handle, > if (endpoint->ep_callbacks.rx) > endpoint->ep_callbacks.rx(endpoint->ep_callbacks.priv, > skb, epid); > + else > + kfree_skb(skb); Shouldn't this be 'goto invalid' like all the other error paths in that function? -Toke
> Shouldn't this be 'goto invalid' like all the other error paths in that > function? It should. What is also important: I mistakenly chose kfree_skb() instead of dev_kfree_skb_any() in another patch so I must fix it. Thanks)
diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c index ca05b07a45e6..7d5041eb5f29 100644 --- a/drivers/net/wireless/ath/ath9k/htc_hst.c +++ b/drivers/net/wireless/ath/ath9k/htc_hst.c @@ -391,7 +391,7 @@ static void ath9k_htc_fw_panic_report(struct htc_target *htc_handle, * HTC Messages are handled directly here and the obtained SKB * is freed. * - * Service messages (Data, WMI) passed to the corresponding + * Service messages (Data, WMI) are passed to the corresponding * endpoint RX handlers, which have to free the SKB. */ void ath9k_htc_rx_msg(struct htc_target *htc_handle, @@ -478,6 +478,8 @@ void ath9k_htc_rx_msg(struct htc_target *htc_handle, if (endpoint->ep_callbacks.rx) endpoint->ep_callbacks.rx(endpoint->ep_callbacks.priv, skb, epid); + else + kfree_skb(skb); } }
It is stated that ath9k_htc_rx_msg() either frees the provided skb or passes its management to another callback function. However, the skb is not freed in case there is no another callback function, and Syzkaller was able to cause a memory leak. Also minor comment fix. Found by Linux Verification Center (linuxtesting.org) with Syzkaller. Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.") Reported-by: syzbot+e008dccab31bd3647609@syzkaller.appspotmail.com Reported-by: syzbot+6692c72009680f7c4eb2@syzkaller.appspotmail.com Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> --- v1->v2: added Reported-by tag drivers/net/wireless/ath/ath9k/htc_hst.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)