[PATCH i-g-t] lib: Inline igt_x86_features() into ifunc resolvers

[Matt Turner]

On Thu, Mar 21, 2024 at 3:03 AM Zbigniew Kempczyński
<zbigniew.kempczynski at intel.com> wrote:
>
> On Wed, Mar 20, 2024 at 11:09:11PM -0400, Matt Turner wrote:
> > On Wed, Mar 13, 2024 at 5:09 AM Zbigniew Kempczyński
> > <zbigniew.kempczynski at intel.com> wrote:
> > >
> > > On Mon, Mar 04, 2024 at 05:16:40PM -0500, Matt Turner wrote:
> > > > Quoting https://sourceware.org/glibc/wiki/GNU_IFUNC
> > > >
> > > > > When LD_BIND_NOW=1 or -Wl,z,now is in effect symbols must be
> > > > > immediately resolved at startup. In cases where an external function
> > > > > call depends needs to be made that may fail if such a call has not
> > > > > been initialized yet (PLT-based relocation which is processed later).
> > > > > For example calling strlen in an IFUNC resolver built with -Wl,z,now
> > > > > may lead to a segfault because the PLT is not yet resolved.
> > > >
> > > > We cannot rely on function calls through the PLT in ifunc resolvers as
> > > > the PLT may not have been initialized yet.
> > > >
> > > > In practice, this causes crashes when igt is linked with -Wl,-z,now or
> > > > when linked with the mold linker.
> > > >
> > > > To avoid this problem, we do two things:
> > > >     1. move igt_x86_features() to igt_x86.h so its definition is
> > > >        available to compilation units that call the function.
> > > >     2. mark the ifunc resolvers with __attribute__((flatten)) to ensure
> > > >        igt_x86_features() is inlined.
> > > >
> > > > Bug: https://bugs.gentoo.org/788625
> > > > Bug: https://bugs.gentoo.org/925348
> > > > Signed-off-by: Matt Turner <mattst88 at gmail.com>
> > >
> > > Hi.
> > >
> > > I started review of your code, but this this touches some linking intrisics
> > > I'm not familiar with yet so I need more time to explore this. I hope
> > > this is not a problem for you.
> >
> > Do you have any questions I can answer? I tried my best to explain the
> > how and the why of the change in the commit message.
>
> I wanted to prepare better for the discussion - I extracted few
> functions on which I'm reproducing segv issue when LD_BIND_NOW=1 is set.
> I confirm, GOT doesn't contain valid function addresses so jump goes
> to nowhere. I'm pretty sure intel_vbd_decode with LD_BIND_NOW=1 only
> reveals the problem, but it lays within linker logic:
>
> # LD_BIND_NOW=1 LD_PRELOAD=./build/lib/libigt.so.0 LD_DEBUG=all ps
>
> ps definitely doesn't call igt_half_to_float() which in turn calls
> unresolved igt_x86_features at plt. It looks linker first tries to
> call all ifunc's to establish addresses, unfortunately it is doing
> this before filling GOT. Backtrace shows we're still in _start
> from /lib64/ld-linux-x86-64.so.2 so we even didn't get a chance
> to get to the main().

Right, exactly. I believe ifuncs are resolved regardless of whether
they're used.

> Before I'll give you my ack (I still want to experiment with this
> one more day) - may you explain what for you need to bind relocs
> immediately? This is slower than lazy binding as it resolves all
> PLTs instead resolving only required symbols.

Gentoo/Hardened uses `-z now` to improve security. By binding upfront,
the loader can mark the GOT as read-only for a security enhancement.
See https://wiki.gentoo.org/wiki/Hardened/Toolchain for more details.

Apparently (according to that page) `-z now` will become the default
in standard Gentoo profiles very soon.