svn commit: r278739 - head/lib/libc/regex

[Bruce Evans]

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