motorhead/qemu
1
0
Fork 0
mirror of https://github.com/Motorhead1991/qemu.git synced 2025-08-10 02:54:58 -06:00
Code Issues Projects Releases Packages Wiki Activity Actions

opts: don't silently truncate long option values

Browse source 
The existing QemuOpts parsing code uses a fixed size 1024 byte buffer
for storing the option values. If a value exceeded this size it was
silently truncated and no error reported to the user. Long option values
is not a common scenario, but it is conceivable that they will happen.
eg if the user has a very deeply nested filesystem it would be possible
to come up with a disk path that was > 1024 bytes. Most of the time if
such data was silently truncated, the user would get an error about
opening a non-existant disk. If they're unlucky though, QEMU might use a
completely different disk image from another VM, which could be
considered a security issue. Another example program was in using the
-smbios command line arg with very large data blobs. In this case the
silent truncation will be providing semantically incorrect data to the
guest OS for SMBIOS tables.

If the operating system didn't limit the user's argv when spawning QEMU,
the code should honour whatever length arguments were given without
imposing its own length restrictions. This patch thus changes the code
to use a heap allocated buffer for storing the values during parsing,
lifting the arbitrary length restriction.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20180416111743.8473-4-berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2018-04-16 12:17:43 +01:00 • committed by Paolo Bonzini
parent e652714f98
commit 950c4e6c94
3 changed files with 82 additions and 66 deletions
Download patch file Download diff file Expand all files Collapse all files

33
hw/i386/multiboot.c
View file

@ -291,12 +291,16 @@ int load_multiboot(FWCfgState *fw_cfg,
cmdline_len = strlen(kernel_filename) + 1;
cmdline_len += strlen(kernel_cmdline) + 1;
if (initrd_filename) {
const char *r = initrd_filename;
const char *r = get_opt_value(initrd_filename, NULL);
cmdline_len += strlen(r) + 1;
mbs.mb_mods_avail = 1;
while (*(r = get_opt_value(NULL, 0, r))) {
mbs.mb_mods_avail++;
r++;
while (1) {
mbs.mb_mods_avail++;
r = get_opt_value(r, NULL);
if (!*r) {
break;
}
r++;
}
}
@ -313,7 +317,8 @@ int load_multiboot(FWCfgState *fw_cfg,
if (initrd_filename) {
const char *next_initrd;
char not_last, tmpbuf[strlen(initrd_filename) + 1];
char not_last;
char *one_file = NULL;
mbs.offset_mods = mbs.mb_buf_size;
@ -322,24 +327,26 @@ int load_multiboot(FWCfgState *fw_cfg,
int mb_mod_length;
uint32_t offs = mbs.mb_buf_size;
next_initrd = get_opt_value(tmpbuf, sizeof(tmpbuf), initrd_filename);
next_initrd = get_opt_value(initrd_filename, &one_file);
not_last = *next_initrd;
/* if a space comes after the module filename, treat everything
after that as parameters */
hwaddr c = mb_add_cmdline(&mbs, tmpbuf);
if ((next_space = strchr(tmpbuf, ' ')))
hwaddr c = mb_add_cmdline(&mbs, one_file);
next_space = strchr(one_file, ' ');
if (next_space) {
*next_space = '\0';
mb_debug("multiboot loading module: %s", tmpbuf);
mb_mod_length = get_image_size(tmpbuf);
}
mb_debug("multiboot loading module: %s", one_file);
mb_mod_length = get_image_size(one_file);
if (mb_mod_length < 0) {
error_report("Failed to open file '%s'", tmpbuf);
error_report("Failed to open file '%s'", one_file);
exit(1);
}
mbs.mb_buf_size = TARGET_PAGE_ALIGN(mb_mod_length + mbs.mb_buf_size);
mbs.mb_buf = g_realloc(mbs.mb_buf, mbs.mb_buf_size);
load_image(tmpbuf, (unsigned char *)mbs.mb_buf + offs);
load_image(one_file, (unsigned char *)mbs.mb_buf + offs);
mb_add_mod(&mbs, mbs.mb_buf_phys + offs,
mbs.mb_buf_phys + offs + mb_mod_length, c);
@ -347,6 +354,8 @@ int load_multiboot(FWCfgState *fw_cfg,
(char *)mbs.mb_buf + offs,
(char *)mbs.mb_buf + offs + mb_mod_length, c);
initrd_filename = next_initrd+1;
g_free(one_file);
one_file = NULL;
} while (not_last);
}