Re: [Qemu-devel] [PATCH 1/1 v3] Allow machines to configure the QEMU_VERSION that's exposed via hardware

[Eric Blake]

On 04/13/2012 01:16 PM, Crístian Viana wrote:
> Based on the following conversation:
> 
> http://mid.gmane.org/address@hidden
> 
>> Which reminds me - qemu sticks the release version in
>> guest visible places like CPU version.
>> This is wrong and causes windows guests to print messages
>> about driver updates when you switch.
>> We should find all these places and stop doing this.
> 
> There is a new field on the struct QEmuMachine, hw_version, which may
> contain the version that the specific machine should report. If that
> field is set, then that machine will report that version to the virtual
> machine.
> 

> +++ b/hw/nseries.c
> @@ -1247,7 +1247,7 @@ static int n8x0_atag_setup(void *p, int model)
>      stw_raw(w ++, 24);                               /* u16 len */
>      strcpy((void *) w, "hw-build");          /* char component[12] */
>      w += 6;
> -    strcpy((void *) w, "QEMU " QEMU_VERSION);        /* char version[12] */
> +    sprintf((void *) w, "QEMU %s", qemu_get_version()); /* char version[12] 
> */


> @@ -242,3 +244,12 @@ ssize_t qemu_recv_full(int fd, void *buf, size_t count, 
> int flags)
>      return total;
>  }
>  
> +void qemu_set_version(const char *version)
> +{
> +    qemu_version = version;
> +}
> +
> +const char *qemu_get_version(void)
> +{
> +    return qemu_version;
> +}

qemu_get_version returns whatever string got put there by
qemu_set_version.  Am I correct that the user has full control over the
string passed to qemu_set_version?  If so, then you have a bug in
nseries.c: sprintf() is asking for a buffer overflow.  Remember,
QEMU_VERSION has a compile-time fixed length, but if qemu_get_version()
is an arbitrary user string, you no longer have a guarantee that you fit
in version[12].  Besides, since you are only pasting in a single string,
you could do this more efficiently through a strcat() variant instead of
switching to snprintf() (I'm not sure off-hand which qemu_* function is
best for your purpose, but there's probably something out there).

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature