On (Tue) 22 Jul 2014 [13:41:43], Markus Armbruster wrote:
> Amit Shah <amit.s...@redhat.com> writes:
>
> > On (Mon) 21 Jul 2014 [17:44:37], John Snow wrote:
> >> If a negative integer is used for the max_bytes parameter, QEMU currently
> >> calls abort() and leaves behind a core dump. This patch adds a simple
> >> error message to make the reason for the termination clearer.
> >>
> >> There is an underlying insufficiency in the parameter parsing code of QEMU
> >> that renders it unable to reject negative values for unsigned properties,
> >> thus the error message "a non-negative integer below 2^63" is the most
> >> user-friendly and correct message we can give until the underlying
> >> insufficiency is corrected.
> >>
> >> Signed-off-by: John Snow <js...@redhat.com>
> >> ---
> >> v3: Adjusted the error message to be more semantically meaningful, but
> >> while acknowledging the limitations of the current unsigned integer
> >> parsing routines.
> >>
> >> hw/virtio/virtio-rng.c | 8 +++++++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> >> index 1356aca..7c5a675 100644
> >> --- a/hw/virtio/virtio-rng.c
> >> +++ b/hw/virtio/virtio-rng.c
> >> @@ -181,7 +181,13 @@ static void virtio_rng_device_realize(DeviceState
> >> *dev, Error **errp)
> >>
> >> vrng->vq = virtio_add_queue(vdev, 8, handle_input);
> >>
> >> - assert(vrng->conf.max_bytes <= INT64_MAX);
> >> + /* Workaround: Property parsing does not enforce unsigned integers,
> >> + * So this is a hack to reject such numbers. */
> >> + if (vrng->conf.max_bytes > INT64_MAX) {
> >> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "max-bytes",
> >> + "a non-negative integer below 2^63");
> >
> > Huh, why do we allow 0? There's no reason to have 0 as a max-bytes
> > value as well...
>
> Could be treated as separate problem.
Yep, don't mean to hold this up for that one.
Thanks for the reviewed-by.
Amit
