* Re: [PATCH] PCI/pwrctrl: Cancel outstanding rescan work when unregistering
2025-04-09 18:53 [PATCH] PCI/pwrctrl: Cancel outstanding rescan work when unregistering Brian Norris
@ 2025-04-23 17:44 ` Bjorn Helgaas
2025-04-23 17:57 ` Manivannan Sadhasivam
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2025-04-23 17:44 UTC (permalink / raw)
To: Brian Norris
Cc: Bartosz Golaszewski, Bjorn Helgaas, linux-kernel, linux-pci,
Manivannan Sadhasivam, Brian Norris, Konrad Dybcio,
Bartosz Golaszewski
On Wed, Apr 09, 2025 at 11:53:13AM -0700, Brian Norris wrote:
> From: Brian Norris <briannorris@google.com>
>
> It's possible to trigger use-after-free here by:
> (a) forcing rescan_work_func() to take a long time and
> (b) utilizing a pwrctrl driver that may be unloaded for some reason.
>
> I'm unlucky to trigger both of these in development. It's likely much
> more difficult to hit this in practice.
>
> Anyway, we should ensure our work is finished before we allow our data
> structures to be cleaned up.
>
> Fixes: 8f62819aaace ("PCI/pwrctl: Rescan bus on a separate thread")
> Cc: Konrad Dybcio <konradybcio@kernel.org>
> Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Brian Norris <briannorris@google.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
Looking for ack/reviewed-by from Bartosz before doing anything here.
> ---
>
> drivers/pci/pwrctrl/core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 9cc7e2b7f2b5..6bdbfed584d6 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -101,6 +101,8 @@ EXPORT_SYMBOL_GPL(pci_pwrctrl_device_set_ready);
> */
> void pci_pwrctrl_device_unset_ready(struct pci_pwrctrl *pwrctrl)
> {
> + cancel_work_sync(&pwrctrl->work);
> +
> /*
> * We don't have to delete the link here. Typically, this function
> * is only called when the power control device is being detached. If
> --
> 2.49.0.604.gff1f9ca942-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] PCI/pwrctrl: Cancel outstanding rescan work when unregistering
2025-04-09 18:53 [PATCH] PCI/pwrctrl: Cancel outstanding rescan work when unregistering Brian Norris
2025-04-23 17:44 ` Bjorn Helgaas
@ 2025-04-23 17:57 ` Manivannan Sadhasivam
2025-04-23 18:35 ` Bartosz Golaszewski
2025-04-23 20:49 ` Bjorn Helgaas
3 siblings, 0 replies; 5+ messages in thread
From: Manivannan Sadhasivam @ 2025-04-23 17:57 UTC (permalink / raw)
To: Brian Norris
Cc: Bartosz Golaszewski, Bjorn Helgaas, linux-kernel, linux-pci,
Brian Norris, Konrad Dybcio, Bartosz Golaszewski
On Wed, Apr 09, 2025 at 11:53:13AM -0700, Brian Norris wrote:
> From: Brian Norris <briannorris@google.com>
>
> It's possible to trigger use-after-free here by:
> (a) forcing rescan_work_func() to take a long time and
> (b) utilizing a pwrctrl driver that may be unloaded for some reason.
>
> I'm unlucky to trigger both of these in development. It's likely much
> more difficult to hit this in practice.
>
I never envisioned this situation :) But yeah, better be safe than sorry.
> Anyway, we should ensure our work is finished before we allow our data
> structures to be cleaned up.
>
> Fixes: 8f62819aaace ("PCI/pwrctl: Rescan bus on a separate thread")
> Cc: Konrad Dybcio <konradybcio@kernel.org>
> Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Brian Norris <briannorris@google.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
- Mani
> ---
>
> drivers/pci/pwrctrl/core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 9cc7e2b7f2b5..6bdbfed584d6 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -101,6 +101,8 @@ EXPORT_SYMBOL_GPL(pci_pwrctrl_device_set_ready);
> */
> void pci_pwrctrl_device_unset_ready(struct pci_pwrctrl *pwrctrl)
> {
> + cancel_work_sync(&pwrctrl->work);
> +
> /*
> * We don't have to delete the link here. Typically, this function
> * is only called when the power control device is being detached. If
> --
> 2.49.0.604.gff1f9ca942-goog
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] PCI/pwrctrl: Cancel outstanding rescan work when unregistering
2025-04-09 18:53 [PATCH] PCI/pwrctrl: Cancel outstanding rescan work when unregistering Brian Norris
2025-04-23 17:44 ` Bjorn Helgaas
2025-04-23 17:57 ` Manivannan Sadhasivam
@ 2025-04-23 18:35 ` Bartosz Golaszewski
2025-04-23 20:49 ` Bjorn Helgaas
3 siblings, 0 replies; 5+ messages in thread
From: Bartosz Golaszewski @ 2025-04-23 18:35 UTC (permalink / raw)
To: Brian Norris
Cc: Bjorn Helgaas, linux-kernel, linux-pci, Manivannan Sadhasivam,
Brian Norris, Konrad Dybcio, Bartosz Golaszewski
On Wed, Apr 9, 2025 at 8:54 PM Brian Norris <briannorris@chromium.org> wrote:
>
> From: Brian Norris <briannorris@google.com>
>
> It's possible to trigger use-after-free here by:
> (a) forcing rescan_work_func() to take a long time and
> (b) utilizing a pwrctrl driver that may be unloaded for some reason.
>
> I'm unlucky to trigger both of these in development. It's likely much
> more difficult to hit this in practice.
>
> Anyway, we should ensure our work is finished before we allow our data
> structures to be cleaned up.
>
> Fixes: 8f62819aaace ("PCI/pwrctl: Rescan bus on a separate thread")
> Cc: Konrad Dybcio <konradybcio@kernel.org>
> Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Brian Norris <briannorris@google.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>
Sorry, it fell off my radar.
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] PCI/pwrctrl: Cancel outstanding rescan work when unregistering
2025-04-09 18:53 [PATCH] PCI/pwrctrl: Cancel outstanding rescan work when unregistering Brian Norris
` (2 preceding siblings ...)
2025-04-23 18:35 ` Bartosz Golaszewski
@ 2025-04-23 20:49 ` Bjorn Helgaas
3 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2025-04-23 20:49 UTC (permalink / raw)
To: Brian Norris
Cc: Bartosz Golaszewski, Bjorn Helgaas, linux-kernel, linux-pci,
Manivannan Sadhasivam, Brian Norris, Konrad Dybcio,
Bartosz Golaszewski
On Wed, Apr 09, 2025 at 11:53:13AM -0700, Brian Norris wrote:
> From: Brian Norris <briannorris@google.com>
>
> It's possible to trigger use-after-free here by:
> (a) forcing rescan_work_func() to take a long time and
> (b) utilizing a pwrctrl driver that may be unloaded for some reason.
>
> I'm unlucky to trigger both of these in development. It's likely much
> more difficult to hit this in practice.
>
> Anyway, we should ensure our work is finished before we allow our data
> structures to be cleaned up.
>
> Fixes: 8f62819aaace ("PCI/pwrctl: Rescan bus on a separate thread")
> Cc: Konrad Dybcio <konradybcio@kernel.org>
> Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Brian Norris <briannorris@google.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
Applied to pci/pwrctrl for v6.16, thanks!
> ---
>
> drivers/pci/pwrctrl/core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 9cc7e2b7f2b5..6bdbfed584d6 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -101,6 +101,8 @@ EXPORT_SYMBOL_GPL(pci_pwrctrl_device_set_ready);
> */
> void pci_pwrctrl_device_unset_ready(struct pci_pwrctrl *pwrctrl)
> {
> + cancel_work_sync(&pwrctrl->work);
> +
> /*
> * We don't have to delete the link here. Typically, this function
> * is only called when the power control device is being detached. If
> --
> 2.49.0.604.gff1f9ca942-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread