mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: Florian Weimer <fweimer@redhat.com>
Cc: Nihal Jere <nihal@nihaljere.xyz>, musl@lists.openwall.com
Subject: Re: [musl] Dynamic linker segfault
Date: Fri, 7 Jan 2022 12:35:38 -0500	[thread overview]
Message-ID: <20220107173536.GK7074@brightrain.aerifal.cx> (raw)
In-Reply-To: <87v8yv3aw1.fsf@oldenburg.str.redhat.com>

On Fri, Jan 07, 2022 at 02:58:54PM +0100, Florian Weimer wrote:
> * Rich Felker:
> 
> >> With a p_align < PAGESIZE check in place, portable binaries need to use
> >> the value 65536.  When running with page size 4096, the loader cannot
> >> know whether p_align was set to this value merely to satisfy the p_align
> >> < PAGESIZE check, or because there is actually some section alignment
> >> that requires 65536 byte alignment.  There is no kernel interface to
> >> request 65536 byte alignment, so the loader has to do extra work to
> >> satisfy this request.  And in the first case (no actual 65536 byte
> >> alignment requirement), that work is unnecessary.
> >
> > Unfortunately it's impossible to distinguish between such an alignment
> > requirement and __attribute__((__aligned__(65536))) appearing in
> > section contents that went into the segment, so disregarding it is a
> > bug (one musl also has) I think.
> 
> Agreed.
> 
> >> > In any case, do you know if this test file is somehow related to that
> >> > work, or is it just a guess? It doesn't seem to be related to me since
> >> > it's essentially a "pageless" mapping setup.
> >> 
> >> The glibc test seems to be just buggy: First we verify p_align against
> >> the page size, then we use that p_align value to check the alignment of
> >> the PT_LOAD segment (mainly file offset congruency).  The p_align check
> >> against the page size looks completely optional to me if we check file
> >> offset congruency directly against the run-time page size.
> >> 
> >> The ELF specification explicitly describes the p_align values 0 and 1 as
> >> valid, indicating no alignment constraint.  So a p_align < PAGESIZE
> >> check is buggy in that regard as well.  This also conflicts with your
> >> interpretation as p_align as the maximum supported page size.
> >
> > The ELF specification describes syntax not semantic requirements on
> > the platform to support anything the syntax can represent.
> 
> That's not entirely accurate, there are exceptions.  p_align seems to be
> one such exception:
> 
> p_align
>     As ``Program Loading'' describes in this chapter of the processor
>     supplement, loadable process segments must have congruent values for
>     p_vaddr and p_offset, modulo the page size. This member gives the
>     value to which the segments are aligned in memory and in the
>     file. Values 0 and 1 mean no alignment is required. Otherwise,
>     p_align should be a positive, integral power of 2, and p_vaddr
>     should equal p_offset, modulo p_align.
> 
> Most processor supplements do not discuss p_align unfortunately?
> 
> > The semantics of the above can't be honored (without memcpy instead of
> > mmap, which is really something you don't want to support) if they
> > produce congruences incompatible with the layout, and they can't be
> > honored at all if they're incompatible with the permissions
> > requirements. Even when they don't conflict with the permissions
> > requirements, they may expose unintended mapped data (which for users
> > wanting separate-code is problematic, since it introduces gadgets). So
> > I'm not convinced it should be supported even when it "can".
> 
> The congruency compatibility is a property of the file layout, not the
> p_align value.  If the file layout is incompatible, changing p_align
> doesn't make the object loadable.
> 
> As far as I can tell, p_align is not at all useful, except for (e.g.)
> __attribute__((__aligned__(65536))) for global.  But even that wasn't
> implemented widely.

By my understanding, p_align implies an understanding by the creator
of the program that the load segment may be "over-mapped" (extra file
contents visible and possibly executable) up to that alignment
boundary due to page granularity. This isn't really a correctness
contract but a security/hardening contract for folks who consider
anti-ROP measures a boundary that should be enforced.

Rich

  reply	other threads:[~2022-01-07 17:35 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-04 21:27 Nihal Jere
2022-01-04 21:42 ` Rich Felker
2022-01-05  8:56   ` Florian Weimer
2022-01-05 13:49     ` Rich Felker
2022-01-05 14:00       ` Florian Weimer
2022-01-05 15:00         ` Rich Felker
2022-01-07 13:58           ` Florian Weimer
2022-01-07 17:35             ` Rich Felker [this message]
2022-01-10 13:30               ` Florian Weimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220107173536.GK7074@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=fweimer@redhat.com \
    --cc=musl@lists.openwall.com \
    --cc=nihal@nihaljere.xyz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).