Page MenuHomeFreeBSD

Add a build knob for _FORTIFY_SOURCE
Needs ReviewPublic

Authored by kevans on Oct 5 2021, 3:48 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 2:56 PM
Unknown Object (File)
Tue, Apr 16, 5:31 AM
Unknown Object (File)
Feb 21 2024, 7:02 PM
Unknown Object (File)
Feb 17 2024, 8:43 PM
Unknown Object (File)
Jan 16 2024, 12:32 PM
Unknown Object (File)
Dec 23 2023, 11:23 AM
Unknown Object (File)
Dec 12 2023, 2:38 PM
Unknown Object (File)
Oct 3 2023, 12:22 AM

Details

Reviewers
imp
Summary

Default to _FORTIFY_SOURCE=2 if SSP is enabled, otherwise default to
_FORTIFY_SOURCE=0. include/*.h include their ssp/*.h equivalents as
needed based on the knob. Programs and users are allowed to override
FORTIFY_SOURCE in their Makefiles or src.conf/make.conf to force it off.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 41980
Build 38868: arc lint + arc unit

Event Timeline

kevans requested review of this revision.Oct 5 2021, 3:48 AM

I think rtld must never be built with this thing enabled. In particular, if it references some libc symbol, then just that symbol should be used, without any wrappers.

Also I think that libthr should avoid it, but might be it is not as important as for rtld. In particular, did you checked that e.g. cancellation for read(2) still works as intended?

In D32308#729152, @kib wrote:

I think rtld must never be built with this thing enabled. In particular, if it references some libc symbol, then just that symbol should be used, without any wrappers.

Also I think that libthr should avoid it, but might be it is not as important as for rtld. In particular, did you checked that e.g. cancellation for read(2) still works as intended?

rtld was already not building with it by virtue of MK_SSP=no, but I've applied the FORTIFY_SOURCE=0 hammer as well to ensure that one can't turn it on manually when building with something like WITHOUT_SSP=yes FORTIFY_SOURCE=2.

libthr is in the same boat, but turning it on and this primitive test still seems to cancel at the beginning of the read:

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <unistd.h>

#define BUFSIZE 512

int
main(int argc, char *argv[])
{
        char buf[BUFSIZE];
        ssize_t rd;

        pthread_cancel(pthread_self());

        rd = read(STDIN_FILENO, buf, sizeof(buf));
        pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL);
        write(STDOUT_FILENO, buf, rd);

        printf("done\n");

        exit(1);
}

(assuming the anticipated failure mode would have been interposing failed and read() was libc-read, so the cancel would fail and maybe cancel at write() thus disabling cancellation just before it)

That said, I don't feel particularly compelled to leave it to the whim of a user-knob that nobody working on libthr is going to test, so I'll force it off in the next version as it will be in rtld.

Force off for rtld/libthr

This one is OK as is, just one question...

include/unistd.h
364

isn't there a _FORTIFY_SOURCE_read or something that this could be done for? Or is the order wrong for that?

kevans marked an inline comment as done.

Drop read() declaration avoidance; whatever was causing problems before, I
cannot reproduce it today.

des added inline comments.
include/stdio.h
534