Page MenuHomeFreeBSD

Created static pic libc with no SSP to be used by rtld
ClosedPublic

Authored by lffpires_ruabrasil.org on May 3 2018, 9:14 PM.

Details

Summary

Created libc_nossp_pic.a to be used when linking rtld.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This looks fine to me, in particular the rtld changes. But I am not competent to review the build changes.

lib/libc/Makefile
204 ↗(On Diff #42104)

I suggest to make this a separate change.

bdrewery added inline comments.
share/mk/bsd.lib.mk
312 ↗(On Diff #42104)

I don't think we need the MK_TOOLCHAIN check here.

This revision is now accepted and ready to land.May 3 2018, 10:39 PM

This seems fine though I'm not super excited about compiling all of libc yet another time.

share/mk/bsd.lib.mk
312 ↗(On Diff #42104)

It looks like it's just a copy of the case above.

This seems fine though I'm not super excited about compiling all of libc yet another time.

Same. The benefit seems low so far if it is just removing a small hack from rtld. Perhaps a limited libc without all of the stuff rtld obviously doesn't need.

Also I suspect this is happening because lib/libc/secure/stack_protector.c was recently updated. We could easily use some ifdefs in that file and directly build it again from rtld with a different -DRTLD define to use a different implementation. Thus fixing having the logic in 2 separate files like before.

Is there something more coming where libc_nossp_pic will be useful?

This seems fine though I'm not super excited about compiling all of libc yet another time.

Same. The benefit seems low so far if it is just removing a small hack from rtld. Perhaps a limited libc without all of the stuff rtld obviously doesn't need.

Also I suspect this is happening because lib/libc/secure/stack_protector.c was recently updated. We could easily use some ifdefs in that file and directly build it again from rtld with a different -DRTLD define to use a different implementation. Thus fixing having the logic in 2 separate files like before.

Is there something more coming where libc_nossp_pic will be useful?

I diagree with the characterization of the issue as small or as a hack. Rtld is both very important and very different from the normal runtime environment so that its correctness must be maintained properly. I would rather argue that the removed code is the hack (which I am guilty of). I do think that libc_nossp_pic is worth it, at least because that it provides the fix which does not depend on the implementation details of the stack protector.

Also, I suggest to splut the change into two parts. One should be the modifications to the build infrastructure, and the other the rtld changes.

  • Separated libc restriction- and rtld-related changes
  • Removed unnecessary MK_TOOLCHAIN check
This revision now requires review to proceed.May 7 2018, 4:53 PM

Made changes according to review comments.

If the reviewers feel that this is a good approach, this revision is now ready to be committed.

I'll post 2 separate revisions for the following 2 changes:
Removed -fstack-protector-all restriction from libc (introduced in rS180012).

Removed rtld-private implementations of stack_chk_guard,
stack_chk_fail() and __chk_fail() symbols (introduced in rS232861), since rtld
is now using libc_nossp_pic.a, which doesn't have SSP.

2 follow up revisions created:
D15340 (Removed -fstack-protector-all restriction from libc)
D15341 (Updated rtld to use libc_nossp_pic.a)

This revision is now accepted and ready to land.May 7 2018, 5:14 PM
This revision was automatically updated to reflect the committed changes.