mbox series

[v5,0/2] fix p_align on PT_LOAD segment in DSO isn't honored

Message ID 20211210123911.86568-1-rongwei.wang@linux.alibaba.com
Headers
Series fix p_align on PT_LOAD segment in DSO isn't honored |

Message

Rongwei Wang Dec. 10, 2021, 12:39 p.m. UTC 
  Hi

This patch mainly to fix a reported bug:

"p_align on PT_LOAD segment in DSO isn't honored"
https://sourceware.org/bugzilla/show_bug.cgi?id=28676

A testcase had been builded by H.J.Lu:
https://sourceware.org/bugzilla/attachment.cgi?id=13838

And in Patch 1/1, I also show a simple testcase which
modified from H.J.Lu.

And a similar bug in ELF binary was also reported:
https://bugzilla.kernel.org/show_bug.cgi?id=215275

A related fix patch could been found:
https://lore.kernel.org/linux-mm/20211009092658.59665-4-rongwei.wang@linux.alibaba.com/
Originally, we send this patch for introducing .text
hugepages, was not aware of it's bug. And now, I am
not sure whether kernel maintainer will regards it as
a bug. But it deserved try again.

Thanks.

Changelog:

v4 -> v5
- Patch "Add a testcase to check alignment of PT_LOAD segment"
add new testcase for PT_LOAD segment
- Patch "elf: Properly align PT_LOAD segments"
fix map_start to use map_start_aligned when second mmap failed

v3 -> v4
- Patch "elf: Properly align PT_LOAD segments"
Call unmap when the second mmap fails.

v2 -> v3
- Patch "elf: Properly align PT_LOAD segments"
move mapalign into 'struct loadcmd'
fix some coding style

RFC/v1 -> v2

- Patch "elf: align the mapping address of LOAD segments with p_align"
fix coding format and add testcase in commit.

RFC link:
https://patchwork.sourceware.org/project/glibc/patch/20211204045848.71105-2-rongwei.wang@linux.alibaba.com/

H.J. Lu (1):
  Add a testcase to check alignment of PT_LOAD segment

Rongwei Wang (1):
  elf: Properly align PT_LOAD segments

 elf/Makefile          | 14 +++++++++++--
 elf/dl-load.c         |  1 +
 elf/dl-load.h         |  2 +-
 elf/dl-map-segments.h | 49 +++++++++++++++++++++++++++++++++++++++----
 elf/tst-align3.c      | 37 ++++++++++++++++++++++++++++++++
 elf/tst-alignmod3.c   | 31 +++++++++++++++++++++++++++
 6 files changed, 127 insertions(+), 7 deletions(-)
 create mode 100644 elf/tst-align3.c
 create mode 100644 elf/tst-alignmod3.c

Comments

H.J. Lu Dec. 10, 2021, 1:13 p.m. UTC | #1 
On Fri, Dec 10, 2021 at 4:39 AM Rongwei Wang
<rongwei.wang@linux.alibaba.com> wrote:
>
> Hi
>
> This patch mainly to fix a reported bug:
>
> "p_align on PT_LOAD segment in DSO isn't honored"
> https://sourceware.org/bugzilla/show_bug.cgi?id=28676
>
> A testcase had been builded by H.J.Lu:
> https://sourceware.org/bugzilla/attachment.cgi?id=13838
>
> And in Patch 1/1, I also show a simple testcase which
> modified from H.J.Lu.
>
> And a similar bug in ELF binary was also reported:
> https://bugzilla.kernel.org/show_bug.cgi?id=215275

I submitted a kernel patch:

https://lore.kernel.org/lkml/20211209174052.370537-1-hjl.tools@gmail.com/T/

Hopefully, it will be landed soon.

> A related fix patch could been found:
> https://lore.kernel.org/linux-mm/20211009092658.59665-4-rongwei.wang@linux.alibaba.com/
> Originally, we send this patch for introducing .text
> hugepages, was not aware of it's bug. And now, I am
> not sure whether kernel maintainer will regards it as
> a bug. But it deserved try again.
>
> Thanks.
>
> Changelog:
>
> v4 -> v5
> - Patch "Add a testcase to check alignment of PT_LOAD segment"
> add new testcase for PT_LOAD segment
> - Patch "elf: Properly align PT_LOAD segments"
> fix map_start to use map_start_aligned when second mmap failed
>
> v3 -> v4
> - Patch "elf: Properly align PT_LOAD segments"
> Call unmap when the second mmap fails.
>
> v2 -> v3
> - Patch "elf: Properly align PT_LOAD segments"
> move mapalign into 'struct loadcmd'
> fix some coding style
>
> RFC/v1 -> v2
>
> - Patch "elf: align the mapping address of LOAD segments with p_align"
> fix coding format and add testcase in commit.
>
> RFC link:
> https://patchwork.sourceware.org/project/glibc/patch/20211204045848.71105-2-rongwei.wang@linux.alibaba.com/
>
> H.J. Lu (1):
>   Add a testcase to check alignment of PT_LOAD segment
>
> Rongwei Wang (1):
>   elf: Properly align PT_LOAD segments
>
>  elf/Makefile          | 14 +++++++++++--
>  elf/dl-load.c         |  1 +
>  elf/dl-load.h         |  2 +-
>  elf/dl-map-segments.h | 49 +++++++++++++++++++++++++++++++++++++++----
>  elf/tst-align3.c      | 37 ++++++++++++++++++++++++++++++++
>  elf/tst-alignmod3.c   | 31 +++++++++++++++++++++++++++
>  6 files changed, 127 insertions(+), 7 deletions(-)
>  create mode 100644 elf/tst-align3.c
>  create mode 100644 elf/tst-alignmod3.c
>
> --
> 2.27.0
>

