[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qga-win: prevent crash when executing fsinf
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qga-win: prevent crash when executing fsinfo command |
Date: |
Thu, 28 Jun 2018 16:44:18 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 06/26/2018 10:10 AM, Sameeh Jubran wrote:
From: Sameeh Jubran <address@hidden>
The fsinfo command is currently implemented for Windows only and it's disk
parameter can be enabled by adding the define "CONFIG_QGA_NTDDSCSI" to the qga
code. When enabled and executed the qemu-ga crashed with the following message:
------------------------------------------------
File qapi/qapi-visit-core.c, Line 49
Expression: !(v->type & VISITOR_OUTPUT) || *obj)
------------------------------------------------
After some digging, turns out that the GuestPCIAddress is null and the
qapi visitor doesn't like that, so we can always allocate it instead and
initiate all it's members to -1.
Adding Markus for a qapi back-compat question.
Is faking an invalid address better than making the output optional instead?
+++ b/qga/commands-win32.c
@@ -485,6 +485,11 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
**errp)
char *buffer = NULL;
GuestPCIAddress *pci = NULL;
char *name = g_strdup(&guid[4]);
+ pci = g_malloc0(sizeof(*pci));
+ pci->domain = -1;
+ pci->slot = -1;
+ pci->function = -1;
+ pci->bus = -1;
Right now, we have:
##
# @GuestDiskAddress:
#
# @pci-controller: controller's PCI address
# @bus-type: bus type
# @bus: bus id
# @target: target id
# @unit: unit id
#
# Since: 2.2
##
{ 'struct': 'GuestDiskAddress',
'data': {'pci-controller': 'GuestPCIAddress',
'bus-type': 'GuestDiskBusType',
'bus': 'int', 'target': 'int', 'unit': 'int'} }
and the problem you ran into is that under certain conditions, we have
no idea what to populate in GuestPCIAddress. Your patch populates
garbage instead; but I'm wondering if it would be better to instead mark
pci-controller as optional, where code that CAN populate it would set
has_pci_controller=true, and the code that crashed will now work by
leaving the struct NULL (and has_pci_controller=false). But that
removes output that the client may expect to be present, hence, this is
a back-compat question of the best way to approach this.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org