hw/pci-bridge: fix QEMU crash because of pcie-root-port

PATCH In review Marcel Apfelbaum 2 participants 4 messages Jan 10, 2018 – Jan 16, 2018

Share & tools

Marcel Apfelbaum

author

2018-01-10 19:09 UTC #0 lore raw

PATCH 1/1 hw/pci-bridge: fix QEMU crash because of pcie-root-port

If we try to use more pcie_root_ports then available slots and an IO hint is passed to the port, QEMU crashes because we try to init the "IO hint" capability even if the device is not created.
Fix it by checking for error before adding the capability, so QEMU can fail gracefully.

Signed-off-by Marcel Apfelbaum <[email protected]>

---
 hw/pci-bridge/gen_pcie_root_port.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

hw/pci-bridge/gen_pcie_root_port.c +6 -1

index ad4e6aa7ff..0e2f2e8bf1 100644
@@ -74,8 +74,13 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
     PCIDevice *d = PCI_DEVICE(dev);
     GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d);
     PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
+    Error *local_err = NULL;
 
-    rpc->parent_realize(dev, errp);
+    rpc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     int rc = pci_bridge_qemu_reserve_cap_init(d, 0, grp->bus_reserve,
             grp->io_reserve, grp->mem_reserve, grp->pref32_reserve,

Signature · 1 line

2.13.5

Laszlo Ersek

Reviewed-by on v1 1/1

2018-01-15 15:54 UTC #1 lore raw

Quoted 11 lines · Marcel Apfelbaum

If we try to use more pcie_root_ports then available slots and an IO hint is passed to the port, QEMU crashes because we try to init the "IO hint" capability even if the device is not created.
Fix it by checking for error before adding the capability, so QEMU can fail gracefully.
Signed-off-by Marcel Apfelbaum <[email protected]>
---
 hw/pci-bridge/gen_pcie_root_port.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

In your opinion, can we add:

Cc [email protected]

Fixes 226263fb5cdaa4a4a95f1680fabbc9dd2123fd67

(Not sure if a stable branch is already open for 2.11. Commit 226263fb5cdaa appeared in 2.11.)

Ah, I'm silly. You CC'd stable up-front. :)

So, what about the Fixes tag?

Quoted 11 lines

hw/pci-bridge/gen_pcie_root_port.c +1 -0

index ad4e6aa7ff..0e2f2e8bf1 100644
@@ -74,8 +74,13 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
     PCIDevice *d = PCI_DEVICE(dev);
     GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d);
     PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
+    Error *local_err = NULL;

hw/pci-bridge/gen_pcie_root_port.c static void gen_rp_realize(DeviceState *dev, Error **errp) · line 86

Quoted 10 lines

hw/pci-bridge/gen_pcie_root_port.c +5 -1

 
-    rpc->parent_realize(dev, errp);
+    rpc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     int rc = pci_bridge_qemu_reserve_cap_init(d, 0, grp->bus_reserve,
             grp->io_reserve, grp->mem_reserve, grp->pref32_reserve,

Reviewed-by Laszlo Ersek <[email protected]>

Thanks
Laszlo

Marcel Apfelbaum

author

2018-01-15 16:31 UTC #2 lore raw

Quoted 18 lines · Laszlo Ersek

Quoted 11 lines · Marcel Apfelbaum
If we try to use more pcie_root_ports then available slots and an IO hint is passed to the port, QEMU crashes because we try to init the "IO hint" capability even if the device is not created.
Fix it by checking for error before adding the capability, so QEMU can fail gracefully.
Signed-off-by Marcel Apfelbaum <[email protected]>
---
  hw/pci-bridge/gen_pcie_root_port.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)
In your opinion, can we add:
Cc [email protected]
Fixes 226263fb5cdaa4a4a95f1680fabbc9dd2123fd67

Quoted 9 lines

?
(Not sure if a stable branch is already open for 2.11. Commit 226263fb5cdaa appeared in 2.11.)
Ah, I'm silly. You CC'd stable up-front. :)
So, what about the Fixes tag?

For sure, I wasn't aware of the "Fixes" tag. I'll be sure to use it in the future.

Do I need to resend, or Michael can pick the tag?

Quoted 18 lines

hw/pci-bridge/gen_pcie_root_port.c +6 -1

index ad4e6aa7ff..0e2f2e8bf1 100644
@@ -74,8 +74,13 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
      PCIDevice *d = PCI_DEVICE(dev);
      GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d);
      PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
+    Error *local_err = NULL;
 
-    rpc->parent_realize(dev, errp);
+    rpc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }

Quoted 6 lines

hw/pci-bridge/gen_pcie_root_port.c static void gen_rp_realize(DeviceState *dev, Error **errp) · line 86
Quoted 3 lines
hw/pci-bridge/gen_pcie_root_port.c +0 -0
      int rc = pci_bridge_qemu_reserve_cap_init(d, 0, grp->bus_reserve,
              grp->io_reserve, grp->mem_reserve, grp->pref32_reserve,
Reviewed-by Laszlo Ersek <[email protected]>

Appreciated!

Thanks,
Marcel

Quoted 3 lines

Thanks
Laszlo

Laszlo Ersek

2018-01-16 19:05 UTC #3 lore raw

Quoted 26 lines · Marcel Apfelbaum

Quoted 25 lines · Laszlo Ersek
Quoted 11 lines · Marcel Apfelbaum
If we try to use more pcie_root_ports then available slots and an IO hint is passed to the port, QEMU crashes because we try to init the "IO hint" capability even if the device is not created.
Fix it by checking for error before adding the capability, so QEMU can fail gracefully.
Signed-off-by Marcel Apfelbaum <[email protected]>
---
hw/pci-bridge/gen_pcie_root_port.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)
In your opinion, can we add:
Cc [email protected]
Fixes 226263fb5cdaa4a4a95f1680fabbc9dd2123fd67
?
(Not sure if a stable branch is already open for 2.11. Commit 226263fb5cdaa appeared in 2.11.)
Ah, I'm silly. You CC'd stable up-front. :)

Quoted 7 lines

Quoted 2 lines
So, what about the Fixes tag?
For sure, I wasn't aware of the "Fixes" tag. I'll be sure to use it in the future.
Do I need to resend, or Michael can pick the tag?

I think Michael can add the tag the same as my R-b.

Thanks,
Laszlo

Quoted 36 lines

Quoted 17 lines

hw/pci-bridge/gen_pcie_root_port.c +4 -0

index ad4e6aa7ff..0e2f2e8bf1 100644
@@ -74,8 +74,13 @@ static void gen_rp_realize(DeviceState *dev, Error
 **errp)
       PCIDevice *d = PCI_DEVICE(dev);
       GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d);
       PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
+    Error *local_err = NULL;
   -    rpc->parent_realize(dev, errp);
+    rpc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);

Quoted 9 lines

Quoted 6 lines
hw/pci-bridge/gen_pcie_root_port.c +2 -0
+        return;
+    }
         int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
 grp->bus_reserve,
grp->io_reserve, grp->mem_reserve, grp->pref32_reserve,
Reviewed-by Laszlo Ersek <[email protected]>

Appreciated!

Thanks,
Marcel

Quoted 3 lines

Thanks
Laszlo