Page MenuHomeFreeBSD

Prototype WITH_PIE knob
ClosedPublic

Authored by emaste on Dec 3 2018, 6:04 PM.

Details

Summary

Add WITH_PIE knob to build position-independent binaries

  • kerberos5/tools/asn1_compile and kerberos5/tools/slc/Makefile link non-internal/private libroken.a explicitly; set MK_PIE=no for them
  • build private and internal libs as PIC when WITH_PIE is set
  • never build i386 boot components as PIE

Diff Detail

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

Event Timeline

emaste created this revision.Dec 3 2018, 6:04 PM
emaste added a comment.Dec 3 2018, 6:06 PM

Presumably we should build INTERNALLIBs and PRIVATELIBs as both libfoo.a and libfoo_pic.a (or some similar scheme) and choose the appropriate one when linking the binary. I don't know how far down that path we want to go though.

kib added a comment.Dec 3 2018, 6:24 PM

Presumably we should build INTERNALLIBs and PRIVATELIBs as both libfoo.a and libfoo_pic.a (or some similar scheme) and choose the appropriate one when linking the binary. I don't know how far down that path we want to go though.

Yes, I argued that this is the only correct way. We must not build normal libXXX.a with PIC, but we do need libXXX_pic.a. Also, formally architectures can have different -fPIC and -fPIE ABIs, so perhaps we really need libXXX_pie.a, and e.g. libc_pic.a and libc_pie.a simultaneously.

emaste added a comment.Dec 3 2018, 7:01 PM

Also, formally architectures can have different -fPIC and -fPIE ABIs, so perhaps we really need libXXX_pie.a, and e.g. libc_pic.a and libc_pie.a simultaneously.

Do you have a reference for this? At least for any architecture relevant to us today I don't think this is a practical case, so perhaps we ought to build _pic.a for PIE use for now, with a comment in bsd.lib.mk?

For reference I see PRIVATELIB or INTERNALLIB under:

  • gnu/usr.bin/binutils
  • gnu/usr.bin/cc
  • gnu/usr.bin/gdb
  • kerberos5/lib
  • lib/atf
  • lib/clang
  • lib/libbsdstat
  • lib/libdevdctl
  • lib/libelftc
  • lib/libevent
  • lib/libifconfig
  • lib/libldns
  • lib/libnetbsd
  • lib/libopenbsd
  • lib/libpe
  • lib/libpmcstat
  • lib/libsm
  • lib/libsmdb
  • lib/libsmutil
  • lib/libsqlite3
  • lib/libtelnet
  • lib/libucl
  • lib/libunbound
  • lib/libzstd
  • rescue
  • sbin/ipf
  • secure/lib/libssh
  • stand/liblua
  • stand/usb
  • tools/tools/net80211
  • usr.sbin/svn/lib
  • usr.sbin/amd
  • usr.sbin/bsnmpd
  • usr.sbin/cron
  • usr.sbin/fifolog
  • usr.sbin/lpr
  • usr.sbin/ntp

Multiple subdirectories under some of these elided, there are a total of 69.

We could of course build _pie.a or _pic.a versions of these libs only if the WITH_PIE knob is set. We could even build only the _pie.a or _pic.a version when set.

kib added a comment.Dec 3 2018, 7:47 PM

Also, formally architectures can have different -fPIC and -fPIE ABIs, so perhaps we really need libXXX_pie.a, and e.g. libc_pic.a and libc_pie.a simultaneously.

Do you have a reference for this? At least for any architecture relevant to us today I don't think this is a practical case, so perhaps we ought to build _pic.a for PIE use for now, with a comment in bsd.lib.mk?

-fPIE code does not need to redirect all its external symbol accesses through GOT/PLT. This does not matter for x86 at compile time, only at the link time, but it might give an opportunity to optimize for other arches. Also the default TLS model might be different.

emaste added a comment.Dec 3 2018, 8:08 PM

This does not matter for x86 at compile time, only at the link time

Right, in particular I wonder if any architectures will do something different at compile time -- we need _pie.a vs _pic.a only if so.

emaste updated this revision to Diff 52229.Dec 21 2018, 3:42 PM

Share the INSTALL_PIC_ARCHIVE logic to install libXXX_pic.a versions of INTERNALLIBs and PRIVATELIBs.

