Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240229181617.GW4163@brightrain.aerifal.cx>
Date: Thu, 29 Feb 2024 13:16:18 -0500
From: Rich Felker <dalias@...c.org>
To: Max Filippov <jcmvbkbc@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: Initial xtensa/fdpic port review

On Thu, Feb 29, 2024 at 08:25:12AM -0800, Max Filippov wrote:
> On Wed, Feb 28, 2024 at 10:30 AM Rich Felker <dalias@...c.org> wrote:
> > On Wed, Feb 28, 2024 at 09:20:33AM -0800, Max Filippov wrote:
> > > > > diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> > > > > index ceca3c98..25563af3 100644
> > > > > --- a/ldso/dynlink.c
> > > > > +++ b/ldso/dynlink.c
> > > > > @@ -1420,6 +1420,7 @@ static void reloc_all(struct dso *p)
> > > > >               if (!DL_FDPIC)
> > > > >                       do_relr_relocs(p, laddr(p, dyn[DT_RELR]), dyn[DT_RELRSZ]);
> > > > >
> > > > > +#if 0
> > > > >               if (head != &ldso && p->relro_start != p->relro_end) {
> > > > >                       long ret = __syscall(SYS_mprotect, laddr(p, p->relro_start),
> > > > >                               p->relro_end-p->relro_start, PROT_READ);
> > > > > @@ -1429,6 +1430,7 @@ static void reloc_all(struct dso *p)
> > > > >                               if (runtime) longjmp(*rtld_fail, 1);
> > > > >                       }
> > > > >               }
> > > > > +#endif
> > > >
> > > > Was this breaking something? Relro linking probably should have been
> > > > disabled on fdpic, and we ignore ENOSYS anyway for nommu.
> > >
> > > Yeah, I do some of the testing in linux-user QEMU, it has MMU
> > > and mprotect calls actually succeed. Failures were coming from the
> > > relocation code, but my impression was that relro should be applied
> > > after the relocation completion and it should just work, hence the
> > > WIP pile.
> >
> > Yes, relro is only supposed to be applied after all relocations were
> > done.
> 
> So I've found two issues with this. First is that loadmap entries generated
> by the QEMU (and by the linux kernel AFAICS) are not page-aligned,
> but the relro segment addresses fetched by the loader are. Since the
> laddr() doesn't check that a translation for the address it was given
> was actually found the results are not always correct, mprotect fails
> and it all terminates early.
> 
> With a workaround for that part in place I see that relro protection is
> applied to the executable image before its __fdpic_fixup had a chance
> to run, there are rofixups for the .init_array and .fini_array, but they both
> are a part of the relro segment.

OK, in that case, we probably should disable relro with fdpic until
it's fixed to work properly.

I think relro processing for the main executable should probably be
moved to run as the first step of __libc_start_init, so that it
happens after __fdpic_fixup has run. We also need to either make
relro_start and relro_end maintain the exact addresses and only get
expanded to full-page just before calling mprotect, or use laddr_pg to
get the containing page.

Unrelated to fdpic, there may also be an issue that relro processing
has a broken dependency on runtime-variable pagesize. In practice it's
probaby okay since the binary must have been linked for maxpagesize >=
PAGE_SIZE, but we should probably double-check if the current
alignment logic is semantically correct.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.