Re: [Qemu-devel] [PATCH v3] virtio-rng: Add human-readable error message for negative max-bytes parameter

[Amit Shah]

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 <address@hidden>
> ---
> 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...

                Amit