116 lines
5.4 KiB
Diff
116 lines
5.4 KiB
Diff
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
|
|
|