On Mon, Nov 08, 2021 at 10:08:59PM +0200, Nir Soffer wrote: > On Mon, Nov 8, 2021 at 9:56 PM Eric Blake <[email protected]> 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) { > > Just to make sure I understand this correctly: > v->cap * itemsize must be valid value since this is the current > allocation, computed in a previous call to generic_vector_reserve... > > > + errno = ENOMEM; > > return -1; /* overflow */ > > + } > > > > newcap = (v->cap * 3 + 1) / 2; > > > > - if (newcap < reqcap) > > + if (newcap * itemsize < reqcap * itemsize) > > So reqcap * itemsize is valid because we passed the check above. > > > newcap = reqcap; > > > > newptr = realloc (v->ptr, newcap * itemsize); > > Looks right to me.
I agree too, it seems right here. Guess it's one case where a proof checker could actually help :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top _______________________________________________ Libguestfs mailing list [email protected] https://listman.redhat.com/mailman/listinfo/libguestfs
