All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL] Fix CVE-2011-1751.
@ 2011-05-19  9:13 Gerd Hoffmann
  2011-05-19  9:13 ` [Qemu-devel] [PATCH] Ignore pci unplug requests for unpluggable devices (CVE-2011-1751) Gerd Hoffmann
  0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2011-05-19  9:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

  Hi folks,

Urgent security fix.  Details are in the commit message.

please pull,
  Gerd

Gerd Hoffmann (1):
  Ignore pci unplug requests for unpluggable devices (CVE-2011-1751)

 hw/acpi_piix4.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

The following changes since commit 96d19bcbf5f679bbaaeab001b572c367fbfb2b03:

  ahci: Unbreak bar registration (2011-05-16 10:15:47 -0500)

are available in the git repository at:
  git://git.kraxel.org/qemu CVE-2011-1751

Gerd Hoffmann (1):
      Ignore pci unplug requests for unpluggable devices (CVE-2011-1751)

 hw/acpi_piix4.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH] Ignore pci unplug requests for unpluggable devices (CVE-2011-1751)
  2011-05-19  9:13 [Qemu-devel] [PULL] Fix CVE-2011-1751 Gerd Hoffmann
@ 2011-05-19  9:13 ` Gerd Hoffmann
  2011-05-19 10:00   ` Markus Armbruster
  0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2011-05-19  9:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch makes qemu ignore unplug requests from the guest for pci
devices which are tagged as non-hotpluggable.  Trouble spot is the
piix4 chipset with the ISA bridge.  Requests to unplug that one will
make it go away together with all ISA bus devices, which are not
prepared to be unplugged and thus don't cleanup, leaving active
qemu timers behind in free'ed memory.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/acpi_piix4.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 96f5222..6c908ff 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -471,11 +471,13 @@ static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
     BusState *bus = opaque;
     DeviceState *qdev, *next;
     PCIDevice *dev;
+    PCIDeviceInfo *info;
     int slot = ffs(val) - 1;
 
     QLIST_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
         dev = DO_UPCAST(PCIDevice, qdev, qdev);
-        if (PCI_SLOT(dev->devfn) == slot) {
+        info = container_of(qdev->info, PCIDeviceInfo, qdev);
+        if (PCI_SLOT(dev->devfn) == slot && !info->no_hotplug) {
             qdev_free(qdev);
         }
     }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] Ignore pci unplug requests for unpluggable devices (CVE-2011-1751)
  2011-05-19  9:13 ` [Qemu-devel] [PATCH] Ignore pci unplug requests for unpluggable devices (CVE-2011-1751) Gerd Hoffmann
@ 2011-05-19 10:00   ` Markus Armbruster
  2011-05-19 11:12     ` Gerd Hoffmann
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2011-05-19 10:00 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

Gerd Hoffmann <kraxel@redhat.com> writes:

> This patch makes qemu ignore unplug requests from the guest for pci
> devices which are tagged as non-hotpluggable.  Trouble spot is the
> piix4 chipset with the ISA bridge.  Requests to unplug that one will
> make it go away together with all ISA bus devices, which are not
> prepared to be unplugged and thus don't cleanup, leaving active
> qemu timers behind in free'ed memory.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/acpi_piix4.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 96f5222..6c908ff 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -471,11 +471,13 @@ static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
>      BusState *bus = opaque;
>      DeviceState *qdev, *next;
>      PCIDevice *dev;
> +    PCIDeviceInfo *info;
>      int slot = ffs(val) - 1;
>  
>      QLIST_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
>          dev = DO_UPCAST(PCIDevice, qdev, qdev);
> -        if (PCI_SLOT(dev->devfn) == slot) {
> +        info = container_of(qdev->info, PCIDeviceInfo, qdev);
> +        if (PCI_SLOT(dev->devfn) == slot && !info->no_hotplug) {
>              qdev_free(qdev);
>          }
>      }

Looks good, but what about pcie_cap_slot_hotplug()?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] Ignore pci unplug requests for unpluggable devices (CVE-2011-1751)
  2011-05-19 10:00   ` Markus Armbruster
@ 2011-05-19 11:12     ` Gerd Hoffmann
  2011-05-19 11:23       ` Markus Armbruster
  0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2011-05-19 11:12 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel

   Hi,

>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>> index 96f5222..6c908ff 100644
>> --- a/hw/acpi_piix4.c
>> +++ b/hw/acpi_piix4.c
>> @@ -471,11 +471,13 @@ static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
>>       BusState *bus = opaque;
>>       DeviceState *qdev, *next;
>>       PCIDevice *dev;
>> +    PCIDeviceInfo *info;
>>       int slot = ffs(val) - 1;
>>
>>       QLIST_FOREACH_SAFE(qdev,&bus->children, sibling, next) {
>>           dev = DO_UPCAST(PCIDevice, qdev, qdev);
>> -        if (PCI_SLOT(dev->devfn) == slot) {
>> +        info = container_of(qdev->info, PCIDeviceInfo, qdev);
>> +        if (PCI_SLOT(dev->devfn) == slot&&  !info->no_hotplug) {
>>               qdev_free(qdev);
>>           }
>>       }
>
> Looks good, but what about pcie_cap_slot_hotplug()?

Dunno, didn't look at q35 yet.  I'd expect the root bus isn't 
hot-pluggable, so the guest wouldn't be able to rip out any essential 
chipset devices.  But having someone more familier with pcie + q35 
double-check would be good ...

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] Ignore pci unplug requests for unpluggable devices (CVE-2011-1751)
  2011-05-19 11:12     ` Gerd Hoffmann
