Message ID | 20200615155029.21002-1-bruceshenzk@gmail.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [v2] net: alx: fix race condition in alx_remove | expand |
From: Zekun Shen <bruceshenzk@gmail.com> Date: Mon, 15 Jun 2020 11:50:29 -0400 > There is a race condition exist during termination. The path is > alx_stop and then alx_remove. An alx_schedule_link_check could be called > before alx_stop by interrupt handler and invoke alx_link_check later. > Alx_stop frees the napis, and alx_remove cancels any pending works. > If any of the work is scheduled before termination and invoked before > alx_remove, a null-ptr-deref occurs because both expect alx->napis[i]. > > This patch fix the race condition by moving cancel_work_sync functions > before alx_free_napis inside alx_stop. Because interrupt handler can call > alx_schedule_link_check again, alx_free_irq is moved before > cancel_work_sync calls too. > > Signed-off-by: Zekun Shen <bruceshenzk@gmail.com> Applied, thank you.
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c index b9b4edb91..9b7f1af5f 100644 --- a/drivers/net/ethernet/atheros/alx/main.c +++ b/drivers/net/ethernet/atheros/alx/main.c @@ -1249,8 +1249,12 @@ static int __alx_open(struct alx_priv *alx, bool resume) static void __alx_stop(struct alx_priv *alx) { - alx_halt(alx); alx_free_irq(alx); + + cancel_work_sync(&alx->link_check_wk); + cancel_work_sync(&alx->reset_wk); + + alx_halt(alx); alx_free_rings(alx); alx_free_napis(alx); } @@ -1855,9 +1859,6 @@ static void alx_remove(struct pci_dev *pdev) struct alx_priv *alx = pci_get_drvdata(pdev); struct alx_hw *hw = &alx->hw; - cancel_work_sync(&alx->link_check_wk); - cancel_work_sync(&alx->reset_wk); - /* restore permanent mac address */ alx_set_macaddr(hw, hw->perm_addr);
There is a race condition exist during termination. The path is alx_stop and then alx_remove. An alx_schedule_link_check could be called before alx_stop by interrupt handler and invoke alx_link_check later. Alx_stop frees the napis, and alx_remove cancels any pending works. If any of the work is scheduled before termination and invoked before alx_remove, a null-ptr-deref occurs because both expect alx->napis[i]. This patch fix the race condition by moving cancel_work_sync functions before alx_free_napis inside alx_stop. Because interrupt handler can call alx_schedule_link_check again, alx_free_irq is moved before cancel_work_sync calls too. Signed-off-by: Zekun Shen <bruceshenzk@gmail.com> --- Changes in v2: - move alx_free_irq before cancel_work_sync calls - prevent another interrupt to cause race condition drivers/net/ethernet/atheros/alx/main.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)