Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 988bd27d authored by Fedor Pchelkin's avatar Fedor Pchelkin Committed by Greg Kroah-Hartman
Browse files

wifi: ath9k: hif_usb: Fix use-after-free in ath9k_hif_usb_reg_in_cb() 



[ Upstream commit dd95f2239fc846795fc926787c3ae0ca701c9840 ]

It is possible that skb is freed in ath9k_htc_rx_msg(), then
usb_submit_urb() fails and we try to free skb again. It causes
use-after-free bug. Moreover, if alloc_skb() fails, urb->context becomes
NULL but rx_buf is not freed and there can be a memory leak.

The patch removes unnecessary nskb and makes skb processing more clear: it
is supposed that ath9k_htc_rx_msg() either frees old skb or passes its
managing to another callback function.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 3deff760 ("ath9k_htc: Increase URB count for REG_IN pipe")
Signed-off-by: default avatarFedor Pchelkin <pchelkin@ispras.ru>
Signed-off-by: default avatarAlexey Khoroshilov <khoroshilov@ispras.ru>
Acked-by: default avatarToke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: default avatarKalle Valo <quic_kvalo@quicinc.com>
Link: https://lore.kernel.org/r/20221008114917.21404-1-pchelkin@ispras.ru


Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent 9850791d

drivers/net/wireless/ath/ath9k/hif_usb.c

+15 −13
Original line number Diff line number Diff line
@@ -709,14 +709,13 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb) 
	struct rx_buf *rx_buf = (struct rx_buf *)urb->context;

	struct hif_device_usb *hif_dev = rx_buf->hif_dev;

	struct sk_buff *skb = rx_buf->skb;

	struct sk_buff *nskb;

	int ret;



	if (!skb)

		return;



	if (!hif_dev)

		goto free;

		goto free_skb;



	switch (urb->status) {

	case 0:
@@ -725,7 +724,7 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb) 
	case -ECONNRESET:

	case -ENODEV:

	case -ESHUTDOWN:

		goto free;

		goto free_skb;

	default:

		skb_reset_tail_pointer(skb);

		skb_trim(skb, 0);
@@ -736,25 +735,27 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb) 
	if (likely(urb->actual_length != 0)) {

		skb_put(skb, urb->actual_length);



		/* Process the command first */

		/*

		 * Process the command first.

		 * skb is either freed here or passed to be

		 * managed to another callback function.

		 */

		ath9k_htc_rx_msg(hif_dev->htc_handle, skb,

				 skb->len, USB_REG_IN_PIPE);





		nskb = alloc_skb(MAX_REG_IN_BUF_SIZE, GFP_ATOMIC);

		if (!nskb) {

		skb = alloc_skb(MAX_REG_IN_BUF_SIZE, GFP_ATOMIC);

		if (!skb) {

			dev_err(&hif_dev->udev->dev,

				"ath9k_htc: REG_IN memory allocation failure\n");

			urb->context = NULL;

			return;

			goto free_rx_buf;

		}



		rx_buf->skb = nskb;

		rx_buf->skb = skb;



		usb_fill_int_urb(urb, hif_dev->udev,

				 usb_rcvintpipe(hif_dev->udev,

						 USB_REG_IN_PIPE),

				 nskb->data, MAX_REG_IN_BUF_SIZE,

				 skb->data, MAX_REG_IN_BUF_SIZE,

				 ath9k_hif_usb_reg_in_cb, rx_buf, 1);

	}
@@ -763,12 +764,13 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb) 
	ret = usb_submit_urb(urb, GFP_ATOMIC);

	if (ret) {

		usb_unanchor_urb(urb);

		goto free;

		goto free_skb;

	}



	return;

free:

free_skb:

	kfree_skb(skb);

free_rx_buf:

	kfree(rx_buf);

	urb->context = NULL;

}