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

[Eric Blake]

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?

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org