[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] virtio-rng: Add human-readable error message
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v3] virtio-rng: Add human-readable error message for negative max-bytes parameter |
Date: |
Tue, 22 Jul 2014 13:41:43 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Amit Shah <address@hidden> 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 <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...
Could be treated as separate problem.