LGTM.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Can you check it in?

Thanks.

--
H.J.
Rongwei Wang Dec. 10, 2021, 1:58 p.m. UTC | #2 
Hi

On 12/10/21 9:13 PM, H.J. Lu wrote:
> On Fri, Dec 10, 2021 at 4:39 AM Rongwei Wang
> <rongwei.wang@linux.alibaba.com> wrote:
>>
>> Hi
>>
>> This patch mainly to fix a reported bug:
>>
>> "p_align on PT_LOAD segment in DSO isn't honored"
>> https://sourceware.org/bugzilla/show_bug.cgi?id=28676
>>
>> A testcase had been builded by H.J.Lu:
>> https://sourceware.org/bugzilla/attachment.cgi?id=13838
>>
>> And in Patch 1/1, I also show a simple testcase which
>> modified from H.J.Lu.
>>
>> And a similar bug in ELF binary was also reported:
>> https://bugzilla.kernel.org/show_bug.cgi?id=215275
> 
> I submitted a kernel patch:
> 
> https://lore.kernel.org/lkml/20211209174052.370537-1-hjl.tools@gmail.com/T/
> 
> Hopefully, it will be landed soon.
Nice!

And if available, you can CC my email. And I think this patch can help 
us to introduce .text hugepages transparently.

> 
>> A related fix patch could been found:
>> https://lore.kernel.org/linux-mm/20211009092658.59665-4-rongwei.wang@linux.alibaba.com/
>> Originally, we send this patch for introducing .text
>> hugepages, was not aware of it's bug. And now, I am
>> not sure whether kernel maintainer will regards it as
>> a bug. But it deserved try again.
>>
>> Thanks.
>>
>> Changelog:
>>
>> v4 -> v5
>> - Patch "Add a testcase to check alignment of PT_LOAD segment"
>> add new testcase for PT_LOAD segment
>> - Patch "elf: Properly align PT_LOAD segments"
>> fix map_start to use map_start_aligned when second mmap failed
>>
>> v3 -> v4
>> - Patch "elf: Properly align PT_LOAD segments"
>> Call unmap when the second mmap fails.
>>
>> v2 -> v3
>> - Patch "elf: Properly align PT_LOAD segments"
>> move mapalign into 'struct loadcmd'
>> fix some coding style
>>
>> RFC/v1 -> v2
>>
>> - Patch "elf: align the mapping address of LOAD segments with p_align"
>> fix coding format and add testcase in commit.
>>
>> RFC link:
>> https://patchwork.sourceware.org/project/glibc/patch/20211204045848.71105-2-rongwei.wang@linux.alibaba.com/
>>
>> H.J. Lu (1):
>>    Add a testcase to check alignment of PT_LOAD segment
>>
>> Rongwei Wang (1):
>>    elf: Properly align PT_LOAD segments
>>
>>   elf/Makefile          | 14 +++++++++++--
>>   elf/dl-load.c         |  1 +
>>   elf/dl-load.h         |  2 +-
>>   elf/dl-map-segments.h | 49 +++++++++++++++++++++++++++++++++++++++----
>>   elf/tst-align3.c      | 37 ++++++++++++++++++++++++++++++++
>>   elf/tst-alignmod3.c   | 31 +++++++++++++++++++++++++++
>>   6 files changed, 127 insertions(+), 7 deletions(-)
>>   create mode 100644 elf/tst-align3.c
>>   create mode 100644 elf/tst-alignmod3.c
>>
>> --
>> 2.27.0
>>
> 
> LGTM.
> 
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> 
> Can you check it in?
I have some doubts. I had subscribed linux-kernel mail list long time, 
but I can not find your patch.

Do you have received any review mail?

Tnanks.

> 
> Thanks.
> 
> --
> H.J.
>