[Bug dynamic-link/28688] New: PT_LOAD p_align check is too strict

[Bug dynamic-link/28688] New: PT_LOAD p_align check is too strict

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28688

            Bug ID: 28688
           Summary: PT_LOAD p_align check is too strict
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: dynamic-link
          Assignee: unassigned at sourceware dot org
          Reporter: fweimer at redhat dot com
  Target Milestone: ---

We currently have:

        case PT_LOAD:
          /* A load command tells us to map in part of the file.
             We record the load commands and process them all later.  */
          if (__glibc_unlikely ((ph->p_align & (GLRO(dl_pagesize) - 1)) != 0))
            {
              errstring = N_("ELF load command alignment not page-aligned");
              goto lose;
            }

This check is too restrictive. p_align does not have to be a multiple of the
page size. Of course, there are file layout constraints, but over-aligning the
first mapping because of a larger page size is fine. The check currently
prevents that.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug dynamic-link/28688] PT_LOAD p_align check is too strict

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28688

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://sourceware.org/bugz
                   |                            |illa/show_bug.cgi?id=28689

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug dynamic-link/28688] PT_LOAD p_align check is too strict

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28688

--- Comment #1 from Florian Weimer <fweimer at redhat dot com> ---
The check below uses p_align:

          if (__glibc_unlikely (((ph->p_vaddr - ph->p_offset)
                                 & (ph->p_align - 1)) != 0))
            {
              errstring
                = N_("ELF load command address/offset not properly aligned");
              goto lose;
            }

If the first check is dropped, it needs to use GLRO(dl_pagesize) instead.
Ideally, the error message would also include the run-time page size.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug dynamic-link/28688] PT_LOAD p_align check is too strict

[hjl.tools at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28688

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unspecified                 |2.26
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |2.35

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> ---
Fixed for 2.35 by

commit 163f625cf9becbb82dfec63a29e566324129c0cd
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Dec 21 12:35:47 2021 -0800

    elf: Remove excessive p_align check on PT_LOAD segments [BZ #28688]

    p_align does not have to be a multiple of the page size.  Only PT_LOAD
    segment layout should be aligned to the page size.

    1: Remove p_align check against the page size.
    2. Use the page size, instead of p_align, to check PT_LOAD segment layout.

    Reviewed-by: Florian Weimer <fweimer@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug dynamic-link/28688] PT_LOAD p_align check is too strict

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28688

--- Comment #3 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The release/2.35/master branch has been updated by H.J. Lu
<hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=a787325e8316830f8d9a118aac5afee3c0e75617

commit a787325e8316830f8d9a118aac5afee3c0e75617
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Feb 24 07:58:36 2022 -0800

    NEWS: Add a bug fix entry for BZ #28688

-- 
You are receiving this mail because:
You are on the CC list for the bug.