From 49a5ae485df2a862c8da19175c4df65e5449e7ea Mon Sep 17 00:00:00 2001
From: Siddhesh Poyarekar <siddhesh@gotplt.org>
Date: Tue, 13 Dec 2022 16:54:36 -0500
Subject: [PATCH] Use dummy allocator to make accesses defined as per standard
 
systemd uses malloc_usable_size() everywhere to use memory blocks
obtained through malloc, but that is abuse since the
malloc_usable_size() interface isn't meant for this kind of use, it is
for diagnostics only.  This is also why systemd behaviour is flaky when
built with _FORTIFY_SOURCE.
 
One way to make this more standard (and hence safer) is to, at every
malloc_usable_size() call, also 'reallocate' the block so that the
compiler can see the larger size.  This is done through a dummy
reallocator whose only purpose is to tell the compiler about the larger
usable size, it doesn't do any actual reallocation.
 
Florian Weimer pointed out that this doesn't solve the problem of an
allocator potentially growing usable size at will, which will break the
implicit assumption in systemd use that the value returned remains
constant as long as the object is valid.  The safest way to fix that is
for systemd to step away from using malloc_usable_size() like this.
 
Resolves #22801.
 
---
 src/basic/alloc-util.c              |  4 +++
 src/basic/alloc-util.h              | 38 +++++++++++++++++++++--------
 src/fundamental/macro-fundamental.h |  1 +
 3 files changed, 33 insertions(+), 10 deletions(-)
 
diff --git a/src/basic/alloc-util.c b/src/basic/alloc-util.c
index b030f45..6063943 100644
--- a/src/basic/alloc-util.c
+++ b/src/basic/alloc-util.c
@@ -102,3 +102,7 @@ void* greedy_realloc0(
 
         return q;
 }
+
+void *expand_to_usable(void *ptr, size_t newsize _unused_) {
+        return ptr;
+}
diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h
index 3c33308..e01ea72 100644
--- a/src/basic/alloc-util.h
+++ b/src/basic/alloc-util.h
@@ -2,6 +2,7 @@
 #pragma once
 
 #include <alloca.h>
+#include <malloc.h>
 #include <stddef.h>
 #include <stdlib.h>
 #include <string.h>
@@ -171,17 +172,34 @@ void* greedy_realloc0(void **p, size_t need, size_t size);
 #  define msan_unpoison(r, s)
 #endif
 
-/* This returns the number of usable bytes in a malloc()ed region as per malloc_usable_size(), in a way that
- * is compatible with _FORTIFY_SOURCES. If _FORTIFY_SOURCES is used many memory operations will take the
- * object size as returned by __builtin_object_size() into account. Hence, let's return the smaller size of
- * malloc_usable_size() and __builtin_object_size() here, so that we definitely operate in safe territory by
- * both the compiler's and libc's standards. Note that __builtin_object_size() evaluates to SIZE_MAX if the
- * size cannot be determined, hence the MIN() expression should be safe with dynamically sized memory,
- * too. Moreover, when NULL is passed malloc_usable_size() is documented to return zero, and
- * __builtin_object_size() returns SIZE_MAX too, hence we also return a sensible value of 0 in this corner
- * case. */
+/* Dummy allocator to tell the compiler that the new size of p is newsize. The implementation returns the
+ * pointer as is; the only reason for its existence is as a conduit for the _alloc_ attribute. This cannot be
+ * a static inline because gcc then loses the attributes on the function.
+ * See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96503 */
+void *expand_to_usable(void *p, size_t newsize) _alloc_(2) _returns_nonnull_;
+
+static inline size_t malloc_sizeof_safe(void **xp) {
+        if (_unlikely_(!xp || !*xp))
+                return 0;
+
+        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
+         * clear that expand_to_usable won't return NULL.
+         * See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79265 */
+        if (!*xp)
+                assert_not_reached("");
+        return sz;
+}
+
+/* This returns the number of usable bytes in a malloc()ed region as per malloc_usable_size(), which may
+ * return a value larger than the size that was actually allocated. Access to that additional memory is
+ * discouraged because it violates the C standard; a compiler cannot see that this as valid. To help the
+ * compiler out, the MALLOC_SIZEOF_SAFE macro 'allocates' the usable size using a dummy allocator function
+ * expand_to_usable. There is a possibility of malloc_usable_size() returning different values during the
+ * lifetime of an object, which may cause problems, but the glibc allocator does not do that at the moment. */
 #define MALLOC_SIZEOF_SAFE(x) \
-        MIN(malloc_usable_size(x), __builtin_object_size(x, 0))
+        malloc_sizeof_safe((void**) &__builtin_choose_expr(__builtin_constant_p(x), (void*) { NULL }, (x)))
 
 /* Inspired by ELEMENTSOF() but operates on malloc()'ed memory areas: typesafely returns the number of items
  * that fit into the specified memory block */
diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h
index 9675186..2a2c0f8 100644
--- a/src/fundamental/macro-fundamental.h
+++ b/src/fundamental/macro-fundamental.h
@@ -14,6 +14,7 @@
 #define _used_ __attribute__((__used__))
 #define _unused_ __attribute__((__unused__))
 #define _cleanup_(x) __attribute__((__cleanup__(x)))
+#define _returns_nonnull_ __attribute__((__returns_nonnull__))
 
 #define XSTRINGIFY(x) #x
 #define STRINGIFY(x) XSTRINGIFY(x)
-- 
2.33.0
 
