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

[Qemu-devel] [PATCH] hw/pci-bridge: fix QEMU crash because of pcie-root-port

Posted by Marcel Apfelbaum 6 years, 3 months ago

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 <marcel@redhat.com>
---
 hw/pci-bridge/gen_pcie_root_port.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
index ad4e6aa7ff..0e2f2e8bf1 100644
--- a/hw/pci-bridge/gen_pcie_root_port.c
+++ b/hw/pci-bridge/gen_pcie_root_port.c
@@ -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,
-- 
2.13.5

Re: [Qemu-devel] [PATCH] hw/pci-bridge: fix QEMU crash because of pcie-root-port

Posted by Laszlo Ersek 6 years, 3 months ago

On 01/10/18 20:09, Marcel Apfelbaum wrote:
> 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 <marcel@redhat.com>
> ---
>  hw/pci-bridge/gen_pcie_root_port.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

In your opinion, can we add:

Cc: qemu-stable@nongnu.org
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?

> 
> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> index ad4e6aa7ff..0e2f2e8bf1 100644
> --- a/hw/pci-bridge/gen_pcie_root_port.c
> +++ b/hw/pci-bridge/gen_pcie_root_port.c
> @@ -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,
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

Re: [Qemu-devel] [PATCH] hw/pci-bridge: fix QEMU crash because of pcie-root-port

Posted by Marcel Apfelbaum 6 years, 3 months ago

On 15/01/2018 17:54, Laszlo Ersek wrote:
> On 01/10/18 20:09, Marcel Apfelbaum wrote:
>> 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 <marcel@redhat.com>
>> ---
>>   hw/pci-bridge/gen_pcie_root_port.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> In your opinion, can we add:
> 
> Cc: qemu-stable@nongnu.org
> 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?
> 

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?

>>
>> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
>> index ad4e6aa7ff..0e2f2e8bf1 100644
>> --- a/hw/pci-bridge/gen_pcie_root_port.c
>> +++ b/hw/pci-bridge/gen_pcie_root_port.c
>> @@ -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,
>>
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 

Appreciated!

Thanks,
Marcel

> Thanks
> Laszlo
>

Re: [Qemu-devel] [PATCH] hw/pci-bridge: fix QEMU crash because of pcie-root-port

Posted by Laszlo Ersek 6 years, 3 months ago

On 01/15/18 17:31, Marcel Apfelbaum wrote:
> On 15/01/2018 17:54, Laszlo Ersek wrote:
>> On 01/10/18 20:09, Marcel Apfelbaum wrote:
>>> 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 <marcel@redhat.com>
>>> ---
>>>   hw/pci-bridge/gen_pcie_root_port.c | 7 ++++++-
>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> In your opinion, can we add:
>>
>> Cc: qemu-stable@nongnu.org
>> 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?
>>
> 
> 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

> 
>>>
>>> diff --git a/hw/pci-bridge/gen_pcie_root_port.c
>>> b/hw/pci-bridge/gen_pcie_root_port.c
>>> index ad4e6aa7ff..0e2f2e8bf1 100644
>>> --- a/hw/pci-bridge/gen_pcie_root_port.c
>>> +++ b/hw/pci-bridge/gen_pcie_root_port.c
>>> @@ -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,
>>>
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
> 
> Appreciated!
> 
> Thanks,
> Marcel
> 
>> Thanks
>> Laszlo
>>
>