linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove()
@ 2021-04-13 16:03 Wei Yongjun
  2021-04-16 15:38 ` Loic Poulain
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Wei Yongjun @ 2021-04-13 16:03 UTC (permalink / raw)
  To: weiyongjun1, Loic Poulain, Manivannan Sadhasivam, Hemant Kumar
  Cc: linux-arm-msm, kernel-janitors, Hulk Robot

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(-)

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)) {


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove()
  2021-04-13 16:03 [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove() Wei Yongjun
@ 2021-04-16 15:38 ` Loic Poulain
  2021-05-03 22:41 ` Hemant Kumar
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Loic Poulain @ 2021-04-16 15:38 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: Manivannan Sadhasivam, Hemant Kumar, linux-arm-msm,
	kernel-janitors, Hulk Robot

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove()
  2021-04-13 16:03 [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove() Wei Yongjun
  2021-04-16 15:38 ` Loic Poulain
@ 2021-05-03 22:41 ` Hemant Kumar
  2021-05-21 12:17 ` Manivannan Sadhasivam
  2021-05-21 17:37 ` Manivannan Sadhasivam
  3 siblings, 0 replies; 6+ messages in thread
From: Hemant Kumar @ 2021-05-03 22:41 UTC (permalink / raw)
  To: Wei Yongjun, Loic Poulain, Manivannan Sadhasivam
  Cc: linux-arm-msm, kernel-janitors, Hulk Robot



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>

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove()
  2021-04-13 16:03 [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove() Wei Yongjun
  2021-04-16 15:38 ` Loic Poulain
  2021-05-03 22:41 ` Hemant Kumar
@ 2021-05-21 12:17 ` Manivannan Sadhasivam
  2021-05-21 12:19   ` Manivannan Sadhasivam
  2021-05-21 17:37 ` Manivannan Sadhasivam
  3 siblings, 1 reply; 6+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 12:17 UTC (permalink / raw)
  To: Wei Yongjun, Loic Poulain
  Cc: Loic Poulain, Hemant Kumar, linux-arm-msm, kernel-janitors, Hulk Robot

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)) {
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove()
  2021-05-21 12:17 ` Manivannan Sadhasivam
@ 2021-05-21 12:19   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 12:19 UTC (permalink / raw)
  To: Wei Yongjun, Loic Poulain
  Cc: Hemant Kumar, linux-arm-msm, kernel-janitors, Hulk Robot

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)) {
> > 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove()
  2021-04-13 16:03 [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove() Wei Yongjun
                   ` (2 preceding siblings ...)
  2021-05-21 12:17 ` Manivannan Sadhasivam
@ 2021-05-21 17:37 ` Manivannan Sadhasivam
  3 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 17:37 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: Loic Poulain, Hemant Kumar, linux-arm-msm, kernel-janitors, Hulk Robot

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)) {
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-05-21 17:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 16:03 [PATCH -next] bus: mhi: pci_generic: Fix possible use-after-free in mhi_pci_remove() Wei Yongjun
2021-04-16 15:38 ` Loic Poulain
2021-05-03 22:41 ` Hemant Kumar
2021-05-21 12:17 ` Manivannan Sadhasivam
2021-05-21 12:19   ` Manivannan Sadhasivam
2021-05-21 17:37 ` Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).