| Message ID | 20210413160318.2003699-1-weiyongjun1@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [-next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove() | expand |
On Tue, 13 Apr 2021 at 17:54, Wei Yongjun <weiyongjun1@huawei.com> wrote: > > This driver's remove path calls del_timer(). However, that function > does not wait until the timer handler finishes. This means that the > timer handler may still be running after the driver's remove function > has finished, which would result in a use-after-free. > > Fix by calling del_timer_sync(), which makes sure the timer handler > has finished, and unable to re-schedule itself. > > Fixes: 8562d4fe34a3 ("mhi: pci_generic: Add health-check") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
On 4/13/21 9:03 AM, Wei Yongjun wrote: > This driver's remove path calls del_timer(). However, that function > does not wait until the timer handler finishes. This means that the > timer handler may still be running after the driver's remove function > has finished, which would result in a use-after-free. > > Fix by calling del_timer_sync(), which makes sure the timer handler > has finished, and unable to re-schedule itself. > > Fixes: 8562d4fe34a3 ("mhi: pci_generic: Add health-check") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Reviewed-by: Hemant kumar <hemantk@codeaurora.org>
On Tue, Apr 13, 2021 at 04:03:18PM +0000, Wei Yongjun wrote: > This driver's remove path calls del_timer(). However, that function > does not wait until the timer handler finishes. This means that the > timer handler may still be running after the driver's remove function > has finished, which would result in a use-after-free. > > Fix by calling del_timer_sync(), which makes sure the timer handler > has finished, and unable to re-schedule itself. > > Fixes: 8562d4fe34a3 ("mhi: pci_generic: Add health-check") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Loic, could you please review this patch as well? Thanks, Mani > --- > drivers/bus/mhi/pci_generic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c > index 7c810f02a2ef..5b19e877d17a 100644 > --- a/drivers/bus/mhi/pci_generic.c > +++ b/drivers/bus/mhi/pci_generic.c > @@ -708,7 +708,7 @@ static void mhi_pci_remove(struct pci_dev *pdev) > struct mhi_pci_device *mhi_pdev = pci_get_drvdata(pdev); > struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl; > > - del_timer(&mhi_pdev->health_check_timer); > + del_timer_sync(&mhi_pdev->health_check_timer); > cancel_work_sync(&mhi_pdev->recovery_work); > > if (test_and_clear_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status)) { >
On Fri, May 21, 2021 at 05:47:44PM +0530, Manivannan Sadhasivam wrote: > On Tue, Apr 13, 2021 at 04:03:18PM +0000, Wei Yongjun wrote: > > This driver's remove path calls del_timer(). However, that function > > does not wait until the timer handler finishes. This means that the > > timer handler may still be running after the driver's remove function > > has finished, which would result in a use-after-free. > > > > Fix by calling del_timer_sync(), which makes sure the timer handler > > has finished, and unable to re-schedule itself. > > > > Fixes: 8562d4fe34a3 ("mhi: pci_generic: Add health-check") > > Reported-by: Hulk Robot <hulkci@huawei.com> > > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Loic, could you please review this patch as well? > Nvm, Loic did review the patch. Thanks, Mani > Thanks, > Mani > > > --- > > drivers/bus/mhi/pci_generic.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c > > index 7c810f02a2ef..5b19e877d17a 100644 > > --- a/drivers/bus/mhi/pci_generic.c > > +++ b/drivers/bus/mhi/pci_generic.c > > @@ -708,7 +708,7 @@ static void mhi_pci_remove(struct pci_dev *pdev) > > struct mhi_pci_device *mhi_pdev = pci_get_drvdata(pdev); > > struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl; > > > > - del_timer(&mhi_pdev->health_check_timer); > > + del_timer_sync(&mhi_pdev->health_check_timer); > > cancel_work_sync(&mhi_pdev->recovery_work); > > > > if (test_and_clear_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status)) { > >
On Tue, Apr 13, 2021 at 04:03:18PM +0000, Wei Yongjun wrote: > This driver's remove path calls del_timer(). However, that function > does not wait until the timer handler finishes. This means that the > timer handler may still be running after the driver's remove function > has finished, which would result in a use-after-free. > > Fix by calling del_timer_sync(), which makes sure the timer handler > has finished, and unable to re-schedule itself. > > Fixes: 8562d4fe34a3 ("mhi: pci_generic: Add health-check") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Applied to mhi-fixes! Thanks, Mani > --- > drivers/bus/mhi/pci_generic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c > index 7c810f02a2ef..5b19e877d17a 100644 > --- a/drivers/bus/mhi/pci_generic.c > +++ b/drivers/bus/mhi/pci_generic.c > @@ -708,7 +708,7 @@ static void mhi_pci_remove(struct pci_dev *pdev) > struct mhi_pci_device *mhi_pdev = pci_get_drvdata(pdev); > struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl; > > - del_timer(&mhi_pdev->health_check_timer); > + del_timer_sync(&mhi_pdev->health_check_timer); > cancel_work_sync(&mhi_pdev->recovery_work); > > if (test_and_clear_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status)) { >
diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c index 7c810f02a2ef..5b19e877d17a 100644 --- a/drivers/bus/mhi/pci_generic.c +++ b/drivers/bus/mhi/pci_generic.c @@ -708,7 +708,7 @@ static void mhi_pci_remove(struct pci_dev *pdev) struct mhi_pci_device *mhi_pdev = pci_get_drvdata(pdev); struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl; - del_timer(&mhi_pdev->health_check_timer); + del_timer_sync(&mhi_pdev->health_check_timer); cancel_work_sync(&mhi_pdev->recovery_work); if (test_and_clear_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status)) {
This driver's remove path calls del_timer(). However, that function does not wait until the timer handler finishes. This means that the timer handler may still be running after the driver's remove function has finished, which would result in a use-after-free. Fix by calling del_timer_sync(), which makes sure the timer handler has finished, and unable to re-schedule itself. Fixes: 8562d4fe34a3 ("mhi: pci_generic: Add health-check") Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> --- drivers/bus/mhi/pci_generic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)