2024-05-31 11:19:27

by Nam Cao

[permalink] [raw]
Subject: [PATCH 0/2] PCI: of_property: Gracefully handle bridges without secondary bus

Hi,

Some functions in of_property.c assume that bridges have subordinate bus.
This may not be true if we run out of bus number.

This series adds safety check in case bridges have no subordinate bus.

Nam Cao (2):
PCI: of_property: Fix NULL pointer defererence in
of_pci_prop_bus_range()
PCI: of_property: Fix NULL pointer defererence in
of_pci_prop_intr_map()

drivers/pci/of_property.c | 6 ++++++
1 file changed, 6 insertions(+)

--
2.39.2



2024-05-31 11:19:27

by Nam Cao

[permalink] [raw]
Subject: [PATCH 1/2] PCI: of_property: Fix NULL pointer defererence in of_pci_prop_bus_range()

The subordinate pointer can be null if we are out of bus number. The
function of_pci_prop_bus_range() deferences this pointer without checking
and may crashes the kernel.

This crash can be reproduced by starting a QEMU instance:
qemu-system-riscv64 -machine virt \
-kernel ../build-pci-riscv/arch/riscv/boot/Image \
-append "console=ttyS0 root=/dev/vda" \
-nographic \
-drive "file=root.img,format=raw,id=hd0" \
-device virtio-blk-device,drive=hd0 \
-device pcie-root-port,bus=pcie.0,slot=1,id=rp1,bus-reserve=0 \
-device pcie-pci-bridge,id=br1,bus=rp1

Then hot-add a bridge with
device_add pci-bridge,id=br2,bus=br1,chassis_nr=1,addr=1

Then the kernel crashes:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000088
[snip]
[<ffffffff804db234>] of_pci_add_properties+0x34c/0x3c6
[<ffffffff804c8228>] of_pci_make_dev_node+0xb6/0x116
[<ffffffff804a6b72>] pci_bus_add_device+0xa8/0xaa
[<ffffffff804a6ba4>] pci_bus_add_devices+0x30/0x6a
[<ffffffff804d3b5c>] shpchp_configure_device+0xa0/0x112
[<ffffffff804d2b3a>] board_added+0xce/0x354
[<ffffffff804d2e9a>] shpchp_enable_slot+0xda/0x30c
[<ffffffff804d336c>] shpchp_pushbutton_thread+0x84/0xa0

NULL check this pointer first before proceeding.

Fixes: 407d1a51921e ("PCI: Create device tree node for bridge")
Signed-off-by: Nam Cao <[email protected]>
Cc: [email protected]
---
drivers/pci/of_property.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index c2c7334152bc..5fb516807ba2 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -91,6 +91,9 @@ static int of_pci_prop_bus_range(struct pci_dev *pdev,
struct of_changeset *ocs,
struct device_node *np)
{
+ if (!pdev->subordinate)
+ return 0;
+
u32 bus_range[] = { pdev->subordinate->busn_res.start,
pdev->subordinate->busn_res.end };

--
2.39.2


2024-05-31 11:19:37

by Nam Cao

[permalink] [raw]
Subject: [PATCH 2/2] PCI: of_property: Fix NULL pointer defererence in of_pci_prop_intr_map()

The subordinate pointer can be null if we are out of bus number. The
function of_pci_prop_intr_map() deferences this pointer without checking
and may crashes the kernel.

This crash can be reproduced by starting a QEMU instance:
qemu-system-riscv64 -machine virt \
-kernel ../build-pci-riscv/arch/riscv/boot/Image \
-append "console=ttyS0 root=/dev/vda" \
-nographic \
-drive "file=root.img,format=raw,id=hd0" \
-device virtio-blk-device,drive=hd0 \
-device pcie-root-port,bus=pcie.0,slot=1,id=rp1,bus-reserve=0 \
-device pcie-pci-bridge,id=br1,bus=rp1

Then hot-add a bridge with
device_add pci-bridge,id=br2,bus=br1,chassis_nr=1,addr=1

Then the kernel crashes:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000028
[snip]
[<ffffffff804dac82>] of_pci_prop_intr_map+0x104/0x362
[<ffffffff804db262>] of_pci_add_properties+0x382/0x3ca
[<ffffffff804c8228>] of_pci_make_dev_node+0xb6/0x116
[<ffffffff804a6b72>] pci_bus_add_device+0xa8/0xaa
[<ffffffff804a6ba4>] pci_bus_add_devices+0x30/0x6a
[<ffffffff804d3b5c>] shpchp_configure_device+0xa0/0x112
[<ffffffff804d2b3a>] board_added+0xce/0x354
[<ffffffff804d2e9a>] shpchp_enable_slot+0xda/0x30c
[<ffffffff804d336c>] shpchp_pushbutton_thread+0x84/0xa0

NULL check this pointer first before proceeding.

Fixes: 407d1a51921e ("PCI: Create device tree node for bridge")
Signed-off-by: Nam Cao <[email protected]>
Cc: [email protected]
---
drivers/pci/of_property.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index 5fb516807ba2..c405978a0b7e 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -199,6 +199,9 @@ static int of_pci_prop_intr_map(struct pci_dev *pdev, struct of_changeset *ocs,
int ret;
u8 pin;

+ if (!pdev->subordinate)
+ return 0;
+
pnode = pci_device_to_OF_node(pdev->bus->self);
if (!pnode)
pnode = pci_bus_to_OF_node(pdev->bus);
--
2.39.2