Re: [PATCH] wifi: ath9k: hif_usb: don't dereference hif_dev after re-arming firmware request

[Toke Høiland-Jørgensen]

xiaoblac <teel4res@xxxxxxxxx> writes:

> From: Cheng Yongkang <teel4res@xxxxxxxxx>
>
> ath9k_hif_request_firmware() re-arms an asynchronous firmware load via
> request_firmware_nowait(), passing hif_dev as the completion context, and
> then still dereferences hif_dev:
>
> 	dev_info(&hif_dev->udev->dev, "ath9k_htc: Firmware %s requested\n",
> 		 hif_dev->fw_name);
>
> The re-armed callback ath9k_hif_usb_firmware_cb() runs on the "events"
> workqueue and, when the firmware is missing, walks the retry chain into
> ath9k_hif_usb_firmware_fail() -> complete_all(&hif_dev->fw_done). That
> releases the wait_for_completion(&hif_dev->fw_done) in a concurrent
> ath9k_hif_usb_disconnect(), which then kfree()s hif_dev. The trailing
> dev_info() in the frame that re-armed the request can therefore read freed
> memory (hif_dev->udev, the first field of struct hif_device_usb):
>
>   BUG: KASAN: slab-use-after-free in ath9k_hif_request_firmware
>   Read of size 8 ... by task kworker/...
>    ath9k_hif_request_firmware
>    ath9k_hif_usb_firmware_cb           drivers/net/wireless/ath/ath9k/hif_usb.c:1247
>    request_firmware_work_func
>   Allocated by ...:
>    ath9k_hif_usb_probe                 drivers/net/wireless/ath/ath9k/hif_usb.c
>   Freed by ...:
>    ath9k_hif_usb_disconnect -> kfree   drivers/net/wireless/ath/ath9k/hif_usb.c
>
> The fw_done barrier only makes disconnect wait for the firmware chain to
> *terminate*; it does not protect the outer ath9k_hif_request_firmware()
> frame that re-armed the request and keeps touching hif_dev afterwards.
>
> Drop the post-request dev_info(): it is the only use of hif_dev after the
> async request is armed, and it is purely informational (the dev_err() on the
> failure path runs only when request_firmware_nowait() did not arm a callback,
> so hif_dev is still alive there).
>
> This was first reported by syzbot as a single, non-reproduced crash that was
> later auto-obsoleted, and was independently rediscovered by the reFuzz fuzzer,
> which produced a C reproducer (USB-gadget connect/disconnect of an ath9k_htc
> device whose firmware download fails). The vulnerable code is unchanged and
> still present in v7.1-rc6, where the slab-use-after-free reproduces under KASAN
> once the (sub-microsecond) race window is widened.
>
> Fixes: e904cf6fe230 ("ath9k_htc: introduce support for different fw versions")
> Reported-by: syzbot+50122cbc2874b1eb25b0@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=50122cbc2874b1eb25b0
> Signed-off-by: Cheng Yongkang <teel4res@xxxxxxxxx>

Thank you for the fix!

Acked-by: Toke Høiland-Jørgensen <toke@xxxxxxx>