@ 2011-05-19 11:23       ` Markus Armbruster
  2011-05-19 11:52         ` Isaku Yamahata
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2011-05-19 11:23 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Isaku Yamahata, qemu-devel

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
> Markus Armbruster <armbru@redhat.com> writes:
>
>> Gerd Hoffmann <kraxel@redhat.com> writes:
>>
>>> This patch makes qemu ignore unplug requests from the guest for pci
>>> devices which are tagged as non-hotpluggable.  Trouble spot is the
>>> piix4 chipset with the ISA bridge.  Requests to unplug that one will
>>> make it go away together with all ISA bus devices, which are not
>>> prepared to be unplugged and thus don't cleanup, leaving active
>>> qemu timers behind in free'ed memory.
>>>
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> ---
>>>  hw/acpi_piix4.c |    4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>>> index 96f5222..6c908ff 100644
>>> --- a/hw/acpi_piix4.c
>>> +++ b/hw/acpi_piix4.c
>>> @@ -471,11 +471,13 @@ static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
>>>       BusState *bus = opaque;
>>>       DeviceState *qdev, *next;
>>>       PCIDevice *dev;
>>> +    PCIDeviceInfo *info;
>>>       int slot = ffs(val) - 1;
>>>
>>>       QLIST_FOREACH_SAFE(qdev,&bus->children, sibling, next) {
>>>           dev = DO_UPCAST(PCIDevice, qdev, qdev);
>>> -        if (PCI_SLOT(dev->devfn) == slot) {
>>> +        info = container_of(qdev->info, PCIDeviceInfo, qdev);
>>> +        if (PCI_SLOT(dev->devfn) == slot&&  !info->no_hotplug) {
>>>               qdev_free(qdev);
>>>           }
>>>       }
>>
>> Looks good, but what about pcie_cap_slot_hotplug()?
>
> Dunno, didn't look at q35 yet.  I'd expect the root bus isn't
> hot-pluggable, so the guest wouldn't be able to rip out any essential
> chipset devices.  But having someone more familier with pcie + q35
> double-check would be good ...

I guess that would be Isaku Yamahata (cc'ed).

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] Ignore pci unplug requests for unpluggable devices (CVE-2011-1751)
  2011-05-19 11:23       ` Markus Armbruster
@ 2011-05-19 11:52         ` Isaku Yamahata
  0 siblings, 0 replies; 6+ messages in thread
From: Isaku Yamahata @ 2011-05-19 11:52 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Gerd Hoffmann, qemu-devel

On Thu, May 19, 2011 at 01:23:18PM +0200, Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> >   Hi,
> >
> > Markus Armbruster <armbru@redhat.com> writes:
> >
> >> Gerd Hoffmann <kraxel@redhat.com> writes:
> >>
> >>> This patch makes qemu ignore unplug requests from the guest for pci
> >>> devices which are tagged as non-hotpluggable.  Trouble spot is the
> >>> piix4 chipset with the ISA bridge.  Requests to unplug that one will
> >>> make it go away together with all ISA bus devices, which are not
> >>> prepared to be unplugged and thus don't cleanup, leaving active
> >>> qemu timers behind in free'ed memory.
> >>>
> >>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> >>> ---
> >>>  hw/acpi_piix4.c |    4 +++-
> >>>  1 files changed, 3 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> >>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> >>> index 96f5222..6c908ff 100644
> >>> --- a/hw/acpi_piix4.c
> >>> +++ b/hw/acpi_piix4.c
> >>> @@ -471,11 +471,13 @@ static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
> >>>       BusState *bus = opaque;
> >>>       DeviceState *qdev, *next;
> >>>       PCIDevice *dev;
> >>> +    PCIDeviceInfo *info;
> >>>       int slot = ffs(val) - 1;
> >>>
> >>>       QLIST_FOREACH_SAFE(qdev,&bus->children, sibling, next) {
> >>>           dev = DO_UPCAST(PCIDevice, qdev, qdev);
> >>> -        if (PCI_SLOT(dev->devfn) == slot) {
> >>> +        info = container_of(qdev->info, PCIDeviceInfo, qdev);
> >>> +        if (PCI_SLOT(dev->devfn) == slot&&  !info->no_hotplug) {
> >>>               qdev_free(qdev);
> >>>           }
> >>>       }
> >>
> >> Looks good, but what about pcie_cap_slot_hotplug()?
> >
> > Dunno, didn't look at q35 yet.  I'd expect the root bus isn't
> > hot-pluggable, so the guest wouldn't be able to rip out any essential
> > chipset devices.  But having someone more familier with pcie + q35
> > double-check would be good ...
> 
> I guess that would be Isaku Yamahata (cc'ed).

The root pci bus of q35 isn't hot pluggable. The pcie bus with
the hotplug capability means that the slot in the bus is always
hot pluggable. So pcie_cap_slot_hotplug() doesn't need to check
no_hotplug.

If some sort of check is wanted, the check should be done at
the device initialization, I think.
Populating a non-hotpluggable devince in hot pluggable slot doesn't make
sense.

thanks,
-- 
yamahata

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-05-19 11:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-19  9:13 [Qemu-devel] [PULL] Fix CVE-2011-1751 Gerd Hoffmann
2011-05-19  9:13 ` [Qemu-devel] [PATCH] Ignore pci unplug requests for unpluggable devices (CVE-2011-1751) Gerd Hoffmann
2011-05-19 10:00   ` Markus Armbruster
2011-05-19 11:12     ` Gerd Hoffmann
2011-05-19 11:23       ` Markus Armbruster
2011-05-19 11:52         ` Isaku Yamahata

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.