On 09/04/2014 09:54 AM, Peter Krempa wrote: > On 08/31/14 06:02, Eric Blake wrote: >> qemu treats blockjob bandwidth as a 64-bit number, in the units >> of bytes/second. But we stupidly modeled block job bandwidth >> after migration bandwidth, which in turn was an 'unsigned long' >> and therefore subject to 32-bit vs. 64-bit interpretations, and >> with a scale of MiB/s. Our code already has to convert between >> the two scales, and report overflow as appropriate; although >> this conversion currently lives in the monitor code. >>
>>
>> - /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol
>> is
>> - * limited to LLONG_MAX also for unsigned values */
>> - speed = bandwidth;
>> - if (speed > LLONG_MAX >> 20) {
>> - virReportError(VIR_ERR_OVERFLOW,
>> - _("bandwidth must be less than %llu"),
>> - LLONG_MAX >> 20);
>
> I'd keep the check for if (speed > LLONG_MAX) here to be sure that we
> don't pass something between LLONG_MAX and ULLONG_MAX to qemu as it
> would be converted to signed by the monitor code.
The callers in qemu_driver.c are making the same check, so by this
point, it is just a redundant safety check. I'm not sure it adds
anything, since it is not arbitrary user input so much as protection
against developer botches.
>
> Possibly we could add a new conversion option to
> qemuMonitorJSONMakeCommand that would check and reject numbers between
> LLONG_MAX and ULLONG_MAX rather than converting them to signed silently...
That actually sounds interesting - a new code that requires a positive
value within a signed value. I'll see what it looks like for v4.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
