Page MenuHomeFreeBSD

Provide libssp based on libc
ClosedPublic

Authored by kevans on Dec 28 2019, 5:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 27, 12:09 PM
Unknown Object (File)
Wed, Dec 25, 3:50 PM
Unknown Object (File)
Tue, Dec 24, 6:40 PM
Unknown Object (File)
Thu, Dec 12, 9:30 PM
Unknown Object (File)
Tue, Dec 10, 11:06 AM
Unknown Object (File)
Thu, Dec 5, 10:03 AM
Unknown Object (File)
Wed, Dec 4, 1:47 PM
Unknown Object (File)
Dec 2 2024, 11:37 PM
Subscribers

Details

Summary

For libssp.so, rebuild stack_protector.c with FORTIFY_SOURCE stubs that just abort built into it.

For libssp_nonshared.a, steal stack_protector_compat.c from ^/lib/libc/secure and massage it to maintain that __stack_chk_fail_local is a hidden symbol.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 28471

Event Timeline

Regenerate with full context... grr...

Remove all the useless version maps; that's not how linker filters work, and the nonshared one was copy-pasto.

Rewrite: libssp now recompiles stack_protector.c and provides the symbols under LIBSSP_1.0 as they're supposed to be compatible.

libssp_nonshared now compiles stack_protector_compat.c instead of the goofiness it was doing before. I've modified it lightly to match what we need in libssp, rather than effectively duplicating the file. I think it's still rather readable...

Address comment by kan- don't bother avoiding the duplication, it's a small object.

The libc/secure changes will get committed independently, as they're a nop that just fix warnings exposed by libssp build (missing declaration, signed vs. unsigned comparison mismatch, directly casting away volatile qualifier)

Provide the missing _FORTIFY_SOURCE symbols, as per kib. These have never been linked against or used as far as we can tell because doing so requires major header surgery to make it happen. We cannot proceed in a reasonable fashion if they do happen to get called, so abort.

This revision is now accepted and ready to land.Dec 28 2019, 11:01 PM

I've created an exp-run for this, PR 242950. The patch in the exp-run is complete removal of gcclibs' libssp from the build and adds an SSP_SUPPORT knob that controls whether libssp libs/tests are built. At the moment it has dependency on MK_SSP so that the defaults remain consistent across the switch, then a later change can remove the dependency and just mark SSP_SUPPORT as BROKEN on mips, default on everywhere else. SSP_SUPPORT specifically controls:

  • libc ssp tests
  • libc linking against libssp_nonshared (possibly going away anyways, but included for completeness)
  • building libssp/libssp_nonshared

because after discussion with emaste, controlling all of this with SSP seems silly -- the verbiage of the option would imply that we just don't use -fstack-protector* when building base, rather than not building any of the libs.

Also adding pfg, who I've now realized also has an interest in libssp.

I am fine with this approach: while we do have an implementation of these functions (GSoC 2015), I don't think FORTIFY_SOURCE solves anything that we don't already do with the strong stack protector. Stubbing the functions out is a quick and practical solution.
We should still consider removing the stubs in a near future: I haven't looked at how linux does it but I think this stuff is not carried by glibc but in GCC libs instead.

Let me add Oliver as he was my student for GSoC 2015.

Two notes to clarify:

  1. the stack protector and FORTIFY_SOURCE are different things, they offer similar protections at different levels, but when implementing them, NetBSD put them together under the SSP blanket: we inherited that mistake.
  2. FORTIFY_SOURCE was never really implemented in our headers so any use we have is an error. Furthermore, it never worked with clang like it did with GCC so I am not sure what the tests we brought form NetBSD actually do.

Since the urgency is deleting the GPL'd library, stubbing is fine, but after that we should just cleanup any trace of FORTIFY_SOURCE.
Musl libc has a particular implementation that is done with inline functions, only in headers: it is GCC-only but it doesn't need to touch the C library. For more details, feel free to ask in private, it doesn't make sense to spam the Differential ;).

op added a subscriber: op.

LGTM

Update based on the patch from the exp-run, with another bug or two in the build pinched. Most of the diff is mechanical replacement of gnu/lib -> lib and libssp_nonshared moving out of lib/libssp.

Also included is a new knob, SSP_SUPPORT, that specifically controls whether libssp is built or not. SSP is forced off if we don't build with SSP_SUPPORT.

This revision now requires review to proceed.Jan 3 2020, 8:39 PM
This revision is now accepted and ready to land.Jan 3 2020, 8:43 PM
emaste added inline comments.
Makefile.inc1
2791

I think the other _SUPPORT knobs have a different meaning - they control whether some optional functionality is used in binaries, e.g. WITHOUT_BZIP2_SUPPORT builds things without linking against libbz2.

I think SSP_SUPPORT actually sounds right (and probably the others all have the wrong sense, and should be something like WITHOUT_BZIP2_USE instead) but it does seem inconsistent. So I don't know what the best answer is, whether this knob should be renamed from SSP_SUPPORT or not.

lib/libssp/Symbol.map
11

Do you know where Linux / glibc puts these? Is our putting them in libssp just a historical accident?

Perhaps we should also add a comment that we believe these have never been used on FreeBSD.

kevans edited the summary of this revision. (Show Details)
  • Knock out the SSP_SUPPORT knob entirely. This is a relatively small part of the build, just do it.
  • Add a SUBDIR_DEPEND_libc on libssp_nonshared instead of another .WAIT
  • "_elf_aux_info" is called "elf_aux_info" when you're not libc, -D_elf_aux_info=elf_aux_info for the libssp build.
  • Add a note about the FORTIFY_SOURCE symbols in the symbol map
This revision now requires review to proceed.Jan 3 2020, 9:54 PM
kevans added inline comments.
lib/libssp/Symbol.map
11

gcc9 still seems to keep them in libssp, so I guess nothing's changed from what we have in base gcclibs.

Let's go for it (pending exp-run) and tidy up / adjust from there.

This revision is now accepted and ready to land.Jan 3 2020, 9:59 PM
kib requested changes to this revision.Jan 4 2020, 4:05 AM
kib added inline comments.
lib/libssp/Makefile
6

Well, if you bump the dso version number, then it makes no sense to keep symbols for binary compatibility. But our promise was that we would never (take it with some skepticism) bump version number for symver-ed dso.

IMO you should keep the existing version and remove libssp.so.0 from obsolete libs. You are providing perfect ABI-compatible replacement.

This revision now requires changes to proceed.Jan 4 2020, 4:05 AM
kevans marked an inline comment as done.

Drop version back to libssp.so.0 and remove corresponding entries from ObsoleteFiles.inc.

lib/libssp/Makefile
6

Sure- sounds reasonable to me.

This revision is now accepted and ready to land.Jan 4 2020, 5:17 AM

Cool.
FWIW, it may make sense to MFC this code: it is under strong copyleft and getting linked with everything.