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.
Tags
None
Referenced Files
F105944684: D15283.diff
Sun, Dec 22, 10:33 PM
F105912502: D15283.diff
Sun, Dec 22, 1:16 PM
Unknown Object (File)
Mon, Dec 9, 4:53 AM
Unknown Object (File)
Mon, Dec 2, 12:59 AM
Unknown Object (File)
Mon, Nov 25, 2:36 PM
Unknown Object (File)
Mon, Nov 25, 6:57 AM
Unknown Object (File)
Sun, Nov 24, 4:21 PM
Unknown Object (File)
Sat, Nov 23, 5:32 AM

Details

Summary

Created libc_nossp_pic.a to be used when linking rtld.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 16414
Build 16335: arc lint + arc unit

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

I suggest to make this a separate change.

bdrewery added inline comments.
share/mk/bsd.lib.mk
312

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

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.