Re: [PATCH 2/2] malloc: make malloc fail with requests larger than PTRDIFF_MAX

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 21/12/2018 20:32, Paul Eggert wrote:
> The patch assumes that PTRDIFF_MAX is well under SIZE_MAX. A while ago Joseph wrote that m32c sometimes has ptrdiff_t wider than size_t <https://gcc.gnu.org/ml/gcc-patches/2011-07/msg00483.html> and if someone ever does a glibc port to such a platform this issue will come up in unlikely cases. Perhaps add something like the following to malloc/malloc.c, to make sure the problem is harder to ignore on such platforms?
> 
> /* malloc.c assumes that ptrdiff_t has no more bits than size_t.
>    Although this assumption could be removed without hurting typical-case
>    performance, doing this is low priority since the assumption holds
>    on all current glibc platforms.  */
> #include <verify.h>
> verify (PTRDIFF_MAX <= SIZE_MAX / 2);
> 
> 
>> +  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
>> +    {
>> +      __set_errno (ENOMEM);
>> +      return NULL;
>> +    }
> 
> Why is the '- padsize' needed in these cases? All the relevant arithmetic is done using size_t, so overflow can't occur when PTRDIFF_MAX - padsize < bytes <= PTRDIFF_MAX, under the PTRDIFF_MAX <= SIZE_MAX / 2 assumption that the code is already making elsewhere.

I added for consistency so all internal allocation, after alignment and padding 
adjustment, will be also limited by PTRDIFF_MAX.  Although the idea is not to 
limit mmap/sbrk to such requirement I think it also helps sysmalloc to be more
conforming regarding the ptrdiff_t requirements.  However I did not made an
extensible analysis, so I don't have a strong opinion here.

> 
> 
>> +  if (INT_MULTIPLY_WRAPV (n, elem_size, &bytes)
>> +      || __glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
> 
> This can be made a bit simpler by changing the type of this particular 'bytes' variable to ptrdiff_t and then by removing the "|| __glibc_unlikely (bytes > PTRDIFF_MAX - padsize)", since the "- padsize" isn't needed and the INT_MULTIPLY_WRAPV will check for exceeding PTRDIFF_MAX if 'bytes' is of type ptrdiff_t.