summaryrefslogtreecommitdiff
path: root/src/zmalloc.c
diff options
context:
space:
mode:
authorsundb <sundbcn@gmail.com>2023-04-11 01:38:40 +0800
committerGitHub <noreply@github.com>2023-04-10 20:38:40 +0300
commite0b378d22b8eb53a5ce87f7b8d36ee63aafa4131 (patch)
tree0e00a11926b6dca95c179627b90ff36b58757c6f /src/zmalloc.c
parente55568edb58c1437d6a84e1baa103dcf5b6153ef (diff)
downloadredis-e0b378d22b8eb53a5ce87f7b8d36ee63aafa4131.tar.gz
Use dummy allocator to make accesses defined as per standard (#11982)
## Issue When we use GCC-12 later or clang 9.0 later to build with `-D_FORTIFY_SOURCE=3`, we can see the following buffer overflow: ``` === REDIS BUG REPORT START: Cut & paste starting from here === 6263:M 06 Apr 2023 08:59:12.915 # Redis 255.255.255 crashed by signal: 6, si_code: -6 6263:M 06 Apr 2023 08:59:12.915 # Crashed running the instruction at: 0x7f03d59efa7c ------ STACK TRACE ------ EIP: /lib/x86_64-linux-gnu/libc.so.6(pthread_kill+0x12c)[0x7f03d59efa7c] Backtrace: /lib/x86_64-linux-gnu/libc.so.6(+0x42520)[0x7f03d599b520] /lib/x86_64-linux-gnu/libc.so.6(pthread_kill+0x12c)[0x7f03d59efa7c] /lib/x86_64-linux-gnu/libc.so.6(raise+0x16)[0x7f03d599b476] /lib/x86_64-linux-gnu/libc.so.6(abort+0xd3)[0x7f03d59817f3] /lib/x86_64-linux-gnu/libc.so.6(+0x896f6)[0x7f03d59e26f6] /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x2a)[0x7f03d5a8f76a] /lib/x86_64-linux-gnu/libc.so.6(+0x1350c6)[0x7f03d5a8e0c6] src/redis-server 127.0.0.1:25111(+0xd5e80)[0x557cddd3be80] src/redis-server 127.0.0.1:25111(feedReplicationBufferWithObject+0x78)[0x557cddd3c768] src/redis-server 127.0.0.1:25111(replicationFeedSlaves+0x1a4)[0x557cddd3cbc4] src/redis-server 127.0.0.1:25111(+0x8721a)[0x557cddced21a] src/redis-server 127.0.0.1:25111(call+0x47a)[0x557cddcf38ea] src/redis-server 127.0.0.1:25111(processCommand+0xbf4)[0x557cddcf4aa4] src/redis-server 127.0.0.1:25111(processInputBuffer+0xe6)[0x557cddd22216] src/redis-server 127.0.0.1:25111(readQueryFromClient+0x3a8)[0x557cddd22898] src/redis-server 127.0.0.1:25111(+0x1b9134)[0x557cdde1f134] src/redis-server 127.0.0.1:25111(aeMain+0x119)[0x557cddce5349] src/redis-server 127.0.0.1:25111(main+0x466)[0x557cddcd6716] /lib/x86_64-linux-gnu/libc.so.6(+0x29d90)[0x7f03d5982d90] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80)[0x7f03d5982e40] src/redis-server 127.0.0.1:25111(_start+0x25)[0x557cddcd7025] ``` The main reason is that when FORTIFY_SOURCE is enabled, GCC or clang will enhance some common functions, such as `strcpy`, `memcpy`, `fgets`, etc, so that they can detect buffer overflow errors and stop program execution, thus improving the safety of the program. We use `zmalloc_usable_size()` everywhere to use memory blocks, but that is an abuse since the malloc_usable_size() isn't meant for this kind of use, it is for diagnostics only. That is also why the behavior is flaky when built with _FORTIFY_SOURCE, the compiler can sense that we reach outside the allocated block and SIGABRT. ### Solution If we need to use the additional memory we got, we need to use a dummy realloc with `alloc_size` attribute and no inlining, (see `extend_to_usable`) to let the compiler see the large of memory we need to use. This can either be an implicit call inside `z*usable` that returns the size, so that the caller doesn't have any other worry, or it can be a normal zmalloc call which means that if the caller wants to use zmalloc_usable_size it must also use extend_to_usable. ### Changes This PR does the following: 1) rename the current z[try]malloc_usable family to z[try]malloc_internal and don't expose them to users outside zmalloc.c, 2) expose a new set of `z[*]_usable` family that use z[*]_internal and `extend_to_usable()` implicitly, the caller gets the size of the allocation and it is safe to use. 3) go over all the users of `zmalloc_usable_size` and convert them to use the `z[*]_usable` family if possible. 4) in the places where the caller can't use `z[*]_usable` and store the real size, and must still rely on zmalloc_usable_size, we still make sure that the allocation used `z[*]_usable` (which has a call to `extend_to_usable()`) and ignores the returning size, this way a later call to `zmalloc_usable_size` is still safe. [4] was done for module.c and listpack.c, all the others places (sds, reply proto list, replication backlog, client->buf) are using [3]. Co-authored-by: Oran Agra <oran@redislabs.com>
Diffstat (limited to 'src/zmalloc.c')
-rw-r--r--src/zmalloc.c78
1 files changed, 65 insertions, 13 deletions
diff --git a/src/zmalloc.c b/src/zmalloc.c
index e7b426277..08ec64fc4 100644
--- a/src/zmalloc.c
+++ b/src/zmalloc.c
@@ -102,9 +102,16 @@ static void zmalloc_default_oom(size_t size) {
static void (*zmalloc_oom_handler)(size_t) = zmalloc_default_oom;
+#ifdef HAVE_MALLOC_SIZE
+void *extend_to_usable(void *ptr, size_t size) {
+ UNUSED(size);
+ return ptr;
+}
+#endif
+
/* Try allocating memory, and return NULL if failed.
* '*usable' is set to the usable size if non NULL. */
-void *ztrymalloc_usable(size_t size, size_t *usable) {
+static inline void *ztrymalloc_usable_internal(size_t size, size_t *usable) {
/* Possible overflow, return NULL, so that the caller can panic or handle a failed allocation. */
if (size >= SIZE_MAX/2) return NULL;
void *ptr = malloc(MALLOC_MIN_SIZE(size)+PREFIX_SIZE);
@@ -123,24 +130,39 @@ void *ztrymalloc_usable(size_t size, size_t *usable) {
#endif
}
+void *ztrymalloc_usable(size_t size, size_t *usable) {
+ size_t usable_size = 0;
+ void *ptr = ztrymalloc_usable_internal(size, &usable_size);
+#ifdef HAVE_MALLOC_SIZE
+ ptr = extend_to_usable(ptr, usable_size);
+#endif
+ if (usable) *usable = usable_size;
+ return ptr;
+}
+
/* Allocate memory or panic */
void *zmalloc(size_t size) {
- void *ptr = ztrymalloc_usable(size, NULL);
+ void *ptr = ztrymalloc_usable_internal(size, NULL);
if (!ptr) zmalloc_oom_handler(size);
return ptr;
}
/* Try allocating memory, and return NULL if failed. */
void *ztrymalloc(size_t size) {
- void *ptr = ztrymalloc_usable(size, NULL);
+ void *ptr = ztrymalloc_usable_internal(size, NULL);
return ptr;
}
/* Allocate memory or panic.
* '*usable' is set to the usable size if non NULL. */
void *zmalloc_usable(size_t size, size_t *usable) {
- void *ptr = ztrymalloc_usable(size, usable);
+ size_t usable_size = 0;
+ void *ptr = ztrymalloc_usable_internal(size, &usable_size);
if (!ptr) zmalloc_oom_handler(size);
+#ifdef HAVE_MALLOC_SIZE
+ ptr = extend_to_usable(ptr, usable_size);
+#endif
+ if (usable) *usable = usable_size;
return ptr;
}
@@ -165,7 +187,7 @@ void zfree_no_tcache(void *ptr) {
/* Try allocating memory and zero it, and return NULL if failed.
* '*usable' is set to the usable size if non NULL. */
-void *ztrycalloc_usable(size_t size, size_t *usable) {
+static inline void *ztrycalloc_usable_internal(size_t size, size_t *usable) {
/* Possible overflow, return NULL, so that the caller can panic or handle a failed allocation. */
if (size >= SIZE_MAX/2) return NULL;
void *ptr = calloc(1, MALLOC_MIN_SIZE(size)+PREFIX_SIZE);
@@ -184,6 +206,16 @@ void *ztrycalloc_usable(size_t size, size_t *usable) {
#endif
}
+void *ztrycalloc_usable(size_t size, size_t *usable) {
+ size_t usable_size = 0;
+ void *ptr = ztrycalloc_usable_internal(size, &usable_size);
+#ifdef HAVE_MALLOC_SIZE
+ ptr = extend_to_usable(ptr, usable_size);
+#endif
+ if (usable) *usable = usable_size;
+ return ptr;
+}
+
/* Allocate memory and zero it or panic.
* We need this wrapper to have a calloc compatible signature */
void *zcalloc_num(size_t num, size_t size) {
@@ -193,35 +225,40 @@ void *zcalloc_num(size_t num, size_t size) {
zmalloc_oom_handler(SIZE_MAX);
return NULL;
}
- void *ptr = ztrycalloc_usable(num*size, NULL);
+ void *ptr = ztrycalloc_usable_internal(num*size, NULL);
if (!ptr) zmalloc_oom_handler(num*size);
return ptr;
}
/* Allocate memory and zero it or panic */
void *zcalloc(size_t size) {
- void *ptr = ztrycalloc_usable(size, NULL);
+ void *ptr = ztrycalloc_usable_internal(size, NULL);
if (!ptr) zmalloc_oom_handler(size);
return ptr;
}
/* Try allocating memory, and return NULL if failed. */
void *ztrycalloc(size_t size) {
- void *ptr = ztrycalloc_usable(size, NULL);
+ void *ptr = ztrycalloc_usable_internal(size, NULL);
return ptr;
}
/* Allocate memory or panic.
* '*usable' is set to the usable size if non NULL. */
void *zcalloc_usable(size_t size, size_t *usable) {
- void *ptr = ztrycalloc_usable(size, usable);
+ size_t usable_size = 0;
+ void *ptr = ztrycalloc_usable_internal(size, &usable_size);
if (!ptr) zmalloc_oom_handler(size);
+#ifdef HAVE_MALLOC_SIZE
+ ptr = extend_to_usable(ptr, usable_size);
+#endif
+ if (usable) *usable = usable_size;
return ptr;
}
/* Try reallocating memory, and return NULL if failed.
* '*usable' is set to the usable size if non NULL. */
-void *ztryrealloc_usable(void *ptr, size_t size, size_t *usable) {
+static inline void *ztryrealloc_usable_internal(void *ptr, size_t size, size_t *usable) {
#ifndef HAVE_MALLOC_SIZE
void *realptr;
#endif
@@ -275,24 +312,39 @@ void *ztryrealloc_usable(void *ptr, size_t size, size_t *usable) {
#endif
}
+void *ztryrealloc_usable(void *ptr, size_t size, size_t *usable) {
+ size_t usable_size = 0;
+ ptr = ztryrealloc_usable_internal(ptr, size, &usable_size);
+#ifdef HAVE_MALLOC_SIZE
+ ptr = extend_to_usable(ptr, usable_size);
+#endif
+ if (usable) *usable = usable_size;
+ return ptr;
+}
+
/* Reallocate memory and zero it or panic */
void *zrealloc(void *ptr, size_t size) {
- ptr = ztryrealloc_usable(ptr, size, NULL);
+ ptr = ztryrealloc_usable_internal(ptr, size, NULL);
if (!ptr && size != 0) zmalloc_oom_handler(size);
return ptr;
}
/* Try Reallocating memory, and return NULL if failed. */
void *ztryrealloc(void *ptr, size_t size) {
- ptr = ztryrealloc_usable(ptr, size, NULL);
+ ptr = ztryrealloc_usable_internal(ptr, size, NULL);
return ptr;
}
/* Reallocate memory or panic.
* '*usable' is set to the usable size if non NULL. */
void *zrealloc_usable(void *ptr, size_t size, size_t *usable) {
- ptr = ztryrealloc_usable(ptr, size, usable);
+ size_t usable_size = 0;
+ ptr = ztryrealloc_usable(ptr, size, &usable_size);
if (!ptr && size != 0) zmalloc_oom_handler(size);
+#ifdef HAVE_MALLOC_SIZE
+ ptr = extend_to_usable(ptr, usable_size);
+#endif
+ if (usable) *usable = usable_size;
return ptr;
}