PCI/pwrctrl: Check the device node exist before device removal

[PATCH] PCI/pwrctrl: Check the device node exist before device removal

Posted by Saurabh Sengar 1 year, 2 months ago

There can be scenarios where device node is NULL, in such cases
of_node_clear_flag accessing the _flags object will cause a NULL
pointer dereference.

Add a check for NULL device node to fix this.

[  226.227601] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000c0
[  226.330031] pc : pci_stop_bus_device+0xe4/0x178
[  226.333117] lr : pci_stop_bus_device+0xd4/0x178
[  226.389703] Call trace:
[  226.391463]  pci_stop_bus_device+0xe4/0x178 (P)
[  226.394579]  pci_stop_bus_device+0xd4/0x178 (L)
[  226.397691]  pci_stop_and_remove_bus_device_locked+0x2c/0x58
[  226.401717]  remove_store+0xac/0xc8
[  226.404359]  dev_attr_store+0x24/0x48
[  226.406929]  sysfs_kf_write+0x50/0x70
[  226.409553]  kernfs_fop_write_iter+0x144/0x1e0
[  226.412682]  vfs_write+0x250/0x3c0
[  226.415003]  ksys_write+0x7c/0x120
[  226.417827]  __arm64_sys_write+0x28/0x40
[  226.420828]  invoke_syscall+0x74/0x108
[  226.423681]  el0_svc_common.constprop.0+0x4c/0x100
[  226.427205]  do_el0_svc+0x28/0x40
[  226.429748]  el0_svc+0x40/0x148
[  226.432295]  el0t_64_sync_handler+0x114/0x140
[  226.435528]  el0t_64_sync+0x1b8/0x1c0

Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Cc: Krishna chaitanya chundru <quic_krichai@quicinc.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Krzysztof Wilczyński <kwilczynski@kernel.org>
Fixes: 681725afb6b9 ("PCI/pwrctl: Remove pwrctl device without iterating over all children of pwrctl parent")
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
 drivers/pci/remove.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 963b8d2855c1..474ec2453e4b 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -21,6 +21,9 @@ static void pci_pwrctrl_unregister(struct device *dev)
 {
 	struct platform_device *pdev;
 
+	if (!dev_of_node(dev))
+		return;
+
 	pdev = of_find_device_by_node(dev_of_node(dev));
 	if (!pdev)
 		return;
-- 
2.43.0

Re: [PATCH] PCI/pwrctrl: Check the device node exist before device removal

Posted by Manivannan Sadhasivam 1 year, 2 months ago

On Fri, Nov 29, 2024 at 06:30:21AM -0800, Saurabh Sengar wrote:
> There can be scenarios where device node is NULL, in such cases
> of_node_clear_flag accessing the _flags object will cause a NULL
> pointer dereference.
> 
> Add a check for NULL device node to fix this.
> 
> [  226.227601] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000c0
> [  226.330031] pc : pci_stop_bus_device+0xe4/0x178
> [  226.333117] lr : pci_stop_bus_device+0xd4/0x178
> [  226.389703] Call trace:
> [  226.391463]  pci_stop_bus_device+0xe4/0x178 (P)
> [  226.394579]  pci_stop_bus_device+0xd4/0x178 (L)
> [  226.397691]  pci_stop_and_remove_bus_device_locked+0x2c/0x58
> [  226.401717]  remove_store+0xac/0xc8
> [  226.404359]  dev_attr_store+0x24/0x48
> [  226.406929]  sysfs_kf_write+0x50/0x70
> [  226.409553]  kernfs_fop_write_iter+0x144/0x1e0
> [  226.412682]  vfs_write+0x250/0x3c0
> [  226.415003]  ksys_write+0x7c/0x120
> [  226.417827]  __arm64_sys_write+0x28/0x40
> [  226.420828]  invoke_syscall+0x74/0x108
> [  226.423681]  el0_svc_common.constprop.0+0x4c/0x100
> [  226.427205]  do_el0_svc+0x28/0x40
> [  226.429748]  el0_svc+0x40/0x148
> [  226.432295]  el0t_64_sync_handler+0x114/0x140
> [  226.435528]  el0t_64_sync+0x1b8/0x1c0
> 
> Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Cc: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Krzysztof Wilczyński <kwilczynski@kernel.org>
> Fixes: 681725afb6b9 ("PCI/pwrctl: Remove pwrctl device without iterating over all children of pwrctl parent")
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>

Thanks for the fix! There was already a patch submitted to fix the same issue:
https://lore.kernel.org/linux-pci/20241126210443.4052876-1-briannorris@chromium.org/

- Mani

> ---
>  drivers/pci/remove.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 963b8d2855c1..474ec2453e4b 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -21,6 +21,9 @@ static void pci_pwrctrl_unregister(struct device *dev)
>  {
>  	struct platform_device *pdev;
>  
> +	if (!dev_of_node(dev))
> +		return;
> +
>  	pdev = of_find_device_by_node(dev_of_node(dev));
>  	if (!pdev)
>  		return;
> -- 
> 2.43.0
> 

-- 
மணிவண்ணன் சதாசிவம்