[v5,1/2] elf: Properly align PT_LOAD segments

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

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Rongwei Wang Dec. 10, 2021, 12:39 p.m. UTC
  When PT_LOAD segment alignment > the page size, allocate
enough space to ensure that the segment can be properly
aligned.

And this fix can help code segments use huge pages become
simple and available.

This fixes [BZ #28676].

Signed-off-by: Xu Yu <xuyu@linux.alibaba.com>
Signed-off-by: Rongwei Wang <rongwei.wang@linux.alibaba.com>
---
 elf/dl-load.c         |  1 +
 elf/dl-load.h         |  2 +-
 elf/dl-map-segments.h | 49 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 47 insertions(+), 5 deletions(-)
  

Comments

H.J. Lu Dec. 10, 2021, 3:43 p.m. UTC | #1
On Fri, Dec 10, 2021 at 4:39 AM Rongwei Wang
<rongwei.wang@linux.alibaba.com> wrote:
>
> When PT_LOAD segment alignment > the page size, allocate
> enough space to ensure that the segment can be properly
> aligned.
>
> And this fix can help code segments use huge pages become
> simple and available.
>
> This fixes [BZ #28676].
>
> Signed-off-by: Xu Yu <xuyu@linux.alibaba.com>
> Signed-off-by: Rongwei Wang <rongwei.wang@linux.alibaba.com>
> ---
>  elf/dl-load.c         |  1 +
>  elf/dl-load.h         |  2 +-
>  elf/dl-map-segments.h | 49 +++++++++++++++++++++++++++++++++++++++----
>  3 files changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index bf8957e73c..9a23590bf4 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1150,6 +1150,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>           c->mapend = ALIGN_UP (ph->p_vaddr + ph->p_filesz, GLRO(dl_pagesize));
>           c->dataend = ph->p_vaddr + ph->p_filesz;
>           c->allocend = ph->p_vaddr + ph->p_memsz;
> +         c->mapalign = ph->p_align;
>           c->mapoff = ALIGN_DOWN (ph->p_offset, GLRO(dl_pagesize));
>
>           /* Determine whether there is a gap between the last segment
> diff --git a/elf/dl-load.h b/elf/dl-load.h
> index e329d49a81..c121e3456c 100644
> --- a/elf/dl-load.h
> +++ b/elf/dl-load.h
> @@ -74,7 +74,7 @@ ELF_PREFERRED_ADDRESS_DATA;
>     Its details have been expanded out and converted.  */
>  struct loadcmd
>  {
> -  ElfW(Addr) mapstart, mapend, dataend, allocend;
> +  ElfW(Addr) mapstart, mapend, dataend, allocend, mapalign;
>    ElfW(Off) mapoff;
>    int prot;                             /* PROT_* bits.  */
>  };
> diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h
> index f9fb110ee3..74abf324ed 100644
> --- a/elf/dl-map-segments.h
> +++ b/elf/dl-map-segments.h
> @@ -18,6 +18,50 @@
>
>  #include <dl-load.h>
>
> +/* Map a segment and align it properly.  */
> +
> +static __always_inline ElfW(Addr)
> +_dl_map_segment (const struct loadcmd *c, ElfW(Addr) mappref,
> +                const size_t maplength, int fd)
> +{
> +  if (__glibc_likely (c->mapalign <= GLRO(dl_pagesize)))
> +    return (ElfW(Addr)) __mmap ((void *) mappref, maplength, c->prot,
> +                               MAP_COPY|MAP_FILE, fd, c->mapoff);
> +
> +  /* If the segment alignment > the page size, allocate enough space to
> +     ensure that the segment can be properly aligned.  */
> +  ElfW(Addr) maplen = (maplength >= c->mapalign
> +                      ? (maplength + c->mapalign)
> +                      : (2 * c->mapalign));
> +  ElfW(Addr) map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplen,
> +                                             PROT_NONE,
> +                                             MAP_ANONYMOUS|MAP_PRIVATE,
> +                                             -1, 0);
> +  if (__glibc_unlikely ((void *) map_start == MAP_FAILED))
> +    return map_start;
> +
> +  ElfW(Addr) map_start_aligned = ALIGN_UP (map_start, c->mapalign);
> +  map_start_aligned = (ElfW(Addr)) __mmap ((void *) map_start_aligned,
> +                                          maplength, c->prot,
> +                                          MAP_COPY|MAP_FILE|MAP_FIXED,
> +                                          fd, c->mapoff);
> +  if (__glibc_unlikely ((void *) map_start_aligned == MAP_FAILED))
> +    __munmap ((void *) map_start, maplen);
> +  else
> +    {
> +      /* Unmap the unused regions.  */
> +      ElfW(Addr) delta = map_start_aligned - map_start;
> +      if (delta)
> +       __munmap ((void *) map_start, delta);
> +      ElfW(Addr) map_end = map_start_aligned + maplength;
> +      delta = map_start + maplen - map_end;
> +      if (delta)
> +       __munmap ((void *) map_end, delta);
> +    }
> +
> +  return map_start_aligned;
> +}
> +
>  /* This implementation assumes (as does the corresponding implementation
>     of _dl_unmap_segments, in dl-unmap-segments.h) that shared objects
>     are always laid out with all segments contiguous (or with gaps
> @@ -53,10 +97,7 @@ _dl_map_segments (struct link_map *l, int fd,
>             - MAP_BASE_ADDR (l));
>
>        /* Remember which part of the address space this object uses.  */
> -      l->l_map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplength,
> -                                            c->prot,
> -                                            MAP_COPY|MAP_FILE,
> -                                            fd, c->mapoff);
> +      l->l_map_start = _dl_map_segment (c, mappref, maplength, fd);
>        if (__glibc_unlikely ((void *) l->l_map_start == MAP_FAILED))
>          return DL_MAP_SEGMENTS_ERROR_MAP_SEGMENT;
>
> --
> 2.27.0
>

