[PATCH wireless] rtlwifi: rtl_pci: Fix possible use-after-free caused by unfinished tasklet

[PATCH wireless] rtlwifi: rtl_pci: Fix possible use-after-free caused by unfinished tasklet

[Duoming Zhou]

The irq_prepare_bcn_tasklet is initialized in rtl_pci_init() and
scheduled when RTL_IMR_BCNINT interrupt is triggered by hardware.
But it is never killed in rtl_pci_deinit(). When the rtlwifi card
probe fails or is being detached, the ieee80211_hw is deallocated.
However, irq_prepare_bcn_tasklet may still be running or pending,
leading to use-after-free when the freed ieee80211_hw is accessed
in _rtl_pci_prepare_bcn_tasklet().

Similar to irq_tasklet, add tasklet_kill() in rtl_pci_deinit() to
ensure that irq_prepare_bcn_tasklet is properly terminated before
the ieee80211_hw is released.

The issue was identified through static analysis.

Fixes: 0c8173385e54 ("rtl8192ce: Add new driver")
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
---
 drivers/net/wireless/realtek/rtlwifi/pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index d080469264c..f0010336e78 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -1674,6 +1674,7 @@ static void rtl_pci_deinit(struct ieee80211_hw *hw)
 
 	synchronize_irq(rtlpci->pdev->irq);
 	tasklet_kill(&rtlpriv->works.irq_tasklet);
+	tasklet_kill(&rtlpriv->works.irq_prepare_bcn_tasklet);
 	cancel_work_sync(&rtlpriv->works.lps_change_work);
 }
 
-- 
2.34.1

RE: [PATCH wireless] rtlwifi: rtl_pci: Fix possible use-after-free caused by unfinished tasklet

[Ping-Ke Shih]

Duoming Zhou <duoming@zju.edu.cn> wrote:
> The irq_prepare_bcn_tasklet is initialized in rtl_pci_init() and
> scheduled when RTL_IMR_BCNINT interrupt is triggered by hardware.
> But it is never killed in rtl_pci_deinit(). When the rtlwifi card
> probe fails or is being detached, the ieee80211_hw is deallocated.
> However, irq_prepare_bcn_tasklet may still be running or pending,
> leading to use-after-free when the freed ieee80211_hw is accessed
> in _rtl_pci_prepare_bcn_tasklet().
> 
> Similar to irq_tasklet, add tasklet_kill() in rtl_pci_deinit() to
> ensure that irq_prepare_bcn_tasklet is properly terminated before
> the ieee80211_hw is released.
> 
> The issue was identified through static analysis.
> 
> Fixes: 0c8173385e54 ("rtl8192ce: Add new driver")
> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>

The tree selection should be "rtw-next" (may be "rtw", but I don't think
this patch is urgent), and subject prefix should be "wifi: rtlwifi: ...".
More, I'd point out the name of tasklet in subject. Then,
"[PATCH rtw-next] wifi: rtlwifi: pci: fix possible use-after-free caused by unfinished irq_prepare_bcn_tasklet"

Otherwise, looks good to me.

Acked-by: Ping-Ke Shih <pkshih@realtek.com>


> ---
>  drivers/net/wireless/realtek/rtlwifi/pci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index d080469264c..f0010336e78 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -1674,6 +1674,7 @@ static void rtl_pci_deinit(struct ieee80211_hw *hw)
> 
>         synchronize_irq(rtlpci->pdev->irq);
>         tasklet_kill(&rtlpriv->works.irq_tasklet);
> +       tasklet_kill(&rtlpriv->works.irq_prepare_bcn_tasklet);
>         cancel_work_sync(&rtlpriv->works.lps_change_work);
>  }
> 
> --
> 2.34.1

Re: [PATCH wireless] rtlwifi: rtl_pci: Fix possible use-after-free caused by unfinished tasklet

[duoming]

Ping-Ke Shih <pkshih@realtek.com> wrote:
> > The irq_prepare_bcn_tasklet is initialized in rtl_pci_init() and
> > scheduled when RTL_IMR_BCNINT interrupt is triggered by hardware.
> > But it is never killed in rtl_pci_deinit(). When the rtlwifi card
> > probe fails or is being detached, the ieee80211_hw is deallocated.
> > However, irq_prepare_bcn_tasklet may still be running or pending,
> > leading to use-after-free when the freed ieee80211_hw is accessed
> > in _rtl_pci_prepare_bcn_tasklet().
> > 
> > Similar to irq_tasklet, add tasklet_kill() in rtl_pci_deinit() to
> > ensure that irq_prepare_bcn_tasklet is properly terminated before
> > the ieee80211_hw is released.
> > 
> > The issue was identified through static analysis.
> > 
> > Fixes: 0c8173385e54 ("rtl8192ce: Add new driver")
> > Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
> 
> The tree selection should be "rtw-next" (may be "rtw", but I don't think
> this patch is urgent), and subject prefix should be "wifi: rtlwifi: ...".
> More, I'd point out the name of tasklet in subject. Then,
> "[PATCH rtw-next] wifi: rtlwifi: pci: fix possible use-after-free caused by unfinished irq_prepare_bcn_tasklet"

Thank you for your time and reply! I will select the "rtw-next" tree,
adjust the subject prefix and send a v2 patch.

> Otherwise, looks good to me.
> 
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>

Best regards,
Duoming Zhou