Re: [PATCH v3] malloc: make malloc fail with requests larger than PTRDIFF_MAX (BZ#23741)

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

On 17/04/2019 18:33, Joseph Myers wrote:
> On Tue, 9 Apr 2019, Adhemerval Zanella wrote:
> 
>> As discussed previously on libc-alpha [1], this patch follows up the idea
>> and add both the __attribute_alloc_size__ on malloc functions (malloc,
>> calloc, realloc, reallocarray, valloc, pvalloc, memaling, and
>> posix_memalign) and limit maximum requested allocation size to up
>> PTRDIFF_MAX (taking into consideration internal padding and alignment).
> 
> __attribute_alloc_size__ is for functions that return a pointer.  That is, 
> it *cannot* be applied to posix_memalign (see remarks in 
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87683>).
> 
> This is an issue with the proposed commit message and ChangeLog entry, not 
> the patch itself which doesn't appear to apply the attribute to 
> posix_memalign after all.  The commit message should discuss why the 
> attribute is not applied to posix_memalign, and the ChangeLog entry should 
> not say it's being applied there.
> 

What about the updates commit message and ChangeLog:

--
malloc: make malloc fail with requests larger than PTRDIFF_MAX (BZ#23741)

As discussed previously on libc-alpha [1], this patch follows up the idea
and add both the __attribute_alloc_size__ on malloc functions (malloc,
calloc, realloc, reallocarray, valloc, pvalloc, memaling, and
posix_memalign) and limit maximum requested allocation size to up
PTRDIFF_MAX (taking into consideration internal padding and alignment).

This aligns glibc with gcc expected size defined by default warning
-Walloc-size-larger-than value which warns for allocation larger than
PTRDIFF_MAX.  It also aligns with gcc expectation regarding libc and
expected size, such as described in PR#67999 [2] and previously discussed
ISO C11 issues [3] on libc-alpha.

>From the RFC thread [4] and previous discussion, it seems that consensus
is only to limit such requested size for malloc functions, not the system
allocation one (mmap, sbrk, etc.).

The implementation changes checked_request2size to check for both overflow
and maximum object size up to PTRDIFF_MAX. No additional checks are done
on sysmalloc, so it can still issue mmap with values larger than
PTRDIFF_T depending on the requested size.

The __attribute_alloc_size__ is for functions that return a pointer only,
which means it cannot be applied to posix_memalign (see remarks in GCC
PR#87683 [5]). The runtime checks to limit maximum requested allocation
size does applies to posix_memalign.

Checked on x86_64-linux-gnu and i686-linux-gnu.

[1] https://sourceware.org/ml/libc-alpha/2018-11/msg00223.html
[2] https://gcc.gnu.org/bugzilla//show_bug.cgi?id=67999
[3] https://sourceware.org/ml/libc-alpha/2011-12/msg00066.html
[4] https://sourceware.org/ml/libc-alpha/2018-11/msg00224.html
[5] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87683

        [BZ #23741]
        * malloc/hooks.c (malloc_check, realloc_check): Use
        __builtin_add_overflow on overflow check and adapt to
        checked_request2size change.
        * malloc/malloc.c (__libc_malloc, __libc_realloc, _mid_memalign,
        __libc_pvalloc, __libc_calloc, _int_memalign): Limit maximum
        allocation size to PTRDIFF_MAX.
        (REQUEST_OUT_OF_RANGE): Remove macro.
        (checked_request2size): Change to inline function, use
        __builtin_add_overflow for overflow check and limit maximum requested
        size to PTRDIFF_MAX.
        (__libc_malloc, __libc_realloc, _int_malloc, _int_memalign): Limit
        maximum allocation size to PTRDIFF_MAX.
        (_mid_memalign): Rely on _int_memalign call for overflow check.
        (__libc_pvalloc): Use __builtin_add_overflow on overflow check.
        (__libc_calloc): Use __builtin_mul_overflow for overflow check and
        limit maximum requested size to PTRDIFF_MAX.
        * malloc/malloc.h (malloc, calloc, realloc, reallocarray, memalign,
        valloc, pvalloc): Add __attribute_alloc_size__.
        * stdlib/stdlib.h (malloc, realloc, reallocarray, valloc): Likewise.
        * malloc/tst-malloc-too-large.c (do_test): Add check for allocation
        larger than PTRDIFF_MAX.
        * malloc/tst-memalign.c (do_test): Disable -Walloc-size-larger-than=
        around tests of malloc with negative sizes.
        * malloc/tst-posix_memalign.c (do_test): Likewise.
        * malloc/tst-pvalloc.c (do_test): Likewise.
        * malloc/tst-valloc.c (do_test): Likewise.
        * malloc/tst-reallocarray.c (do_test): Replace call to reallocarray
        with resulting size allocation larger than PTRDIFF_MAX with
        reallocarray_nowarn.
        (reallocarray_nowarn): New function.
        * NEWS: Mention the malloc function semantic change.