From 3b825735d8ad2645bafac8269e4078349dc1d949 Mon Sep 17 00:00:00 2001 From: FRIGN Date: Tue, 10 Mar 2015 21:16:21 +0100 Subject: [PATCH] Implement reallocarray() Stateless and I stumbled upon this issue while discussing the semantics of read, accepting a size_t but only being able to return ssize_t, effectively lacking the ability to report successful reads > SSIZE_MAX. The discussion went along and we came to the topic of input-based memory allocations. Basically, it was possible for the argument to a memory-allocation-function to overflow, leading to a segfault later. The OpenBSD-guys came up with the ingenious reallocarray-function, and I implemented it as ereallocarray, which automatically returns on error. Read more about it here[0]. A simple testcase is this (courtesy to stateless): $ sbase-strings -n (2^(32|64) / 4) This will segfault before this patch and properly return an OOM- situation afterwards (thanks to the overflow-check in reallocarray). [0]: http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/calloc.3 --- Makefile | 1 + col.c | 2 +- cut.c | 2 +- expand.c | 4 ++-- find.c | 10 ++++----- libutil/reallocarray.c | 51 ++++++++++++++++++++++++++++++++++++++++++ ls.c | 6 ++--- paste.c | 8 +++---- printf.c | 4 ++-- sed.c | 7 ++++-- strings.c | 4 ++-- tr.c | 6 ++--- unexpand.c | 4 ++-- util.h | 2 ++ 14 files changed, 84 insertions(+), 27 deletions(-) create mode 100644 libutil/reallocarray.c diff --git a/Makefile b/Makefile index 42f723f..5c55f43 100644 --- a/Makefile +++ b/Makefile @@ -57,6 +57,7 @@ LIBUTILSRC =\ libutil/md5.c\ libutil/mode.c\ libutil/putword.c\ + libutil/reallocarray.c\ libutil/recurse.c\ libutil/rm.c\ libutil/sha1.c\ diff --git a/col.c b/col.c index 89f30e1..585d6aa 100644 --- a/col.c +++ b/col.c @@ -177,7 +177,7 @@ allocbuf(void) { char **bp; - buff = emalloc(sizeof(*buff) * pagsize); + buff = ereallocarray(buff, pagsize, sizeof(*buff)); for (bp = buff; bp < &buff[pagsize]; ++bp) *bp = emalloc(NCOLS); } diff --git a/cut.c b/cut.c index d846cce..1658a5c 100644 --- a/cut.c +++ b/cut.c @@ -56,7 +56,7 @@ parselist(char *str) if (*s == ',') n++; } - r = emalloc(n * sizeof(Range)); + r = ereallocarray(r, n, sizeof(*r)); for (s = str; n; n--, s++) { r->min = (*s == '-') ? 1 : strtoul(s, &s, 10); r->max = (*s == '-') ? strtoul(s + 1, &s, 10) : r->min; diff --git a/expand.c b/expand.c index d47c567..2f02e26 100644 --- a/expand.c +++ b/expand.c @@ -19,12 +19,12 @@ parselist(const char *s) for (i = 0; (p = strsep(&tmp, " ,")); i++) { if (*p == '\0') eprintf("empty field in tablist\n"); - tablist = erealloc(tablist, (i + 1) * sizeof(*tablist)); + tablist = ereallocarray(tablist, i + 1, sizeof(*tablist)); tablist[i] = estrtonum(p, 1, MIN(LLONG_MAX, SIZE_MAX)); if (i > 0 && tablist[i - 1] >= tablist[i]) eprintf("tablist must be ascending\n"); } - tablist = erealloc(tablist, (i + 1) * sizeof(*tablist)); + tablist = ereallocarray(tablist, i + 1, sizeof(*tablist)); /* tab length = 1 for the overflowing case later in the matcher */ tablist[i] = 1; return i; diff --git a/find.c b/find.c index b0f3e31..ebea89b 100644 --- a/find.c +++ b/find.c @@ -380,7 +380,7 @@ pri_exec(Arg *arg) /* if we have too many files, realloc (with space for NULL termination) */ if (e->u.p.next + 1 == e->u.p.cap) - e->argv = erealloc(e->argv, (e->u.p.cap *= 2) * sizeof(*e->argv)); + e->argv = ereallocarray(e->argv, e->u.p.cap *= 2, sizeof(*e->argv)); e->argv[e->u.p.next++] = estrdup(arg->path); e->u.p.filelen += len + sizeof(arg->path); @@ -595,7 +595,7 @@ get_exec_arg(char *argv[], Extra *extra) e->u.p.arglen = e->u.p.filelen = 0; e->u.p.first = e->u.p.next = arg - argv - 1; e->u.p.cap = (arg - argv) * 2; - e->argv = emalloc(e->u.p.cap * sizeof(*e->argv)); + e->argv = ereallocarray(e->argv, e->u.p.cap, sizeof(*e->argv)); for (arg = argv, new = e->argv; *arg; arg++, new++) { *new = *arg; @@ -604,7 +604,7 @@ get_exec_arg(char *argv[], Extra *extra) arg++; /* due to our extra NULL */ } else { e->argv = argv; - e->u.s.braces = emalloc(++nbraces * sizeof(*e->u.s.braces)); /* ++ for NULL */ + e->u.s.braces = ereallocarray(e->u.s.braces, ++nbraces, sizeof(*e->u.s.braces)); /* ++ for NULL */ for (arg = argv, braces = e->u.s.braces; *arg; arg++) if (!strcmp(*arg, "{}")) @@ -632,7 +632,7 @@ get_ok_arg(char *argv[], Extra *extra) *arg = NULL; o->argv = argv; - o->braces = emalloc(++nbraces * sizeof(*o->braces)); + o->braces = ereallocarray(o->braces, ++nbraces, sizeof(*o->braces)); for (arg = argv, braces = o->braces; *arg; arg++) if (!strcmp(*arg, "{}")) @@ -824,7 +824,7 @@ parse(int argc, char **argv) * https://en.wikipedia.org/wiki/Shunting-yard_algorithm * read from infix, resulting rpn ends up in rpn, next position in rpn is out * push operators onto stack, next position in stack is top */ - rpn = emalloc((ntok + gflags.print) * sizeof(*rpn)); + rpn = ereallocarray(rpn, ntok + gflags.print, sizeof(*rpn)); for (tok = infix, out = rpn, top = stack; tok->type != END; tok++) { switch (tok->type) { case PRIM: *out++ = *tok; break; diff --git a/libutil/reallocarray.c b/libutil/reallocarray.c new file mode 100644 index 0000000..fb28267 --- /dev/null +++ b/libutil/reallocarray.c @@ -0,0 +1,51 @@ +/* $OpenBSD: reallocarray.c,v 1.1 2014/05/08 21:43:49 deraadt Exp $ */ +/* + * Copyright (c) 2008 Otto Moerbeek + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include +#include +#include +#include + +#include "../util.h" + +/* + * 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 (1UL << (sizeof(size_t) * 4)) + +void * +reallocarray(void *optr, size_t nmemb, size_t size) +{ + if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) && + nmemb > 0 && SIZE_MAX / nmemb < size) { + errno = ENOMEM; + return NULL; + } + return realloc(optr, size * nmemb); +} + +void * +ereallocarray(void *optr, size_t nmemb, size_t size) +{ + void *p; + + if (!(p = reallocarray(optr, nmemb, size))) + eprintf("reallocarray: out of memory\n"); + + return p; +} diff --git a/ls.c b/ls.c index ac38a09..44d430f 100644 --- a/ls.c +++ b/ls.c @@ -217,7 +217,7 @@ lsdir(const char *path) mkent(&ent, d->d_name, Fflag || lflag || pflag || iflag || Rflag, Lflag); ls(&ent, Rflag); } else { - ents = erealloc(ents, ++n * sizeof(*ents)); + ents = ereallocarray(ents, ++n, sizeof(*ents)); name = p = estrdup(d->d_name); if (qflag) { q = d->d_name; @@ -269,7 +269,7 @@ usage(void) int main(int argc, char *argv[]) { - struct entry *ents; + struct entry *ents = NULL; size_t i; ARGBEGIN { @@ -341,7 +341,7 @@ main(int argc, char *argv[]) if (argc == 0) *--argv = ".", argc++; - ents = emalloc(argc * sizeof(*ents)); + ents = ereallocarray(ents, argc, sizeof(*ents)); for (i = 0; i < argc; i++) mkent(&ents[i], argv[i], 1, Hflag || Lflag); diff --git a/paste.c b/paste.c index a80ca35..7a3878e 100644 --- a/paste.c +++ b/paste.c @@ -85,8 +85,8 @@ usage(void) int main(int argc, char *argv[]) { - struct fdescr *dsc; - Rune *delim; + struct fdescr *dsc = NULL; + Rune *delim = NULL; size_t i, len; int seq = 0; char *adelim = "\t"; @@ -107,11 +107,11 @@ main(int argc, char *argv[]) /* populate delimiters */ unescape(adelim); - delim = emalloc((utflen(adelim) + 1) * sizeof(*delim)); + delim = ereallocarray(delim, utflen(adelim) + 1, sizeof(*delim)); len = utftorunestr(adelim, delim); /* populate file list */ - dsc = emalloc(argc * sizeof(*dsc)); + dsc = ereallocarray(dsc, argc, sizeof(*dsc)); for (i = 0; i < argc; i++) { if (strcmp(argv[i], "-") == 0) diff --git a/printf.c b/printf.c index 3563790..e959a46 100644 --- a/printf.c +++ b/printf.c @@ -111,7 +111,7 @@ main(int argc, char *argv[]) break; case 'c': unescape(arg); - rarg = emalloc((utflen(arg) + 1) * sizeof(*rarg)); + rarg = ereallocarray(rarg, utflen(arg) + 1, sizeof(*rarg)); utftorunestr(arg, rarg); efputrune(rarg, stdout, ""); free(rarg); @@ -125,7 +125,7 @@ main(int argc, char *argv[]) if (arg[j] == '\'' || arg[j] == '\"') { arg += j + 1; unescape(arg); - rarg = emalloc((utflen(arg) + 1) * sizeof(*rarg)); + rarg = ereallocarray(rarg, utflen(arg) + 1, sizeof(*rarg)); utftorunestr(arg, rarg); num = rarg[0]; } else diff --git a/sed.c b/sed.c index bad7492..8e98062 100644 --- a/sed.c +++ b/sed.c @@ -275,7 +275,7 @@ resize(void **ptr, size_t *nmemb, size_t size, size_t new_nmemb, void **next) void *n, *tmp; if (new_nmemb) { - tmp = erealloc(*ptr, new_nmemb * size); + tmp = ereallocarray(*ptr, new_nmemb, size); } else { /* turns out realloc(*ptr, 0) != free(*ptr) */ free(*ptr); tmp = NULL; @@ -596,7 +596,10 @@ chompr(char *s, Rune rune) Rune * strtorunes(char *s, size_t nrunes) { - Rune *rs = emalloc(sizeof(*rs) * nrunes + 1), *rp = rs; + Rune *rs = NULL, *rp; + + rs = ereallocarray(rs, nrunes + 1, sizeof(*rs)); + rp = rs; while (nrunes--) s += chartorune(rp++, s); diff --git a/strings.c b/strings.c index cab0fe9..8b16069 100644 --- a/strings.c +++ b/strings.c @@ -11,11 +11,11 @@ static char *format = ""; static void strings(FILE *fp, const char *fname, size_t len) { - Rune r, *rbuf; + Rune r, *rbuf = NULL; size_t i, bread; off_t off; - rbuf = emalloc(len * sizeof(*rbuf)); + rbuf = ereallocarray(rbuf, len, sizeof(*rbuf)); for (off = 0, i = 0; (bread = efgetrune(&r, fp, fname)); ) { off += bread; diff --git a/tr.c b/tr.c index 9e52835..52b223f 100644 --- a/tr.c +++ b/tr.c @@ -71,16 +71,16 @@ rstrmatch(Rune *r, char *s, size_t n) static size_t makeset(char *str, struct range **set, int (**check)(Rune)) { - Rune *rstr; + Rune *rstr = NULL; size_t len, i, j, m, n; size_t q, setranges = 0; int factor, base; /* rstr defines at most len ranges */ unescape(str); - rstr = emalloc((utflen(str) + 1) * sizeof(*rstr)); + rstr = ereallocarray(rstr, utflen(str) + 1, sizeof(*rstr)); len = utftorunestr(str, rstr); - *set = emalloc(len * sizeof(**set)); + *set = ereallocarray(*set, len, sizeof(**set)); for (i = 0; i < len; i++) { if (rstr[i] == '[') { diff --git a/unexpand.c b/unexpand.c index feb965e..c565670 100644 --- a/unexpand.c +++ b/unexpand.c @@ -19,12 +19,12 @@ parselist(const char *s) for (i = 0; (p = strsep(&tmp, " ,")); i++) { if (*p == '\0') eprintf("empty field in tablist\n"); - tablist = erealloc(tablist, (i + 1) * sizeof(*tablist)); + tablist = ereallocarray(tablist, i + 1, sizeof(*tablist)); tablist[i] = estrtonum(p, 1, MIN(LLONG_MAX, SIZE_MAX)); if (i > 0 && tablist[i - 1] >= tablist[i]) eprintf("tablist must be ascending\n"); } - tablist = erealloc(tablist, (i + 1) * sizeof(*tablist)); + tablist = ereallocarray(tablist, i + 1, sizeof(*tablist)); return i; } diff --git a/util.h b/util.h index 3f16731..7370272 100644 --- a/util.h +++ b/util.h @@ -26,6 +26,8 @@ void apathmax(char **, size_t *); void *ecalloc(size_t, size_t); void *emalloc(size_t); void *erealloc(void *, size_t); +void *reallocarray(void *, size_t, size_t); +void *ereallocarray(void *, size_t, size_t); char *estrdup(const char *); char *estrndup(const char *, size_t); void *encalloc(int, size_t, size_t);