[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220929230707.GA29905@brightrain.aerifal.cx>
Date: Thu, 29 Sep 2022 19:07:08 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Revisiting LFS64 removal
On Tue, Sep 27, 2022 at 03:08:54PM -0400, Rich Felker wrote:
> On Tue, Sep 27, 2022 at 03:03:57PM -0400, Rich Felker wrote:
> > On Tue, Sep 27, 2022 at 08:20:05AM -0400, Rich Felker wrote:
> > > On Tue, Sep 27, 2022 at 11:09:48AM +0200, Gabriel Ravier wrote:
> > > > On 9/27/22 00:04, Rich Felker wrote:
> > > > >On Sun, Sep 25, 2022 at 09:03:40PM -0400, Rich Felker wrote:
> > > > >>[...]
> > > > >>Of course these interfaces should not be used, and we never intended
> > > > >>for them to be used just there for linking-compat. So, I've wanted to
> > > > >>get rid of them for a long time now.
> > > > >>
> > > > >>I believe the simplest short-term way is probably going to be just
> > > > >>having the dynamic linker symbol lookup error path make one final
> > > > >>check before bailing out with an error:
> > > > >>
> > > > >>- If the symbol to lookup ends in "64"..
> > > > >>- ..and it's in a hard-coded list of LFS64-compat symbols..
> > > > >>- ..and looking up the name with the "64" removed in libc succeeds..
> > > > >>
> > > > >>Then use the version without the "64" suffix and go on with relocation
> > > > >>processing.
> > > > >Proposed patch attached.
> > > > >
> > > > Looks at though the patch contains a buffer overflow to me, as the
> > > > length of `name` appears to be unbounded, but it's then copied into
> > > > `buf` which has its size limited to 16, all without checking for `l
> > > > >= sizeof buf` until after the copying is done (which might just
> > > > even get optimized out by GCC since it knows `l` can't be larger
> > > > than buf without UB occuring)
> > >
> > > Thanks for the catch! It was a late change I made to avoid
> > > re-iterating but indeed it's wrong. (Note that strlen, etc. can't be
> > > used here because external function calls or even references are not
> > > valid in the context this can be called in; strcmp is a macro that
> > > expands to a static function call.)
> >
> > Updated version.
>
> And the follow-up patch which removes the macro interfaces. Note that
> there's not necessarily any reason these need to be done together.
> Removing the macros is not safe without removing the symbols, due to
> autoconf badness, but once the symbols are gone we're free to choose
> if/when to remove the macros.
As an alternative, maybe we should consider leaving these but only
under explict _LARGEFILE64_SOURCE rather than implicitly via
_GNU_SOURCE for at least one release cycle. This would allow makeshift
fixing of any builds that break by just adding -D_LARGEFILE64_SOURCE
until a proper fix can be applied.
Any preference here?
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.
