[Libguestfs] [libnbd PATCH] common/utils/vector: Better overflow handling
Nir Soffer
nsoffer at redhat.com
Tue Nov 9 12:14:38 UTC 2021
On Mon, Nov 8, 2021 at 9:56 PM Eric Blake <eblake at redhat.com> wrote:
>
> Check newcap * itemsize for overflow prior to calling realloc, so that
> we don't accidentally truncate an existing array. Set errno to ENOMEM
> on all failure paths, rather than leaving it indeterminate on
> overflow. The patch works by assuming a prerequisite that
> v->cap*itemsize is always smaller than size_t.
>
> Fixes: 985dfa72ae (common/utils/vector.c: Optimize vector append)
> ---
>
> Unless you see problems in this, I'll push this and also port it to
> nbdkit.
>
> common/utils/vector.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/common/utils/vector.c b/common/utils/vector.c
> index a4b43ce..c37f0c3 100644
> --- a/common/utils/vector.c
> +++ b/common/utils/vector.c
> @@ -1,5 +1,5 @@
> /* nbdkit
> - * Copyright (C) 2018-2020 Red Hat Inc.
> + * Copyright (C) 2018-2021 Red Hat Inc.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions are
> @@ -44,12 +44,14 @@ generic_vector_reserve (struct generic_vector *v, size_t n, size_t itemsize)
> size_t reqcap, newcap;
>
> reqcap = v->cap + n;
> - if (reqcap < v->cap)
> + if (reqcap * itemsize < v->cap * itemsize) {
> + errno = ENOMEM;
> return -1; /* overflow */
> + }
>
> newcap = (v->cap * 3 + 1) / 2;
Should we use:
newcap = v->cap + (v->cap + 1) / 2
so we don't overflow too early?
And if we overflow - meaning that we cannot grow by factor of 1.5,
grow to maximum size:
if (newcap * itemsize < v->cap * itemsize)
newcap = SIZE_MAX / itemsize;
This makes a difference when itemsize is 1 or 2. The current
code will stop growing efficiently when allocation is 1.45g, and
then go back to realloc() on every call.
> - if (newcap < reqcap)
> + if (newcap * itemsize < reqcap * itemsize)
> newcap = reqcap;
If we handle overflow before, we can keep the old check
comparing caps.
I can post a patch with this changes if you like.
Nir
More information about the Libguestfs
mailing list