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? drivers/net/wireless/ath/ath9k/htc_drv_init.c | 6 +++--- drivers/net/wireless/ath/ath9k/htc_hst.c | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c index ff61ae34ecdf..e497a44aff88 100644 --- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c +++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c @@ -931,7 +931,6 @@ static int ath9k_init_device(struct ath9k_htc_priv *priv, int ath9k_htc_probe_device(struct htc_target *htc_handle, struct device *dev, u16 devid, char *product, u32 drv_info) { - struct hif_device_usb *hif_dev; struct ath9k_htc_priv *priv; struct ieee80211_hw *hw; int ret; @@ -969,10 +968,11 @@ int ath9k_htc_probe_device(struct htc_target *htc_handle, struct device *dev, err_init: ath9k_stop_wmi(priv); - hif_dev = (struct hif_device_usb *)htc_handle->hif_dev; - ath9k_hif_usb_dealloc_urbs(hif_dev); + ath9k_hif_usb_dealloc_urbs((struct hif_device_usb *)htc_handle->hif_dev); ath9k_destroy_wmi(priv); err_free: + ath9k_hif_usb_dealloc_urbs((struct hif_device_usb *)htc_handle->hif_dev); + htc_handle->drv_priv = NULL; ieee80211_free_hw(hw); return ret; } diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c index 994ec48b2f66..d461eca389ab 100644 --- a/drivers/net/wireless/ath/ath9k/htc_hst.c +++ b/drivers/net/wireless/ath/ath9k/htc_hst.c @@ -468,6 +468,9 @@ void ath9k_htc_rx_msg(struct htc_target *htc_handle, } } +/* A dummy object used until device is initialized. */ +static struct ath9k_htc_priv ath9k_uninitialized_htc_priv; + struct htc_target *ath9k_htc_hw_alloc(void *hif_handle, struct ath9k_htc_hif *hif, struct device *dev) @@ -493,6 +496,8 @@ struct htc_target *ath9k_htc_hw_alloc(void *hif_handle, atomic_set(&target->tgt_ready, 0); + target->drv_priv = &ath9k_uninitialized_htc_priv; + return target; } -- 2.34.1