emaste added inline comments.Dec 21 2018, 3:50 PM
kerberos5/tools/asn1_compile/Makefile
9 ↗(On Diff #52229)

We could instead build a libroken_pic.a and link against it, but it doesn't seem worth the effort.

share/mk/bsd.lib.mk
72–79 ↗(On Diff #52229)

Moving this is not necessary now, will move back.

248 ↗(On Diff #52229)

Perhaps rearrange this file to move SOBJS+= out of the INTERNALLIB block, but I think it's easier to leave it this way for development and review.

emaste added inline comments.
share/mk/bsd.lib.mk
252 ↗(On Diff #52229)

Will remove PRIVATELIB from here, it should be redundant (PRIVATELIBs are installed .sos).

321 ↗(On Diff #52229)

and remove PRIVATELIB from here.

In D18423#392118, @kib wrote:

Presumably we should build INTERNALLIBs and PRIVATELIBs as both libfoo.a and libfoo_pic.a (or some similar scheme) and choose the appropriate one when linking the binary. I don't know how far down that path we want to go though.

Yes, I argued that this is the only correct way. We must not build normal libXXX.a with PIC, but we do need libXXX_pic.a. Also, formally architectures can have different -fPIC and -fPIE ABIs, so perhaps we really need libXXX_pie.a, and e.g. libc_pic.a and libc_pie.a simultaneously.

What architectures would require having both a _pic.a and _pie.a? In a userland address space that addresses less than 48 bits (32-bit archs, fbsd/riscv), address space randomization is effectively useless. So, really, amd64, arm64, and mips64 are currently the only relevant architectures with respect to address space randomization and position-independent executables. If FreeBSD were to use SV64 instead of SV39 for riscv, then riscv would be in the picture, too.

This is suboptimal - consider this test code:

int global_variable;

int
lib_function1(int a)
{
        return (a + global_variable);
}

int
lib_function2(int a)
{
        return (lib_function1(a));
}

-fPIC causes Clang to emit a GOT access to global_variable and a PLT call to lib_function1 while -fPIE omits GOT and PLT use. (By comparison GCC emits GOT and PLT use in both cases.)

Demo: https://github.com/emaste/pic-pie
Clang diff: https://people.freebsd.org/~emaste/pic-pie/clang.html
GCC diff: https://people.freebsd.org/~emaste/pic-pie/gcc.html

So we'll want to build a separate _pie.a lib.

emaste updated this revision to Diff 53880.Feb 13 2019, 5:52 PM

Compile objs with -fpie or -fPIE and use _pie suffix instead of reusing _pic build infrastructure as generated code may be different. (PIE objects do not need to support symbol interposition and thus avoid GOT/PLT use.)

emaste added inline comments.Feb 13 2019, 5:58 PM
libexec/rtld-elf/Makefile
11 ↗(On Diff #53880)

Probably should have a comment explaining why this is here -- it's because rtld is already built PIC using bespoke logic.

share/mk/bsd.lib.mk
135 ↗(On Diff #53880)

I'm cheating here and have omitted -DPIC from the C++ case, because Clang has function arguments named PIC. Probably the right approach is to leave -DPIC here and add CXXFLAGS+=-UPIC in the Makefile for Clang libs.

share/mk/bsd.opts.mk
64 ↗(On Diff #53880)

Added to __DEFAULT_YES_OPTIONS in my tree for testing, would be disabled by default for commit (at least initially).

usr.bin/clang/clang.prog.mk
14–15 ↗(On Diff #53880)

Maybe just use MK_PIE:= no for now

usr.bin/svn/Makefile.inc
41–43 ↗(On Diff #53880)

Maybe just MK_PIE:=no for now

dim added inline comments.Feb 13 2019, 6:33 PM
share/mk/bsd.lib.mk
135 ↗(On Diff #53880)

In my opinion, we should stop defining PIC, and just use __PIC__ instead, which is automatically defined by the compiler when you compile in PIC mode. This kind of identifier can always clash with contributed code.

emaste added a subscriber: cem.Feb 13 2019, 9:41 PM
emaste added inline comments.Feb 14 2019, 2:01 PM
share/mk/bsd.lib.mk
135 ↗(On Diff #53880)

Sounds good to me. I'll leave it out of the C++ .pieo case.

emaste updated this revision to Diff 53917.Feb 14 2019, 2:03 PM

Just turn off PIE for clang and svn (their Makefiles explicitly reference .a archives and need invasive changes to add ${PIE_SUFFIX} - that may come in a later commit but this smaller change is easier to review.

I believe this is ready to commit now (disabled by default); commit message:

Add WITH_PIE knob to build base system as Position Independent Executables

Building binaries as PIE allows the executable itself to be loaded at a
random address with ASLR enabled, not just its shared libraries.

PIE objects have a .pieo extension and INTERNALLIB libraries 
libXXX_pie.a.

MK_PIE is disabled for some kerberos5 tools, Clang, and Subversion, as 
they explicitly reference .a libraries in their Makefiles.  These can 
be addressed on an individual basis later.  MK_PIE is also disabled for
rtld-elf because it is already position-independent using bespoke
Makefile rules.
kib added a comment.Feb 15 2019, 3:10 PM

I believe this is ready to commit now (disabled by default); commit message:

Do static binaries work ? I suspect they do not, or they work by a chance. Most scary is init(8) of course, but other static binaries esp. rescue(8) are also questionable.

In D18423#410852, @kib wrote:

Do static binaries work ? I suspect they do not, or they work by a chance. Most scary is init(8) of course, but other static binaries esp. rescue(8) are also questionable.

Untested, the intent is that the knob applies only to dynamically linked binaries in this iteration. There was indeed a bug in bsd.prog.mk though, will be updated shortly.

emaste updated this revision to Diff 53967.Feb 15 2019, 3:28 PM

fix bsd.prog.mk

kib accepted this revision.Feb 15 2019, 7:06 PM
This revision is now accepted and ready to land.Feb 15 2019, 7:06 PM
This revision was automatically updated to reflect the committed changes.