On Sat, Aug 21, 2021 at 08:35:35PM +0530, Ani Sinha wrote:
> Bsel property of the pci bus indicates whether the bus supports acpi hotplug.
> We need to validate the presence of this property before performing any
> hotplug
> related callback operations. Currently validation of the existence of this
> property was absent from acpi_pcihp_device_unplug_cb() function but is present
> in other hotplug/unplug callback functions. Hence, this change adds the
> missing
> check for the above function.
>
> Signed-off-by: Ani Sinha <a...@anisinha.ca>
I queued this but I have a general question:
are all these errors logged with LOG_GUEST_ERROR?
Because if not we have a security problem.
I also note that bsel is an internal property,
I am not sure we should be printing this to users,
it might just confuse them.
Same question for all the other places validating bsel.
> ---
> hw/acpi/pcihp.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index 0fd0c1d811..9982815a87 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -372,9 +372,15 @@ void acpi_pcihp_device_unplug_cb(HotplugHandler
> *hotplug_dev, AcpiPciHpState *s,
> DeviceState *dev, Error **errp)
> {
> PCIDevice *pdev = PCI_DEVICE(dev);
> + int bsel = acpi_pcihp_get_bsel(pci_get_bus(pdev));
> +
> + trace_acpi_pci_unplug(PCI_SLOT(pdev->devfn), bsel);
>
> - trace_acpi_pci_unplug(PCI_SLOT(pdev->devfn),
> - acpi_pcihp_get_bsel(pci_get_bus(pdev)));
> + if (bsel < 0) {
> + error_setg(errp, "Unsupported bus. Bus doesn't have property '"
> + ACPI_PCIHP_PROP_BSEL "' set");
> + return;
> + }
>
> /*
> * clean up acpi-index so it could reused by another device
> --
> 2.25.1
