|
|
Message-ID: <20260421135025.GO1827@brightrain.aerifal.cx>
Date: Tue, 21 Apr 2026 09:50:25 -0400
From: Rich Felker <dalias@...c.org>
To: Florian Schmaus <florian.schmaus@...asip.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] dynlink: set errno in case mprotect() fails
On Tue, Apr 21, 2026 at 09:13:17AM -0400, Rich Felker wrote:
> On Tue, Apr 21, 2026 at 08:34:11AM -0400, Rich Felker wrote:
> > On Tue, Apr 21, 2026 at 09:57:27AM +0200, Florian Schmaus wrote:
> > > The format string of the error message uses %m to print the value of
> > > errno in human-readable form. But the low-level __syscall() macro,
> > > used to invoke mprotect(), will not implicitly set errno. Fix this by
> > > setting errno explicitly before emitting the error message.
> > > ---
> > > ldso/dynlink.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> > > index 715948f4f8b3..db0f9f52461c 100644
> > > --- a/ldso/dynlink.c
> > > +++ b/ldso/dynlink.c
> > > @@ -1424,6 +1424,7 @@ static void reloc_all(struct dso *p)
> > > long ret = __syscall(SYS_mprotect, laddr(p, p->relro_start),
> > > p->relro_end-p->relro_start, PROT_READ);
> > > if (ret != 0 && ret != -ENOSYS) {
> > > + errno = -ret;
> > > error("Error relocating %s: RELRO protection failed: %m",
> > > p->name);
> > > if (runtime) longjmp(*rtld_fail, 1);
> > > --
> > > 2.53.0
> >
> > I think this requires further analysis. See commit
> > 63c67053a3e42e9dff788de432f82ff07d4d772a. Avoiding accessing errno was
> > intentional here, and the access buried in the error path is probably
> > a bug. But we need to look back at the constraints to determine how to
> > fix it right.
>
> At the early stage where there is no errno, error points to error_noop
> so there is no problem except for lack of message. This is only for
> libc itself and probably only happens if it was mislinked or if
> there's bad seccomp policy.
>
> However the case you're looking at is for other libraries after stage
> 2 has finished, and indeed there the message lacks a meaningful errno.
>
> We could probably move the assignment into errno into the error()
> function, by passing an error code to replace errno as the first
> argument before the format string.
>
> The other approach would be making a way to lookup strerror directly
> here and use that, but calling libc functions is not valid yet at this
> point. In practice it would probably happen to work, but that depends
> on strerror not using errno/TLS and is not correct in our model of
> what's legal to call at stage 2.
Or we could just add a new function pointer like error in dynlink.c
but that's for setting errno, initially a nop/dummy. That's probably
cleaner than altering error's signature.
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.