lkml.org 
[lkml]   [2023]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3] ELF: document some de-facto PT_* ABI quirks
    On Thu, Dec 07, 2023 at 09:03:45AM -0600, Eric W. Biederman wrote:
    > Kees Cook <keescook@chromium.org> writes:
    >
    > > *thread necromancy* Question below...
    > >
    > > On Sat, Apr 15, 2023 at 08:37:29PM +0300, Alexey Dobriyan wrote:
    > >> Turns out rules about PT_INTERP, PT_GNU_STACK and PT_GNU_PROPERTY
    > >> program headers are slightly different.
    > >>
    > >> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
    > >> ---
    > >>
    > >> v3: move to Documentation/userspace-api/
    > >> v2: integrate into documentation build system
    > >>
    > >> Documentation/userspace-api/ELF.rst | 34 ++++++++++++++++++++++++++++++++++
    > >> Documentation/userspace-api/index.rst | 1 +
    > >> 2 files changed, 35 insertions(+)
    > >>
    > >> new file mode 100644
    > >> --- /dev/null
    > >> +++ b/Documentation/userspace-api/ELF.rst
    > >> @@ -0,0 +1,34 @@
    > >> +.. SPDX-License-Identifier: GPL-2.0
    > >> +
    > >> +=================================
    > >> +Linux-specific ELF idiosyncrasies
    > >> +=================================
    > >> +
    > >> +Definitions
    > >> +===========
    > >> +
    > >> +"First" program header is the one with the smallest offset in the file:
    > >> +e_phoff.
    >
    > Confusing e_phoff is the defined location of the array of program
    > headers.
    >
    > Perhaps the "First" in that array with the lowest e_phnum?
    >
    > >> +"Last" program header is the one with the biggest offset in the file:
    > >> +e_phoff + (e_phnum - 1) * sizeof(Elf_Phdr).
    >
    > Ditto the "Last" in the array with the largest array index.
    >
    > I nit pick this because it sounded at first like you were talking about
    > p_offset. Which is a value contained in the program header entry.
    >
    > >> +PT_INTERP
    > >> +=========
    > >> +
    > >> +First PT_INTERP program header is used to locate the filename of ELF
    > >> +interpreter. Other PT_INTERP headers are ignored (since Linux 2.4.11).
    > >> +
    > >> +PT_GNU_STACK
    > >> +============
    > >> +
    > >> +Last PT_GNU_STACK program header defines userspace stack executability
    > >> +(since Linux 2.6.6). Other PT_GNU_STACK headers are ignored.
    > >> +
    > >> +PT_GNU_PROPERTY
    > >> +===============
    > >> +
    > >> +ELF interpreter's last PT_GNU_PROPERTY program header is used (since
    > >> +Linux 5.8). If interpreter doesn't have one, then the last PT_GNU_PROPERTY
    > >> +program header of an executable is used. Other PT_GNU_PROPERTY headers
    > >> +are ignored.
    >
    > A more interesting property to document is that PT_GNU_PROPERTY must
    > precede PT_INTERP in the linux implementation, otherwise we ignore it.
    >
    > > Should we perhaps solve some of these in some way? What would folks
    > > prefer the behaviors be? (I like to have things been "as expected", but
    > > it's not very obvious here for redundant headers...)
    >
    > All of these are really headers that should appear only once.

    Yes.

    > Quite frankly if we are going to do something with this my sense is that
    > we should fail the execve with a clear error code as userspace should
    > not be doing this, and accepting a malformed executable will hide
    > errors, and perhaps hide someone causing problems.

    Maybe do it for PT_GNU_PROPERTY which is relatively new.

    > I really don't think having multiple copies of these headers with
    > different values is something we should encourage.
    >
    > It looks like -ELIBBAD is the documented way to fail and report
    > a bad file format.

    It is obvious you don't know how much will break.

    > For PT_GNU_PROPTERTY perhaps we should accept it anywhere, instead of
    > silently ignoring it depending upon it's location?
    >
    > I thinking change the code to talk one pass through the program headers
    > to identify the interesting headers, and then with the interesting
    > headers all identified we go do something with them.
    >
    > Anyway just my opinion, but that is what it feels like to me.

    _Not_ checking for duplicates will result in the simplest and fastest exec.
    which is what current code does.

    \
     
     \ /
      Last update: 2023-12-10 12:46    [W:4.968 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site