From: Siddhesh Poyarekar <siddhesh@gotplt.org>
To: libc-alpha@sourceware.org
Cc: "Adhemerval Zanella" <adhemerval.zanella@linaro.org>,
"Andreas Schwab" <schwab@linux-m68k.org>,
"Carlos O'Donell" <carlos@redhat.com>,
"Florian Weimer" <fweimer@redhat.com>,
"Jakub Jelinek" <jakub@redhat.com>,
"Martin Liška" <mliska@suse.cz>
Subject: [RFC] _FORTIFY_SOURCE strictness
Date: Thu, 7 Apr 2022 11:56:18 +0530 [thread overview]
Message-ID: <d0b28dea-0d61-49bf-aa75-a96fc71e3d07@gotplt.org> (raw)
Hi,
_FORTIFY_SOURCE levels up to 2 rely on static object size computation to
make decisions at compile time to either call a fortified
bounds-checking variant or a regular variant of a function. Because the
size estimates were static and often upper limit estimates, the bounds
were either optimistic or not present at all. This made aggressive
bounds checks such as those in __snprintf_chk[1] or __wcrtomb_chk[2]
safe enough as to have low false positives (where there's no actual
buffer overflow but we have defined stricter requirements in the
fortified functions) and hence pass through uncontested.
This situation has changed with _FORTIFY_SOURCE=3, where it is now
possible to get non-constant expressions for object sizes, providing not
just more precise estimates, but also greater coverage, which appears to
have increased the possibility of such false positives.
This is not limited to the two known examples either; __strncpy_chk for
example will crash if n is greater than the destination buffer size and
is similarly prone to such false positives. One could envision a
situation where an strncpy call is deeply nested and through compiler
advances and attribute annotations, the callsite now gets precise size
expressions for the call and not just an upper limit estimate or a
(size_t)-1.
Of course, good defensive programming practice means that one always
pass n that is within limits of both source and destination bounds in
strncpy but is a runtime check the right place to enforce good practices
as opposed to only crashing on actual overflows?
There appear to be two alternatives out of this situations and I wanted
to build consensus around one of these or even a third alternative that
I hadn't thought of. I of course volunteer to work on the approach we
have consensus on:
Option 1: Do nothing
We could take the stand that it is what it is and that fortification
checks are necessarily stricter than standards allowances to enforce
better application programming practice. In this case I propose we
improve the overflow messages to clearly indicate that we're enforcing a
stricter limitation than the standard requires and not give the
incorrect impression that we _detected_ an overflow.
The downside of this approach is the possibility that some applications
don't fortify beyond level 2, insisting that their usage is safe enough.
At the moment it seems like a reasonably small set of applications are
running into these, so maybe it's OK?
Option 2: Improve checking accuracy at _FORTIFY_SOURCE=3
This would involve making the checking functions smarter, at level 3 and
abort only on actual overflows. This could have a small runtime
overhead in cases like __wcrtomb_chk but a larger one for cases like
__snprintf_chk. It is also a more invasive change since it will involve
changing all implementations. This essentially is similar to
sanitizer-like functionality.
The downside of this approach is the possibility that some applications
don't fortify beyond level 2 because the performance overhead is
unacceptable.
Thoughts? Maybe an Option 3 that's less worse than the above two options?
Thanks,
Siddhesh
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=28989
[2] https://lists.gnu.org/archive/html/bug-ncurses/2022-04/msg00002.html
next reply other threads:[~2022-04-07 6:26 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-07 6:26 Siddhesh Poyarekar [this message]
2022-04-07 10:16 ` Andreas Schwab
2022-04-08 3:24 ` Siddhesh Poyarekar
2022-04-08 2:26 ` Paul Eggert
2022-04-08 3:32 ` Siddhesh Poyarekar
2022-04-08 5:37 ` Florian Weimer
2022-04-08 6:02 ` Siddhesh Poyarekar
2022-04-08 21:07 ` Paul Eggert
2022-04-11 8:02 ` Siddhesh Poyarekar
2022-05-05 18:43 ` [PATCH 0/2] More compliant wcrtomb Siddhesh Poyarekar
2022-05-05 18:43 ` [PATCH 1/2] benchtests: Add wcrtomb microbenchmark Siddhesh Poyarekar
2022-05-06 9:10 ` Florian Weimer
2022-05-06 12:49 ` [committed] " Siddhesh Poyarekar
2022-05-06 12:50 ` [PATCH 1/2] " Adhemerval Zanella
2022-05-06 12:59 ` Siddhesh Poyarekar
2022-05-06 13:20 ` Adhemerval Zanella
2022-05-06 13:26 ` Siddhesh Poyarekar
2022-05-06 13:36 ` Siddhesh Poyarekar
2022-05-06 13:46 ` Adhemerval Zanella
2022-05-05 18:43 ` [PATCH 2/2] wcrtomb: Make behavior POSIX compliant Siddhesh Poyarekar
2022-05-06 9:25 ` Paul Eggert
2022-05-06 13:40 ` Adhemerval Zanella
2022-05-06 13:46 ` Siddhesh Poyarekar
2022-05-06 14:04 ` [PATCH v2] " Siddhesh Poyarekar
2022-05-09 13:22 ` Adhemerval Zanella
2022-05-09 13:35 ` Siddhesh Poyarekar
2022-05-12 13:15 ` [PATCH v3] " Siddhesh Poyarekar
2022-05-13 4:56 ` Paul Eggert
2022-05-13 5:28 ` Paul Eggert
2022-05-13 11:31 ` Siddhesh Poyarekar
2022-05-13 11:38 ` Florian Weimer
2022-05-13 11:51 ` Siddhesh Poyarekar
2022-05-13 12:55 ` Florian Weimer
2022-05-13 12:30 ` Adhemerval Zanella
2022-05-13 13:42 ` Siddhesh Poyarekar
2022-05-13 17:58 ` Paul Eggert
2022-05-13 13:45 ` [committed] " Siddhesh Poyarekar
2022-05-13 8:18 ` [PATCH v3] " Siddhesh Poyarekar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d0b28dea-0d61-49bf-aa75-a96fc71e3d07@gotplt.org \
--to=siddhesh@gotplt.org \
--cc=adhemerval.zanella@linaro.org \
--cc=carlos@redhat.com \
--cc=fweimer@redhat.com \
--cc=jakub@redhat.com \
--cc=libc-alpha@sourceware.org \
--cc=mliska@suse.cz \
--cc=schwab@linux-m68k.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).