| Message ID | 20211213111554.62394-5-konstantin@daynix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gqa-win: get_pci_info: Fix memory leak | expand |
On Mon, Dec 13, 2021 at 3:16 PM Kostiantyn Kostiuk <konstantin@daynix.com> wrote: > > Microsoft suggests this solution in the documentation: > https://docs.microsoft.com/en-us/windows/win32/api/setupapi/nf-setupapi-setupdigetdeviceinterfacedetaila > > Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com> > Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com> Arguably, the next patch should come first, as you introduce extra leaking points here - alternatively, update the commit message. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > qga/commands-win32.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index cef14a8762..6bde5260e8 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -552,10 +552,10 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp) > if (SetupDiEnumDeviceInterfaces(dev_info, &dev_info_data, > &GUID_DEVINTERFACE_DISK, 0, > &dev_iface_data)) { > - while (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data, > - pdev_iface_detail_data, > - size, &size, > - &dev_info_data)) { > + if (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data, > + pdev_iface_detail_data, > + size, &size, > + &dev_info_data)) { > if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) { > pdev_iface_detail_data = g_malloc(size); > pdev_iface_detail_data->cbSize = > @@ -567,6 +567,16 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp) > } > } > > + if (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data, > + pdev_iface_detail_data, > + size, &size, > + &dev_info_data)) { > + // pdev_iface_detail_data already is allocated > + error_setg_win32(errp, GetLastError(), > + "failed to get device interfaces"); > + goto cleanup; > + } > + > dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0, > FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, > NULL); > @@ -597,8 +607,8 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp) > ULONG dev_id_size = 0; > > size = 0; > - while (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data, > - parent_dev_id, size, &size)) { > + if (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data, > + parent_dev_id, size, &size)) { > if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) { > parent_dev_id = g_malloc(size); > } else { > @@ -608,6 +618,14 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp) > } > } > > + if (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data, > + parent_dev_id, size, &size)) { > + // parent_dev_id already is allocated > + error_setg_win32(errp, GetLastError(), > + "failed to get device instance ID"); > + goto cleanup; > + } > + > /* > * CM API used here as opposed to > * SetupDiGetDeviceProperty(..., DEVPKEY_Device_Parent, ...) > -- > 2.25.1 >
diff --git a/qga/commands-win32.c b/qga/commands-win32.c index cef14a8762..6bde5260e8 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -552,10 +552,10 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp) if (SetupDiEnumDeviceInterfaces(dev_info, &dev_info_data, &GUID_DEVINTERFACE_DISK, 0, &dev_iface_data)) { - while (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data, - pdev_iface_detail_data, - size, &size, - &dev_info_data)) { + if (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data, + pdev_iface_detail_data, + size, &size, + &dev_info_data)) { if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) { pdev_iface_detail_data = g_malloc(size); pdev_iface_detail_data->cbSize = @@ -567,6 +567,16 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp) } } + if (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data, + pdev_iface_detail_data, + size, &size, + &dev_info_data)) { + // pdev_iface_detail_data already is allocated + error_setg_win32(errp, GetLastError(), + "failed to get device interfaces"); + goto cleanup; + } + dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL); @@ -597,8 +607,8 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp) ULONG dev_id_size = 0; size = 0; - while (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data, - parent_dev_id, size, &size)) { + if (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data, + parent_dev_id, size, &size)) { if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) { parent_dev_id = g_malloc(size); } else { @@ -608,6 +618,14 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp) } } + if (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data, + parent_dev_id, size, &size)) { + // parent_dev_id already is allocated + error_setg_win32(errp, GetLastError(), + "failed to get device instance ID"); + goto cleanup; + } + /* * CM API used here as opposed to * SetupDiGetDeviceProperty(..., DEVPKEY_Device_Parent, ...)