Date: Sat, 14 Feb 2015 21:52:27 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bryan Drewery <bdrewery@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Xin LI <delphij@freebsd.org> Subject: Re: svn commit: r278739 - head/lib/libc/regex Message-ID: <20150214213333.H945@besplex.bde.org> In-Reply-To: <54DE993E.2050704@FreeBSD.org> References: <201502140023.t1E0Nspc090570@svn.freebsd.org> <54DE98AC.6030309@FreeBSD.org> <54DE993E.2050704@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 13 Feb 2015, Bryan Drewery wrote: > On 2/13/2015 6:37 PM, Bryan Drewery wrote: >> On 2/13/2015 6:23 PM, Xin LI wrote: >>> Author: delphij >>> Date: Sat Feb 14 00:23:53 2015 >>> New Revision: 278739 >>> URL: https://svnweb.freebsd.org/changeset/base/278739 >>> >>> Log: >>> Disallow pattern spaces which would cause intermediate calculations to >>> overflow size_t. >>> ... >>> Modified: head/lib/libc/regex/regcomp.c >>> ============================================================================== >>> --- head/lib/libc/regex/regcomp.c Sat Feb 14 00:03:43 2015 (r278738) >>> +++ head/lib/libc/regex/regcomp.c Sat Feb 14 00:23:53 2015 (r278739) >>> @@ -192,6 +192,7 @@ regcomp(regex_t * __restrict preg, >>> struct parse *p = &pa; >>> int i; >>> size_t len; >>> + size_t maxlen; >>> #ifdef REDEBUG >>> # define GOODFLAGS(f) (f) >>> #else >>> @@ -213,7 +214,23 @@ regcomp(regex_t * __restrict preg, >>> g = (struct re_guts *)malloc(sizeof(struct re_guts)); >>> if (g == NULL) >>> return(REG_ESPACE); >>> + /* >>> + * Limit the pattern space to avoid a 32-bit overflow on buffer >>> + * extension. Also avoid any signed overflow in case of conversion >>> + * so make the real limit based on a 31-bit overflow. >>> + * >>> + * Likely not applicable on 64-bit systems but handle the case >>> + * generically (who are we to stop people from using ~715MB+ >>> + * patterns?). >>> + */ >>> + maxlen = ((size_t)-1 >> 1) / sizeof(sop) * 2 / 3; >>> + if (len >= maxlen) { >>> + free((char *)g); >> >> I was planning to submit a patch for review to remove all of this >> casting / and discuss. > > To be clear, I only mean in free(3) calls. But they are least bogus for the free() calls. >> In this example the malloc is casted to struct re_gets* but the free is >> casted to char *. Why different and why cast in free at all? Because this code attempted to be portable to K&R compilers (including broken ones, but with no standard it was hard to tell what was broken). With no prototypes, the arg to free() had to be cast. (Except, all pointers have the same representation except on exotic machines, so the cast was rarely necessary then or now.) With no void * in K&R1, free() took a char * arg and the cast had to be to that. STDC have the grandfather kludge of requiring char * and void * to be almost interchangable, so you can probably cast to either. I forget if it requires char * and void * to have the same representation, so that you can certainly cast to either. Even more than the same representation is required -- they must be passed in the same way. Old programs cast the result of malloc() to break warnings about malloc() not being declared, or possibly for portability to broken compilers which require the cast. Actually, it was unclear if they were broken -- before void * existed, malloc() returned char *, and it is not so clear for old char * as for not so old void * that automatic conversion from char * to any pointer type does or should happen (without a warning). New C++ programs require the cast even for void *. I don't like this. I also don't like the spelling of the sizeof arg in: g = (struct re_guts *)malloc(sizeof(struct re_guts)); It is clearer to write: g = malloc(sizeof(*g)); This works for any pointer g. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150214213333.H945>