[PATCH, libfortran] Backport xmallocarray to 4.8/4.9 (CVE-2014-5044)

[Tobias Burnus]

Jakub Jelinek wrote:
> On Sat, Aug 02, 2014 at 12:09:24AM +0300, Janne Blomqvist wrote:
>>> --- libgfortran/runtime/memory.c.jj     2014-06-18 08:50:33.000000000 +0200
>>> +++ libgfortran/runtime/memory.c        2014-08-01 14:41:08.385856116 +0200
>>> @@ -56,7 +56,9 @@ xmallocarray (size_t nmemb, size_t size)
>>>
>>>     if (!nmemb || !size)
>>>       size = nmemb = 1;
>>> -  else if (nmemb > SIZE_MAX / size)
>>> +#define HALF_SIZE_T (((size_t) 1) << (__CHAR_BIT__ * sizeof (size_t) / 2))
>>> +  else if (__builtin_expect ((nmemb | size) >= HALF_SIZE_T, 0)
>>> +          && nmemb > SIZE_MAX / size)
>>>       {
>>>         errno = ENOMEM;
>>>         os_error ("Integer overflow in xmallocarray");
>> Nice, though as os_error() has the _Noreturn specifier the
>> __builtin_expect() is not necessary, right? In libgfortran.h we have
> The reason for __builtin_expect here was to make already the
> nmemb > SIZE_MAX / size
> computation as unlikely, the noreturn predictor will of course DTRT with the
> {} block.

But there is a difference in probability between __builtin_expect and 
"noreturn". __builtin_expect had until two years ago a probability of 
99%, now it has a probability of only 90% (which is tunable with a 
-param)* – while "noreturn" has a higher probability. Thus, at least if 
you had used
else if (unlikely(... & ...))
os_error
you would have made the basic block with os_error more likely than 
without the "unlikely" (alias __builtin_expect). However, I don't know 
what happens with using "unlikely(cond1) && cond2".

Tobias

* Or internally in the compiler only, by passing a third argument.