alloc-util: make malloc_sizeof_safe() compatible with clang's _FORTIFY_SOURCE=3
Turns out that clang's interprocedural analysis is quite smart and can look through our expand_to_usable() trick, which then causes crashes with _FORTIFY_SOURCE=3. clang's interprocedural analysis can see that expand_to_usable() simply returns its first argument, so it replaces all uses of the return value with that argument (the original realloc() result). This effectively bypasses expand_to_usable()'s alloc_size attribute, causing the fortify check to use the (smaller) size from realloc() instead, which eventually leads to a false-positive buffer overflow: $ build/test-varlink-idl /* test_parse_format */ ... *** buffer overflow detected ***: terminated Aborted (core dumped) build/test-varlink-idl This is not an issue with gcc (at least not yet), since gcc sees expand_to_usable() as an opaque user-defined allocation-like function and simply trusts the alloc_size attribute that comes with it. To fix this, let's add a simple no-op barrier to malloc_sizeof_safe() that clobbers the input pointer, which causes __builtin_dynamic_object_size() to return (size_t)-1 - this is interpreted as an "unknown" size by the following fortify check which is then skipped instead of triggering the assertion. Similarly, test-alloc-util now doesn't call malloc_usable_size() directly but instead goes through malloc_sizeof_safe(), so it's also guarded by the barrier. This follows the already established pile of similar workaround for the same class of issues we encountered with gcc, namely [0], which prompted [1], that was later reverted in [2], and then followed by another couple of fixes in [3] and [4]. Resolves: #43178 [0] https://github.com/systemd/systemd/issues/22801 [1] https://github.com/systemd/systemd/commit/0bd292567a543d124cd303f7dd61169a209cae64 [2] https://github.com/systemd/systemd/commit/2cfb790391958ada34284290af1f9ab863a515c7 [3] https://github.com/systemd/systemd/commit/7929e180aa47a2692ad4f053afac2857d7198758 [4] https://github.com/systemd/systemd/commit/4f79f545b3c46c358666c9f5f2b384fe50aac4b4
This commit is contained in:
committed by
Yu Watanabe
parent
21ab2fe6ec
commit
a56d7db48f
@@ -40,6 +40,14 @@ size_t malloc_sizeof_safe(void **xp) {
|
||||
if (_unlikely_(!xp || !*xp))
|
||||
return 0;
|
||||
|
||||
/* Prevent the compiler from retaining a tighter object-size bound on *xp from a prior allocator call
|
||||
* (e.g. realloc()). This adds a simple no-op barrier after which the *xp pointer becomes clobbered,
|
||||
* which forces clang to skip fortify checks. Without this, clang's __builtin_dynamic_object_size()
|
||||
* (used with _FORTIFY_SOURCE=3) keeps the requested-size bound from realloc() and ignores
|
||||
* expand_to_usable()'s __alloc_size__, causing false-positive fortify failures when we zero the
|
||||
* malloc_usable_size() region in realloc0()/greedy_realloc0(). */
|
||||
asm volatile("" : "+r"(*xp));
|
||||
|
||||
size_t sz = malloc_usable_size(*xp);
|
||||
*xp = expand_to_usable(*xp, sz);
|
||||
/* GCC doesn't see the _returns_nonnull_ when built with ubsan, so yet another hint to make it doubly
|
||||
|
||||
@@ -112,7 +112,7 @@ TEST(memdup_multiply_and_greedy_realloc) {
|
||||
assert_se(dup[1] == 2);
|
||||
assert_se(dup[2] == 3);
|
||||
|
||||
memzero(dup + 3, malloc_usable_size(dup) - sizeof(int) * 3);
|
||||
memzero(dup + 3, MALLOC_SIZEOF_SAFE(dup) - sizeof(int) * 3);
|
||||
|
||||
p = dup;
|
||||
assert_se(GREEDY_REALLOC0(dup, 2) == p);
|
||||
|
||||
Reference in New Issue
Block a user