On Sat, May 23, 2015 at 12:28 PM, Theo Buehler <[email protected]> wrote:
> This set of three patches adds overflow checking to ksh in the spirit
> of the malloc(A*B) -> reallocarray(NULL, A, B) conversions that were
> ongoing since last summer. I've been running these patches on my main
> laptop since January on amd64/CURRENT and didn't notice any issues.
>
> ksh has its own memory management functions and only calls malloc and
> realloc in the functions alloc() and aresize() in alloc.c. There the
> `size' arguments have the form
>
> sizeof(struct link) + size
>
> where struct link is defined as
>
> struct link {
> struct link *prev;
> struct link *next;
> };
>
> In order to ensure that this doesn't overflow if `size' is a product of
> two numbers, wrap alloc() and aresize() into two functions which take
> the two factors as arguments and take care of the overflow checking:
>
> void *allocarray(size_t nmemb, size_t size, Area *ap);
> void *aresizearray(void *ptr, size_t nmemb, size_t size, Area *ap);
>
> The mathematically optimal check would test whether at least one of
> `size' and `nmemb' exceeds `sqrt(SIZE_MAX - sizeof(nmemb))' before
> proceeding. I went for something which is easier to compute.
>
> This first patch introduces the two wrappers and adds the overflow
> checking.
>
> The other two patches consist of purely mechanical conversions:
>
> Expand the macro
>
> #define sizeofN(type, n) (sizeof(type) * n)
>
> and take care of all explicit multiplications.
>
Hi, Please don't forget to include Otto's license to the code, that
you modified.
>
> Index: alloc.c
> ===================================================================
> RCS file: /cvs/src/bin/ksh/alloc.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 alloc.c
> --- alloc.c 21 Jul 2008 17:30:08 -0000 1.8
> +++ alloc.c 23 May 2015 11:58:27 -0000
> @@ -74,6 +74,33 @@ alloc(size_t size, Area *ap)
> return L2P(l);
> }
>
> +/*
> + * This is sqrt(SIZE_MAX+1), as s1*s2 <= SIZE_MAX
> + * if both s1 < MUL_NO_OVERFLOW and s2 < MUL_NO_OVERFLOW
> + */
> +#define MUL_NO_OVERFLOW ((size_t)1 << (sizeof(size_t) * 4))
> +/*
> + * Generous upper bound for sqrt(sizeof(struct link)).
> + */
> +#define SQRT_SIZ_STR_LINK (sizeof(struct link) / 2)
> +
> +void *
> +allocarray(size_t nmemb, size_t size, Area *ap)
> +{
> + /*
> + * Ensure that `sizeof(struct link) + size * nmemb' doesn't overflow.
> + * If overflow occurs, at least one of `size' and `nmemb' must be
> + * larger than sqrt(SIZE_MAX - sizeof(struct link)).
> + * Note: sqrt(a - b) > sqrt(a) - sqrt(b) for a > b.
> + */
> + if ((nmemb >= MUL_NO_OVERFLOW - SQRT_SIZ_STR_LINK ||
> + size >= MUL_NO_OVERFLOW - SQRT_SIZ_STR_LINK) &&
> + nmemb > 0 && (SIZE_MAX - sizeof(struct link)) / nmemb < size)
> + internal_errorf(1, "unable to allocate memory");
> +
> + return (alloc(size * nmemb, ap));
> +}
> +
> void *
> aresize(void *ptr, size_t size, Area *ap)
> {
> @@ -97,6 +124,18 @@ aresize(void *ptr, size_t size, Area *ap
> lnext->prev = l2;
>
> return L2P(l2);
> +}
> +
> +void *
> +aresizearray(void *ptr, size_t nmemb, size_t size, Area *ap)
> +{
> + /* Ensure that `sizeof(struct link) + size * nmemb' doesn't overflow.
> */
> + if ((size >= MUL_NO_OVERFLOW - SQRT_SIZ_STR_LINK ||
> + nmemb >= MUL_NO_OVERFLOW - SQRT_SIZ_STR_LINK) &&
> + nmemb > 0 && (SIZE_MAX - sizeof(struct link)) / nmemb < size)
> + internal_errorf(1, "unable to allocate memory");
> +
> + return (aresize(ptr, size * nmemb, ap));
> }
>
> void
> Index: proto.h
> ===================================================================
> RCS file: /cvs/src/bin/ksh/proto.h,v
> retrieving revision 1.35
> diff -u -p -r1.35 proto.h
> --- proto.h 4 Sep 2013 15:49:19 -0000 1.35
> +++ proto.h 23 May 2015 11:58:27 -0000
> @@ -10,7 +10,9 @@
> Area * ainit(Area *);
> void afreeall(Area *);
> void * alloc(size_t, Area *);
> +void * allocarray(size_t, size_t, Area *);
> void * aresize(void *, size_t, Area *);
> +void * aresizearray(void *, size_t, size_t, Area *);
> void afree(void *, Area *);
> /* c_ksh.c */
> int c_hash(char **);
> Index: sh.h
> ===================================================================
> RCS file: /cvs/src/bin/ksh/sh.h,v
> retrieving revision 1.33
> diff -u -p -r1.33 sh.h
> --- sh.h 18 Dec 2013 13:53:12 -0000 1.33
> +++ sh.h 23 May 2015 11:58:27 -0000
> @@ -15,6 +15,7 @@
> #include <setjmp.h>
> #include <stdbool.h>
> #include <stddef.h>
> +#include <stdint.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <string.h>
>
