2023-04-20 14:16:57

by Duoming Zhou

Subject: [PATCH net] ethernet: ixgb: fix use after free bugs caused by circular dependency problem

The watchdog_timer can schedule tx_timeout_task and tx_timeout_task
can also arm watchdog_timer. The process is shown below:

----------- timer schedules work ------------
ixgb_watchdog() //timer handler
schedule_work(&adapter->tx_timeout_task)

----------- work arms timer ------------
ixgb_tx_timeout_task() //workqueue callback function
ixgb_up()
mod_timer(&adapter->watchdog_timer,...)

When ixgb device is detaching, the timer and workqueue
could still be rearmed. The process is shown below:

(cleanup routine) | (timer and workqueue routine)
ixgb_remove() |
| ixgb_tx_timeout_task() //workqueue
| ixgb_up()
| mod_timer()
cancel_work_sync() |
free_netdev(netdev) //FREE | ixgb_watchdog() //timer
| netif_carrier_ok(netdev) //USE

This patch adds timer_shutdown_sync() in ixgb_remove(), which
could prevent rearming of the timer from the workqueue.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Duoming Zhou <[email protected]>
---
drivers/net/ethernet/intel/ixgb/ixgb_main.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
index b4d47e7a76c..6ce3601904b 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
@@ -516,6 +516,7 @@ ixgb_remove(struct pci_dev *pdev)
struct net_device *netdev = pci_get_drvdata(pdev);
struct ixgb_adapter *adapter = netdev_priv(netdev);

+ timer_shutdown_sync(&adapter->watchdog_timer);
cancel_work_sync(&adapter->tx_timeout_task);

unregister_netdev(netdev);
--
2.17.1

2023-04-20 14:41:35

by Jakub Kicinski

Subject: Re: [PATCH net] ethernet: ixgb: fix use after free bugs caused by circular dependency problem

On Thu, 20 Apr 2023 22:01:57 +0800 Duoming Zhou wrote:
> The watchdog_timer can schedule tx_timeout_task and tx_timeout_task
> can also arm watchdog_timer. The process is shown below:
>
> ----------- timer schedules work ------------
> ixgb_watchdog() //timer handler
> schedule_work(&adapter->tx_timeout_task)
>
> ----------- work arms timer ------------
> ixgb_tx_timeout_task() //workqueue callback function
> ixgb_up()
> mod_timer(&adapter->watchdog_timer,...)
>
> When ixgb device is detaching, the timer and workqueue
> could still be rearmed. The process is shown below:
>
> (cleanup routine) | (timer and workqueue routine)
> ixgb_remove() |
> | ixgb_tx_timeout_task() //workqueue
> | ixgb_up()
> | mod_timer()
> cancel_work_sync() |
> free_netdev(netdev) //FREE | ixgb_watchdog() //timer
> | netif_carrier_ok(netdev) //USE
>
> This patch adds timer_shutdown_sync() in ixgb_remove(), which
> could prevent rearming of the timer from the workqueue.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Duoming Zhou <[email protected]>

The driver has been removed. No point.