* [Qemu-devel] [PATCH] vfio/pci: Cleanup vfio_early_setup_msix() error path
@ 2015-07-20 16:41 Alex Williamson
2015-07-20 17:30 ` Laszlo Ersek
0 siblings, 1 reply; 2+ messages in thread
From: Alex Williamson @ 2015-07-20 16:41 UTC (permalink / raw)
To: alex.williamson, qemu-devel; +Cc: lersek
With the addition of the Chelsio quirk we have an error path out of
vfio_early_setup_msix() that doesn't free the allocated VFIOMSIXInfo
struct. This doesn't introduce a leak as it still gets freed in the
vfio_put_device() path, but it's complicated and sloppy to rely on
that. Restructure to free the allocated data on error and only link
it into the vdev on success.
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Reported-by: Laszlo Ersek <lersek@redhat.com>
---
hw/vfio/pci.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 70d82d4..8c6127a 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2204,6 +2204,7 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
uint16_t ctrl;
uint32_t table, pba;
int fd = vdev->vbasedev.fd;
+ VFIOMSIXInfo *msix;
pos = pci_find_capability(&vdev->pdev, PCI_CAP_ID_MSIX);
if (!pos) {
@@ -2229,21 +2230,19 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
table = le32_to_cpu(table);
pba = le32_to_cpu(pba);
- vdev->msix = g_malloc0(sizeof(*(vdev->msix)));
- vdev->msix->table_bar = table & PCI_MSIX_FLAGS_BIRMASK;
- vdev->msix->table_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
- vdev->msix->pba_bar = pba & PCI_MSIX_FLAGS_BIRMASK;
- vdev->msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
- vdev->msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
+ msix = g_malloc0(sizeof(*msix));
+ msix->table_bar = table & PCI_MSIX_FLAGS_BIRMASK;
+ msix->table_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
+ msix->pba_bar = pba & PCI_MSIX_FLAGS_BIRMASK;
+ msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
+ msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
/*
* Test the size of the pba_offset variable and catch if it extends outside
* of the specified BAR. If it is the case, we need to apply a hardware
* specific quirk if the device is known or we have a broken configuration.
*/
- if (vdev->msix->pba_offset >=
- vdev->bars[vdev->msix->pba_bar].region.size) {
-
+ if (msix->pba_offset >= vdev->bars[msix->pba_bar].region.size) {
PCIDevice *pdev = &vdev->pdev;
uint16_t vendor = pci_get_word(pdev->config + PCI_VENDOR_ID);
uint16_t device = pci_get_word(pdev->config + PCI_DEVICE_ID);
@@ -2255,18 +2254,18 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
* is 0x1000, so we hard code that here.
*/
if (vendor == PCI_VENDOR_ID_CHELSIO && (device & 0xff00) == 0x5800) {
- vdev->msix->pba_offset = 0x1000;
+ msix->pba_offset = 0x1000;
} else {
error_report("vfio: Hardware reports invalid configuration, "
"MSIX PBA outside of specified BAR");
+ g_free(msix);
return -EINVAL;
}
}
- trace_vfio_early_setup_msix(vdev->vbasedev.name, pos,
- vdev->msix->table_bar,
- vdev->msix->table_offset,
- vdev->msix->entries);
+ trace_vfio_early_setup_msix(vdev->vbasedev.name, pos, msix->table_bar,
+ msix->table_offset, msix->entries);
+ vdev->msix = msix;
return 0;
}
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] vfio/pci: Cleanup vfio_early_setup_msix() error path
2015-07-20 16:41 [Qemu-devel] [PATCH] vfio/pci: Cleanup vfio_early_setup_msix() error path Alex Williamson
@ 2015-07-20 17:30 ` Laszlo Ersek
0 siblings, 0 replies; 2+ messages in thread
From: Laszlo Ersek @ 2015-07-20 17:30 UTC (permalink / raw)
To: Alex Williamson, qemu-devel
On 07/20/15 18:41, Alex Williamson wrote:
> With the addition of the Chelsio quirk we have an error path out of
> vfio_early_setup_msix() that doesn't free the allocated VFIOMSIXInfo
> struct. This doesn't introduce a leak as it still gets freed in the
> vfio_put_device() path, but it's complicated and sloppy to rely on
> that. Restructure to free the allocated data on error and only link
> it into the vdev on success.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> ---
> hw/vfio/pci.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 70d82d4..8c6127a 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2204,6 +2204,7 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
> uint16_t ctrl;
> uint32_t table, pba;
> int fd = vdev->vbasedev.fd;
> + VFIOMSIXInfo *msix;
>
> pos = pci_find_capability(&vdev->pdev, PCI_CAP_ID_MSIX);
> if (!pos) {
> @@ -2229,21 +2230,19 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
> table = le32_to_cpu(table);
> pba = le32_to_cpu(pba);
>
> - vdev->msix = g_malloc0(sizeof(*(vdev->msix)));
> - vdev->msix->table_bar = table & PCI_MSIX_FLAGS_BIRMASK;
> - vdev->msix->table_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
> - vdev->msix->pba_bar = pba & PCI_MSIX_FLAGS_BIRMASK;
> - vdev->msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
> - vdev->msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
> + msix = g_malloc0(sizeof(*msix));
> + msix->table_bar = table & PCI_MSIX_FLAGS_BIRMASK;
> + msix->table_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
> + msix->pba_bar = pba & PCI_MSIX_FLAGS_BIRMASK;
> + msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
> + msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
>
> /*
> * Test the size of the pba_offset variable and catch if it extends outside
> * of the specified BAR. If it is the case, we need to apply a hardware
> * specific quirk if the device is known or we have a broken configuration.
> */
> - if (vdev->msix->pba_offset >=
> - vdev->bars[vdev->msix->pba_bar].region.size) {
> -
> + if (msix->pba_offset >= vdev->bars[msix->pba_bar].region.size) {
> PCIDevice *pdev = &vdev->pdev;
> uint16_t vendor = pci_get_word(pdev->config + PCI_VENDOR_ID);
> uint16_t device = pci_get_word(pdev->config + PCI_DEVICE_ID);
> @@ -2255,18 +2254,18 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
> * is 0x1000, so we hard code that here.
> */
> if (vendor == PCI_VENDOR_ID_CHELSIO && (device & 0xff00) == 0x5800) {
> - vdev->msix->pba_offset = 0x1000;
> + msix->pba_offset = 0x1000;
> } else {
> error_report("vfio: Hardware reports invalid configuration, "
> "MSIX PBA outside of specified BAR");
> + g_free(msix);
> return -EINVAL;
> }
> }
>
> - trace_vfio_early_setup_msix(vdev->vbasedev.name, pos,
> - vdev->msix->table_bar,
> - vdev->msix->table_offset,
> - vdev->msix->entries);
> + trace_vfio_early_setup_msix(vdev->vbasedev.name, pos, msix->table_bar,
> + msix->table_offset, msix->entries);
> + vdev->msix = msix;
>
> return 0;
> }
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-07-20 17:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-20 16:41 [Qemu-devel] [PATCH] vfio/pci: Cleanup vfio_early_setup_msix() error path Alex Williamson
2015-07-20 17:30 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).