Page MenuHomeFreeBSD

powerpc: Transition to Secure-PLT, like most other OSs
ClosedPublic

Authored by jhibbits on Jun 11 2019, 3:37 AM.

Details

Summary

PowerPC has two PLT models: BSS-PLT and Secure-PLT. BSS-PLT uses
runtime code generation to generate the PLT stubs. Secure-PLT was
introduced with GCC 4.1 and Binutils 2.17 (base has GCC 4.2.1 and
Binutils 2.17), and is a more secure PLT format, using a read-only
linkage table, with the dynamic linker populating a non-executable
index table.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 24827
Build 23579: arc lint + arc unit

Event Timeline

jhibbits created this revision.Jun 11 2019, 3:37 AM
nwhitehorn added inline comments.Jun 11 2019, 3:40 AM
libexec/rtld-elf/rtld.c
1298

Are these two cases actually different?

share/mk/bsd.cpu.mk
375 ↗(On Diff #58508)

Don't we want to make this the compiler default, rather than adding default CFLAGS here?

jhibbits added inline comments.Jun 11 2019, 3:45 AM
libexec/rtld-elf/rtld.c
1298

No, they're not. We could reuse obj->glink instead. I took ~80% of these changes from NetBSD, and I'm guessing they added another field to the struct just to match the object component name. It really does serve the same purpose as .glink (and might even actually be called that, too..)

share/mk/bsd.cpu.mk
375 ↗(On Diff #58508)

We do want to, but we also need to support compilers where that's not the default (everything up to whenever we officially switch and set the default).

bdragon added inline comments.Jun 11 2019, 8:32 PM
libexec/rtld-elf/powerpc/rtld_start.S
49

comment block will need updating to explain new procedure instead.

jhibbits updated this revision to Diff 58551.Jun 12 2019, 1:21 AM

Address feedback, fix a crash of syncicache() too big for the PLT.

jhibbits added a subscriber: dim.

Add @dim, for the clang part. The clang part will be upstreamed once this is all done and stable.

dim added a comment.Jun 12 2019, 5:07 AM

The clang part LGTM.

jhibbits updated this revision to Diff 58634.Jun 14 2019, 6:58 PM

Fix _ctx_start relocation for abort() call.

nwhitehorn accepted this revision.Jun 17 2019, 8:34 PM
This revision is now accepted and ready to land.Jun 17 2019, 8:34 PM

@jhibbits , for the clang part, seems like your change isn't enough to force secure-plt. I was able to get it work with the following change:

diff --git a/contrib/llvm/lib/Target/PowerPC/PPCSubtarget.cpp b/contrib/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
index 1fdf74549de..9a8f669d894 100644
--- a/contrib/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ b/contrib/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -138,7 +138,7 @@ void PPCSubtarget::initSubtargetFeatures(StringRef CPU, StringRef FS) {
   if (isDarwin())
     HasLazyResolverStubs = true;
 
-  if (TargetTriple.isOSNetBSD() || TargetTriple.isOSOpenBSD())
+  if (TargetTriple.isOSNetBSD() || TargetTriple.isOSOpenBSD() || TargetTriple.isOSFreeBSD())
     SecurePlt = true;
 
   if (HasSPE && IsPPC64)
jhibbits updated this revision to Diff 58936.Jun 24 2019, 3:20 AM

Add private __stack_chk_fail_local() to kernel machdep.c. It's only needed by
32-bit powerpc, not any other target, so it's private to powerpc.

This revision now requires review to proceed.Jun 24 2019, 3:20 AM

@alfredo.junior_eldorado.org.br Thanks, I added that to my latest diff.

contrib/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
141 ↗(On Diff #58936)

I think you might want check the OS version here, like you did on PPC.cpp

bdragon accepted this revision.Jun 24 2019, 3:49 PM

Current version tested on iBook G4, still works.
Current version also tested on rb800 (powerpcspe), works as well.

There's still the issue where ld tends to err on the side of defaulting to bss-plt a lot, but stuff like sh is using secure-plt and not crashing, so I'm happy.

This revision is now accepted and ready to land.Jun 24 2019, 3:49 PM
pfg accepted this revision.Jun 24 2019, 8:10 PM
pfg added a subscriber: pfg.

Very exciting ... will upstream GCC require changes as well?

@pfg Thankfully all we need is to update the ports to include --enable-secureplt in the CONFIGURE_ARGS (will submit that Diff or PR when this lands). Secure-PLT is fully compatible with BSS-PLT, so there's no issue with compatibility.

I think andreast@ and gerald@ may want to be kept in the loop for the GCC update then :).

Off the record: Apache OpenOffice has been working fine for powerpc with modern GCC and it uses some low level functionality related to exception handling in the "bridges" code that may need adapting for LLVM.

This revision was automatically updated to reflect the committed changes.

Is there anything to be done for modern versions of GCC (GCC 8 or later)?

Note: in 2011 config/rs6000/tramp.asm was moved to
libgcc/config/rs6000/tramp.S.

pfg added a comment.Jun 30 2019, 3:56 PM

Is there anything to be done for modern versions of GCC (GCC 8 or later)?

I think we should just change the default for FreeBSD PPC to Secure-PLT, and we will detect any other requirement fairly easily in the ports tree ;).

@gerald newer GCC uses if defined(PIC) already, so no changes needed there. The only change needed is a small patch I'll post tomorrow.

Justin wrote "Secure-PLT is fully compatible with BSS-PLT, so there's no issue with compatibility".

Well, modern ld from devel/powerpc64-binutils exits with an error code when it reports things
like:

QUOTE
bss-plt forced due to /usr/obj/powerpcvtsc_clang_altbinutils/powerpc.powerpc/usr/src/powerpc.powerpc/tmp/usr/lib/crtbeginS.o
END QUOTE

The error code stops buildworld. (The old system binutils' ld does not exit with an error code and so does not stop the build.)

Justin wrote "Secure-PLT is fully compatible with BSS-PLT, so there's no issue with compatibility".
Well, modern ld from devel/powerpc64-binutils exits with an error code when it reports things
like:
QUOTE
bss-plt forced due to /usr/obj/powerpcvtsc_clang_altbinutils/powerpc.powerpc/usr/src/powerpc.powerpc/tmp/usr/lib/crtbeginS.o
END QUOTE
The error code stops buildworld. (The old system binutils' ld does not exit with an error code and so does not stop the build.)

Until the patch in PR 239007 is committed (the correct fix, not the patch I posted), you'll need to add "-msecure-plt" to your CFLAGS. I'm testing another build right now with base GCC to confirm that normal builds do not fail. In the case of a normal build with base GCC, you will need to rebuild the toolchain first.

FYI: I was using devel/powerpc64-binutils with system clang (8.0.0 or now 8.0.1), not gcc.
Changes to gcc will not change the behavior for what I was doing.

Will system-clang use always require separately adding -msecure-plt or some such in
CFLAGS order to be used with devel/powerpc64-binutils and its ld?

markmi_dsl-only.net added a comment.EditedJul 10 2019, 12:58 AM

Using -msecure-plt did no good with system-clang. Despite the -msecure-plt as shown below:

Meta data file /usr/obj/powerpcvtsc_clang_altbinutils/powerpc.powerpc/usr/src/powerpc.powerpc/gnu/lib/csu/crtbeginS.o.meta
CMD cc -target powerpc-unknown-freebsd13.0 --sysroot=/usr/obj/powerpcvtsc_clang_altbinutils/powerpc.powerpc/usr/src/powerpc.powerpc/tmp -B/usr/local/powerpc64-unknown-freebsd13.0/bin/ -O2 -pipe -B/usr/local/powerpc64-unknown-freebsd13.0/bin/ -msecure-plt -DIN_GCC -DHAVE_LD_EH_FRAME_HDR -DDT_CONFIG -DGLIBC=3 -fno-inline-functions -fno-exceptions -fno-zero-initialized-in-bss -fno-asynchronous-unwind-tables -fno-omit-frame-pointer -I/usr/src/contrib/gcclibs/include -I/usr/src/contrib/gcc/config -I/usr/src/contrib/gcc -I. -I/usr/src/gnu/usr.bin/cc/cc_tools -g -std=gnu89 -Qunused-arguments -g0 -DCRT_BEGIN -DCRTSTUFFS_O -DSHARED -fpic -c -o crtbeginS.o /usr/src/contrib/gcc/crtstuff.c

I still got:

  • libc.so.7.full ---

building shared library libc.so.7
/usr/local/powerpc64-unknown-freebsd13.0/bin/ld: bss-plt forced due to /usr/obj/powerpcvtsc_clang_altbinutils/powerpc.powerpc/usr/src/powerpc.powerpc/tmp/usr/lib/crtbeginS.o
cc: error: linker command failed with exit code 1 (use -v to see invocation)

  • [libc.so.7.full] Error code 1

The clang output or control of ld does not meet the criteria /usr/local/powerpc64-unknown-freebsd13.0/bin/ld
uses. In ld:

if (htab->plt_type == PLT_UNSET)

apparently is true and:

ppc_elf_tdata (ibfd)->has_rel16

is apparently false. (I'm keeping this short by being incomplete.)
This leads to:

plt_type = PLT_OLD;

instead of to PLT_NEW being used.

markmi_dsl-only.net added a subscriber: rm.EditedJul 10 2019, 1:44 AM

FYI: The addition to CFLAGS.powerpc did not add -msecure-plt to the cc for linking libc.so.7.full :

Meta data file /usr/obj/powerpcvtsc_clang_altbinutils/powerpc.powerpc/usr/src/powerpc.powerpc/lib/libc/libc.so.7.full.meta
CMD at echo building shared library libc.so.7
CMD at rm -f libc.so.7 libc.so
CMD cc -target powerpc-unknown-freebsd13.0 --sysroot=/usr/obj/powerpcvtsc_clang_altbinutils/powerpc.powerpc/usr/src/powerpc.powerpc/tmp -B/usr/local/powerpc64-unknown-freebsd13.0/bin/ -Wl,--secure-plt -nodefaultlibs -Wl,--version-script=Version.map -shared -Wl,-x -Wl,--fatal-warnings -Wl,--warn-shared-textrel -o libc.so.7.full -Wl,-soname,libc.so.7 `NM='/usr/local/powerpc64-unknown-freebsd13.0/bin/nm' NMFLAGS='' lorder . . .
. . .
CWD /usr/obj/powerpcvtsc_clang_altbinutils/powerpc.powerpc/usr/src/powerpc.powerpc/lib/libc
TARGET libc.so.7.full
command output - -
building shared library libc.so.7
/usr/local/powerpc64-unknown-freebsd13.0/bin/ld: bss-plt forced due to /usr/obj/powerpcvtsc_clang_altbinutils/powerpc.powerpc/usr/src/powerpc.powerpc/tmp/usr/lib/crtbeginS.o
cc: error: linker command failed with exit code 1 (use -v to see invocation)

Note added later: Sorry for my ignorance of the notational consequences of some pasted text. I've made some adjustments to avoid them.