[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.