[PATCH] malloc: Do not make out-of-bounds madvise call on non-aligned heap

[Dev Jain]

On 02/12/25 5:54 pm, Dev Jain wrote:
> Currently, if the initial program break is not aligned to the system page
> size, then we align the pointer down to the page size. If there is a gap
> before the heap VMA, then such an adjustment means that the madvise() range
> now contains a gap. The behaviour in the upstream kernel is currently this:
> madvise() will return -ENOMEM, even though the operation will still succeed
> in the sense that the VM_HUGEPAGE flag will be set on the heap VMA. We
> *must not* depend on this behaviour - this is an internal kernel
> implementation, and earlier kernels may possibly abort the operation
> altogether.
>
> The other case is that there is no gap, and as a result we may end up
> setting the VM_HUGEPAGE flag on that other VMA too, which is an
> unnecessary side effect.
>
> Let us fix this by aligning the pointer up to the page size. We should
> also subtract the pointer difference from the size, because if we don't,
> since the pointer is now aligned up, the size may cross the heap VMA, thus
> leading to the same problem but at the other end.
>
> There is no need to check this new size against mp_.thp_pagesize to decide
> whether to make the madvise() call. The reason we make this check at the
> start of madvise_thp() is to check whether the size of the VMA is enough
> to map THPs into it. Since that check has passed, all that we need to
> ensure now is that q + size does not cross the heap VMA.
>
> A 0.95% improvement is observed on SPEC.
> ---

Please do omit out the SPEC improvement - I double checked and the improvement
comes on the new master branch, not because of this patch.


> Patch applies on 3dd2cbfa35e0.
>
>   malloc/malloc.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index bd92d5c396..2c56c1f124 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -2071,8 +2071,8 @@ madvise_thp (void *p, INTERNAL_SIZE_T size)
>        inputs happens only for initial data segment.  */
>     if (__glibc_unlikely (!PTR_IS_ALIGNED (p, GLRO (dl_pagesize))))
>       {
> -      void *q = PTR_ALIGN_DOWN (p, GLRO (dl_pagesize));
> -      size += PTR_DIFF (p, q);
> +      void *q = PTR_ALIGN_UP (p, GLRO (dl_pagesize));
> +      size -= PTR_DIFF (q, p);
>         p = q;
>       }
>