Re: [PATCH] elf: Align argument of __munmap to page size [BZ #28676]

[Florian Weimer <fweimer@redhat.com>]

* H. J. Lu:

> On Mon, Dec 13, 2021 at 11:40 PM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu via Libc-alpha:
>>
>> > On Mon, Dec 13, 2021 at 6:49 PM Rongwei Wang
>> > <rongwei.wang@linux.alibaba.com> wrote:
>> >>
>> >> Hi, HJ
>> >>
>> >> Thanks for your help to fix this bug. And I just found out that the
>> >> patch "elf: Properly align PT_LOAD segments" had been merged into
>> >> glibc/master.
>> >>
>> >> And I saw Florian's email:
>> >>
>> >>  > We should check munmap failure though and rollback everything if
>> >>  > necessary.  It's possible we can undo the initial PROT_NONE mapping even
>> >>  > if future munmap calls fail because unmapping the first mapping does not
>> >>  > need to split a mapping.
>> >>
>> >> It seems rollback initial PROT_NONE mapping when munmap fails is good
>> >> idea. Nice to have. Of course, IMO, this your patch is also can fix this
>> >> munmap failure well. what do you think?
>> >>
>> >> When this fix is stable in glibc upstream, I will backport it into our
>> >> internal glibc repo. So, if you have any improve this fix suggestions, I
>> >> will respond immediately. Sorry for my speed is much slower than your.
>> >>
>> >>
>> >
>> > Let's wait for a few days.
>>
>> Can we please fix the regression in some way?  It impacts development,
>> particularly at this late stage during the 2.34 cycle.
>>
>> Either revert the change, or at least add the unchecked __munmap, and
>> fix that later.
>
> What is the unchecked __munmap?

The __munmap at the end:

diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h
index 70a4c40695..54e606aa87 100644
--- a/elf/dl-map-segments.h
+++ b/elf/dl-map-segments.h
@@ -55,6 +55,7 @@ _dl_map_segment (const struct loadcmd *c, ElfW(Addr) mappref,
       if (delta)
 	__munmap ((void *) map_start, delta);
       ElfW(Addr) map_end = map_start_aligned + maplength;
+      map_end = ALIGN_UP (map_end, GLRO(dl_pagesize));
       delta = map_start + maplen - map_end;
       if (delta)
 	__munmap ((void *) map_end, delta);

Thanks,
Florian