[libvirt] [PATCH] virsh: Avoid division by 0 in vshCalloc
Peter Krempa
pkrempa at redhat.com
Wed Jul 4 10:14:32 UTC 2012
On 07/04/12 11:43, Daniel P. Berrange wrote:
> On Wed, Jul 04, 2012 at 11:38:10AM +0200, Peter Krempa wrote:
>> On 07/04/12 11:09, Daniel P. Berrange wrote:
>>> On Wed, Jul 04, 2012 at 11:05:40AM +0200, Peter Krempa wrote:
>>>> vshCalloc function uses xalloc_oversized macro that can't take 0 as it's
>>>> second argument. If vshCalloc is called with size 0, virsh ends with a
>>>> floating point exception.
>>>>
>>>> This patch changes vshCalloc to return NULL if no memory is requested.
>>>> ---
>>>> tools/virsh.c | 3 +++
>>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/tools/virsh.c b/tools/virsh.c
>>>> index 53d1825..d3d5c6a 100644
>>>> --- a/tools/virsh.c
>>>> +++ b/tools/virsh.c
>>>> @@ -460,6 +460,9 @@ _vshCalloc(vshControl *ctl, size_t nmemb, size_t size, const char *filename, int
>>>> {
>>>> char *x;
>>>>
>>>> + if (!size)
>>>> + return NULL;
>>>> +
>>>> if (!xalloc_oversized(nmemb, size) &&
>>>
>>>
>>> IMHO this div-by-zero problem is a bug in the xalloc_oversized
>>> macro & we should fix it there. The scenario seen here in virsh
>>> is a fairly common and so div-by-zero could affect any other
>>> usage of that macro
>>
>> Yes it could. But the docs for the macro state that it shouldn't be called with 0 as the second argument:
>>
>> /* Return 1 if an array of N objects, each of size S, cannot exist due
>> to size arithmetic overflow. S must be positive and N must be
>> nonnegative. This is a macro, not an inline function, so that it
>> works correctly even when SIZE_MAX < N.
>>
>> But assuming that 0 elements of something will never overflow we could change the macro to:
>>
>> from:
>> # ifndef xalloc_oversized
>> # define xalloc_oversized(n, s) \
>> ((size_t) (sizeof(ptrdiff_t) <= sizeof(size_t) ? -1 : -2) / (s) < (n))
>> # endif
>>
>> to:
>> (s?((size_t) (sizeof(ptrdiff_t) <= sizeof(size_t) ? -1 : -2) / (s) < (n)):0)
>>
>> which would take care of the 0 argument.
>>
>> Is this what you had in mind?
>
> Yes, I think it is wrong to expect that 'S' must be non-zero,
> so we should change the docs too.
I found the actual problem for this. I think that this macro is in
order. S is the size of the array element, which doesn't make sense to
be 0 and N is the count of elements. The floating point exception
happened because the count and size arguments were interchanged in the
problematic place. I'll send the patch anyways, but I don't think now,
that the macro requires fixing.
Peter
>
> Daniel
>
More information about the libvir-list
mailing list