[libvirt] [PATCH] _virtualboxCreateMachine: Avoid unbound stack
If the stars are in the right position and you're building with VBox >= 4.2.0 it will happen that compiler thinks an array allocated on the stack may be unbound: In file included from vbox/vbox_V4_2.c:13:0: vbox/vbox_tmpl.c: In function '_virtualboxCreateMachine': vbox/vbox_tmpl.c:2811:1: error: stack usage might be unbounded [-Werror=stack-usage=] _virtualboxCreateMachine(vboxGlobalData *data, virDomainDefPtr def, IMachine **machine, char *uuidstr ATTRIBUTE_UNUSED) ^ Well, given how the variable is declared, I had some hard time seeing it is actually bounded. Surprisingly compiler does not complain because of -Wframe-larger-than. This is because variable length arrays do not count into that warning. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/vbox/vbox_tmpl.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 0fbd5b3..7a8205d 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2812,7 +2812,7 @@ _virtualboxCreateMachine(vboxGlobalData *data, virDomainDefPtr def, IMachine **m { vboxIID iid = VBOX_IID_INITIALIZER; PRUnichar *machineNameUtf16 = NULL; - nsresult rc; + nsresult rc = -1; VBOX_UTF8_TO_UTF16(def->name, &machineNameUtf16); vboxIIDFromUUID(&iid, def->uuid); @@ -2843,18 +2843,12 @@ _virtualboxCreateMachine(vboxGlobalData *data, virDomainDefPtr def, IMachine **m override, machine); #else /* VBOX_API_VERSION >= 4002000 */ - const char *flagsUUIDPrefix = "UUID="; - const char *flagsForceOverwrite = "forceOverwrite=0"; - const char *flagsSeparator = ","; - char createFlags[strlen(flagsUUIDPrefix) + VIR_UUID_STRING_BUFLEN + strlen(flagsSeparator) + strlen(flagsForceOverwrite) + 1]; + char *createFlags = NULL; PRUnichar *createFlagsUtf16 = NULL; - snprintf(createFlags, sizeof(createFlags), "%s%s%s%s", - flagsUUIDPrefix, - uuidstr, - flagsSeparator, - flagsForceOverwrite - ); + if (virAsprintf(&createFlags, + "UUID=%s,forceOverwrite=0", uuidstr) < 0) + goto cleanup; VBOX_UTF8_TO_UTF16(createFlags, &createFlagsUtf16); rc = data->vboxObj->vtbl->CreateMachine(data->vboxObj, NULL, @@ -2864,6 +2858,8 @@ _virtualboxCreateMachine(vboxGlobalData *data, virDomainDefPtr def, IMachine **m nsnull, createFlagsUtf16, machine); + cleanup: + VIR_FREE(createFlags); #endif /* VBOX_API_VERSION >= 4002000 */ } VBOX_UTF16_FREE(machineNameUtf16); -- 2.4.10
On Sat, Mar 05, 2016 at 14:05:31 +0100, Michal Privoznik wrote:
If the stars are in the right position and you're building with
One would think that electron alignment in the material of your SSD that creates the bits that finally align to the file creating the software on your machine would cause this ... I'm not sure that star alignment has to do much with it.
VBox >= 4.2.0 it will happen that compiler thinks an array allocated on the stack may be unbound:
There's quite a substantial semantic difference between "unbound" and
In file included from vbox/vbox_V4_2.c:13:0: vbox/vbox_tmpl.c: In function '_virtualboxCreateMachine': vbox/vbox_tmpl.c:2811:1: error: stack usage might be unbounded [-Werror=stack-usage=]
... unbounded. I'd suggest you get your language straight. Plus one other instance in the subhect.
_virtualboxCreateMachine(vboxGlobalData *data, virDomainDefPtr def, IMachine **machine, char *uuidstr ATTRIBUTE_UNUSED) ^
Well, given how the variable is declared, I had some hard time seeing it is actually bounded. Surprisingly compiler does not
Here it's correct!
complain because of -Wframe-larger-than. This is because variable length arrays do not count into that warning.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/vbox/vbox_tmpl.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
ACK, the original code was a mess ... Peter
participants (2)
-
Michal Privoznik -
Peter Krempa