I built a FreeBSD image with FORTIFY_SOURCE=2 and ran the regression test suite. There are some test failures: lib/libc/stdio/gets_s_test:n_gt_rmax -> broken: Empty test result or no new line [0.345s] lib/libc/stdio/gets_s_test:n_gt_rmax_handler -> broken: Empty test result or no new line [0.059s] lib/libc/sys/getgroups_test:getgroups_err -> broken: Empty test result or no new line [0.065s] sys/audit/network:recvmsg_failure -> broken: Empty test result or no new line [0.008s] All of them are tests which make sure that various libc functions return an error when given invalid parameters, but SSP checks turn that into a fatal abort. Is that a bug in the FORTIFY_SOURCE integration? Should we build tests with FORTIFY_SOURCE enabled at all?
Kyle, do you have any thoughts on the questions above?
Hmm, I have mixed feelings here. These are all good examples of things designed to take a buffer size and it's not unreasonable for us to let these transgressions slide because the functions are designed acccordingly, on paper, to handle these situations. However, the flip side of that: 1. We're triggering because we *do* know the buffer size, and we know that they're trying to exceed that. How do you tell the difference between a logical error, and someone maybe trying to do something malicious? 2. Following the latter part of that question, if they *are* trying to do something malicious, is it better to err on the side of caution or to rely on the fact that we're testing the correct handling of this scenario? My first instinct is to err on the side of caution and assume that the implementation could be flawed, disable FORTIFY_SOURCE (or at least these specific fortified shims) for these tests so that we have coverage for the scenario that the size is wrong, but still crash out at runtime.
I realize now the second question was more general and I only answered it in a kind-of roundabout way: I think it's good to build tests with FORTIFY_SOURCE enabled in general because they can be a good canary for the FORTIFY implementation doing something wrong. The kind of tests it's likely to break are definitely in the minority, I think, since we're usually not trying to overflow buffers on purpose.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=d98f4f0698ef0c5178882c544b4c38542d4780f0 commit d98f4f0698ef0c5178882c544b4c38542d4780f0 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2026-05-01 02:57:51 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2026-05-01 02:58:48 +0000 ssp: fix our gets_s implementation under _FORTIFY_SOURCE Annex K specifies an interface for handling constraint violations from gets_s, but we previously broke this for some classes of get_s misuse. Provide a more nuanced version that tries to dodge errors that would trigger a constraint handler while still providing value. Notably, we don't want to trigger a failure unless the passed-in length reasonably fits within an RSIZE_MAX, because gets_s will immediately call larger lengths bogus and fail. PR: 294881 Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D56734 include/ssp/stdio.h | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=c46a0b590716144d772eeba83ca88d96ee12c2f1 commit c46a0b590716144d772eeba83ca88d96ee12c2f1 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2026-05-01 02:57:51 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2026-05-01 02:58:48 +0000 build: provide a FORTIFY_SOURCE.<src file> override For native files we can do more minimal fixes to avoid this large of a hammer, but for third party files it may not be worth the effort to try and patch them. NetBSD has the original _FORTIFY_SOURCE implementation that ours is based on, for instance, but tests sourced from there can't do an __ssp_real(foo) without being certain that `foo` actually has a fortified definition. This change does always define _FORTIFY_SOURCE as a result, so gate it on CFLAGS not already containing _FORTIFY_SOURCE definitions. PR: 294881 Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D56733 share/mk/bsd.sys.mk | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=910f78a5143af32dfcb237a463397aa0c31c07df commit 910f78a5143af32dfcb237a463397aa0c31c07df Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2026-05-01 02:57:51 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2026-05-01 02:58:48 +0000 tests: fix remaining test failures under _FORTIFY_SOURCE The getgroups test is a NetBSD tests, so just apply our larger hammer and disable the feature entirely. The audit test can take a more surgical approach and use __ssp_real() appropriately, since it's a local one. PR: 294881 Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D56735 include/ssp/stdio.h | 7 +++++++ lib/libc/tests/sys/Makefile | 1 + tests/sys/audit/network.c | 4 +++- 3 files changed, 11 insertions(+), 1 deletion(-)
A commit in branch stable/15 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=52e2e6bfc31a54e53109978434bc8c43005aa367 commit 52e2e6bfc31a54e53109978434bc8c43005aa367 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2026-05-01 02:57:51 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2026-06-10 04:00:35 +0000 ssp: fix our gets_s implementation under _FORTIFY_SOURCE Annex K specifies an interface for handling constraint violations from gets_s, but we previously broke this for some classes of get_s misuse. Provide a more nuanced version that tries to dodge errors that would trigger a constraint handler while still providing value. Notably, we don't want to trigger a failure unless the passed-in length reasonably fits within an RSIZE_MAX, because gets_s will immediately call larger lengths bogus and fail. PR: 294881 Reviewed by: markj (cherry picked from commit d98f4f0698ef0c5178882c544b4c38542d4780f0) include/ssp/stdio.h | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=13184a69faa700319ab16357cd39708a0e89fc15 commit 13184a69faa700319ab16357cd39708a0e89fc15 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2026-06-17 22:40:46 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2026-06-17 22:40:46 +0000 build: provide a FORTIFY_SOURCE.<src file> override For native files we can do more minimal fixes to avoid this large of a hammer, but for third party files it may not be worth the effort to try and patch them. NetBSD has the original _FORTIFY_SOURCE implementation that ours is based on, for instance, but tests sourced from there can't do an __ssp_real(foo) without being certain that `foo` actually has a fortified definition. This change does always define _FORTIFY_SOURCE as a result, so gate it on CFLAGS not already containing _FORTIFY_SOURCE definitions. This re-applies c46a0b59071614, but without re-defining _FORTIFY_SOURCE needlessly. PR: 294881 Reviewed by: markj, sjg (both previous version) Differential Revision: https://reviews.freebsd.org/D57356 share/mk/bsd.sys.mk | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)