LGTM.

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

I will check it for you.

Thanks.
  
Florian Weimer Dec. 10, 2021, 3:45 p.m. UTC | #2
* H. J. Lu:

> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
>
> I will check it for you.

Please adjust the copyright line due to Signed-off-by.  Thanks.

Florian
  
H.J. Lu Dec. 10, 2021, 6:54 p.m. UTC | #3
On Fri, Dec 10, 2021 at 7:45 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> >
> > I will check it for you.
>
> Please adjust the copyright line due to Signed-off-by.  Thanks.
>
> Florian

This is the patch I am checking in.

Thanks.
  
H.J. Lu Dec. 10, 2021, 6:57 p.m. UTC | #4
On Fri, Dec 10, 2021 at 10:54 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Dec 10, 2021 at 7:45 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu:
> >
> > > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> > >
> > > I will check it for you.
> >
> > Please adjust the copyright line due to Signed-off-by.  Thanks.
> >
> > Florian
>
> This is the patch I am checking in.
>
> Thanks.
>
> --
> H.J.

This is the right one.
  

Patch

diff --git a/elf/dl-load.c b/elf/dl-load.c
index bf8957e73c..9a23590bf4 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1150,6 +1150,7 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 	  c->mapend = ALIGN_UP (ph->p_vaddr + ph->p_filesz, GLRO(dl_pagesize));
 	  c->dataend = ph->p_vaddr + ph->p_filesz;
 	  c->allocend = ph->p_vaddr + ph->p_memsz;
+	  c->mapalign = ph->p_align;
 	  c->mapoff = ALIGN_DOWN (ph->p_offset, GLRO(dl_pagesize));
 
 	  /* Determine whether there is a gap between the last segment
diff --git a/elf/dl-load.h b/elf/dl-load.h
index e329d49a81..c121e3456c 100644
--- a/elf/dl-load.h
+++ b/elf/dl-load.h
@@ -74,7 +74,7 @@  ELF_PREFERRED_ADDRESS_DATA;
    Its details have been expanded out and converted.  */
 struct loadcmd
 {
-  ElfW(Addr) mapstart, mapend, dataend, allocend;
+  ElfW(Addr) mapstart, mapend, dataend, allocend, mapalign;
   ElfW(Off) mapoff;
   int prot;                             /* PROT_* bits.  */
 };
diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h
index f9fb110ee3..74abf324ed 100644
--- a/elf/dl-map-segments.h
+++ b/elf/dl-map-segments.h
@@ -18,6 +18,50 @@ 
 
 #include <dl-load.h>
 
+/* Map a segment and align it properly.  */
+
+static __always_inline ElfW(Addr)
+_dl_map_segment (const struct loadcmd *c, ElfW(Addr) mappref,
+		 const size_t maplength, int fd)
+{
+  if (__glibc_likely (c->mapalign <= GLRO(dl_pagesize)))
+    return (ElfW(Addr)) __mmap ((void *) mappref, maplength, c->prot,
+				MAP_COPY|MAP_FILE, fd, c->mapoff);
+
+  /* If the segment alignment > the page size, allocate enough space to
+     ensure that the segment can be properly aligned.  */
+  ElfW(Addr) maplen = (maplength >= c->mapalign
+		       ? (maplength + c->mapalign)
+		       : (2 * c->mapalign));
+  ElfW(Addr) map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplen,
+					      PROT_NONE,
+					      MAP_ANONYMOUS|MAP_PRIVATE,
+					      -1, 0);
+  if (__glibc_unlikely ((void *) map_start == MAP_FAILED))
+    return map_start;
+
+  ElfW(Addr) map_start_aligned = ALIGN_UP (map_start, c->mapalign);
+  map_start_aligned = (ElfW(Addr)) __mmap ((void *) map_start_aligned,
+					   maplength, c->prot,
+					   MAP_COPY|MAP_FILE|MAP_FIXED,
+					   fd, c->mapoff);
+  if (__glibc_unlikely ((void *) map_start_aligned == MAP_FAILED))
+    __munmap ((void *) map_start, maplen);
+  else
+    {
+      /* Unmap the unused regions.  */
+      ElfW(Addr) delta = map_start_aligned - map_start;
+      if (delta)
+	__munmap ((void *) map_start, delta);
+      ElfW(Addr) map_end = map_start_aligned + maplength;
+      delta = map_start + maplen - map_end;
+      if (delta)
+	__munmap ((void *) map_end, delta);
+    }
+
+  return map_start_aligned;
+}
+
 /* This implementation assumes (as does the corresponding implementation
    of _dl_unmap_segments, in dl-unmap-segments.h) that shared objects
    are always laid out with all segments contiguous (or with gaps
@@ -53,10 +97,7 @@  _dl_map_segments (struct link_map *l, int fd,
            - MAP_BASE_ADDR (l));
 
       /* Remember which part of the address space this object uses.  */
-      l->l_map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplength,
-                                            c->prot,
-                                            MAP_COPY|MAP_FILE,
-                                            fd, c->mapoff);
+      l->l_map_start = _dl_map_segment (c, mappref, maplength, fd);
       if (__glibc_unlikely ((void *) l->l_map_start == MAP_FAILED))
         return DL_MAP_SEGMENTS_ERROR_MAP_SEGMENT;