Hi Tetsuo, On 4/29/22 14:18, Tetsuo Handa wrote:
Although hif_dev->htc_handle is allocated by ath9k_htc_hw_alloc() from ath9k_hif_usb_firmware_cb(), hif_dev->htc_handle->drv_priv is not assigned until ieee80211_alloc_hw() from ath9k_htc_probe_device() from ath9k_htc_hw_init() from ath9k_hif_usb_firmware_cb() returns. However, as soon as ath9k_hif_usb_alloc_rx_urbs() from ath9k_hif_usb_alloc_urbs() from ath9k_hif_usb_dev_init() from ath9k_hif_usb_firmware_cb() returns, a timer interrupt can access hif_dev->htc_handle->drv_priv via RX_STAT_INC() from ath9k_hif_usb_rx_stream() from ath9k_hif_usb_rx_cb() from usb_hcd_giveback_urb(), which results in NULL pointer deference problem. Also, even after htc_handle->drv_priv is assigned, when ath9k_htc_wait_for_target() from ath9k_htc_probe_device() from ath9k_htc_hw_init() from ath9k_hif_usb_firmware_cb() fails, ieee80211_free_hw() (which does not reset hif_dev->htc_handle->drv_priv) is immediately called due to "goto err_free;". As a result, a timer interrupt which happens after ieee80211_free_hw() triggers use-after-free problem at the abovementioned location. We can flush in-flight requests by calling ath9k_hif_usb_dealloc_urbs() before calling ieee80211_free_hw(). But we need to take from two choices for not yet assigned case. One is to change e.g. RX_STAT_INC() to check for NULL because it depends on CONFIG_ATH9K_HTC_DEBUGFS=y. The other is to assign a dummy object which is used until initialization. This patch took the latter. Link: https://syzkaller.appspot.com/bug?extid=03110230a11411024147 Reported-by: syzbot <syzbot+03110230a11411024147@xxxxxxxxxxxxxxxxxxxxxxxxx> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> Tested-by: syzbot <syzbot+03110230a11411024147@xxxxxxxxxxxxxxxxxxxxxxxxx> --- Pavel Skripkin has tested "check for NULL" approach, but not yet accepted. What was wrong with Pavel's approach?
I don't know. IIRC the problem is that nobody has tested my patch on real hw, so they can't accept it as-is. And maybe it just got lost
You can check out [1] thread. It's the latest version I have posted[1] https://lore.kernel.org/all/80962aae265995d1cdb724f5362c556d494c7566.1644265120.git.paskripkin@xxxxxxxxx/
With regards, Pavel Skripkin
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
