qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/2] qstring: Assert size calculations don't ove


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH 1/2] qstring: Assert size calculations don't overflow
Date: Thu, 26 Jul 2018 19:27:46 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

Eric Blake <address@hidden> writes:

> On 07/26/2018 01:18 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>   qobject/qstring.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/qobject/qstring.c b/qobject/qstring.c
>> index 18b8eb82f8..7990569c5a 100644
>> --- a/qobject/qstring.c
>> +++ b/qobject/qstring.c
>> @@ -41,17 +41,19 @@ QString *qstring_from_substr(const char *str, size_t 
>> start, size_t end)
>>   {
>>       QString *qstring;
>>   +    assert(start <= end + 1);
>
> end + 1 can overflow size_t, but it is unsigned so well-defined, and
> the assert will trigger as desired.
>
>> @@ -68,7 +70,9 @@ QString *qstring_from_str(const char *str)
>>   static void capacity_increase(QString *qstring, size_t len)
>>   {
>>       if (qstring->capacity < (qstring->length + len)) {
>> +        assert(len <= SIZE_MAX - qstring->capacity);
>>           qstring->capacity += len;
>
> You've asserted that this addition won't overflow...
>
>> +        assert(qstring->capacity + len <= SIZE_MAX / 2);
>
> ...but now that qstring->capacity is larger, this could overflow. Do
> you really need the +len in here, given that...
>
>>           qstring->capacity *= 2; /* use exponential growth */
>
> ...you are really only trying to prevent overflow of doubling
> qstring->capacity without adding yet another len in the mix?

You're right, my assertion is broken.  v2 coming.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]