diff --git a/src/debug.c b/src/debug.c index 83f37cd3d..68dcdcc9a 100644 --- a/src/debug.c +++ b/src/debug.c @@ -520,7 +520,7 @@ NULL restartServer(flags,delay); addReplyError(c,"failed to restart the server. Check server logs."); } else if (!strcasecmp(c->argv[1]->ptr,"oom")) { - void *ptr = zmalloc(ULONG_MAX); /* Should trigger an out of memory. */ + void *ptr = zmalloc(SIZE_MAX/2); /* Should trigger an out of memory. */ zfree(ptr); addReply(c,shared.ok); } else if (!strcasecmp(c->argv[1]->ptr,"assert")) { diff --git a/src/listpack_malloc.h b/src/listpack_malloc.h index 3a9050052..a8a81c35e 100644 --- a/src/listpack_malloc.h +++ b/src/listpack_malloc.h @@ -39,8 +39,11 @@ #ifndef LISTPACK_ALLOC_H #define LISTPACK_ALLOC_H #include "zmalloc.h" -#define lp_malloc zmalloc -#define lp_realloc zrealloc +/* We use zmalloc_usable/zrealloc_usable instead of zmalloc/zrealloc + * to ensure the safe invocation of 'zmalloc_usable_size(). + * See comment in zmalloc_usable_size(). */ +#define lp_malloc(sz) zmalloc_usable(sz,NULL) +#define lp_realloc(ptr,sz) zrealloc_usable(ptr,sz,NULL) #define lp_free zfree #define lp_malloc_size zmalloc_usable_size #endif diff --git a/src/module.c b/src/module.c index ae75b3dc2..e29277956 100644 --- a/src/module.c +++ b/src/module.c @@ -516,13 +516,20 @@ void moduleCreateContext(RedisModuleCtx *out_ctx, RedisModule *module, int ctx_f * You should avoid using malloc(). * This function panics if unable to allocate enough memory. */ void *RM_Alloc(size_t bytes) { - return zmalloc(bytes); + /* Use 'zmalloc_usable()' instead of 'zmalloc()' to allow the compiler + * to recognize the additional memory size, which means that modules can + * use the memory reported by 'RM_MallocUsableSize()' safely. In theory this + * isn't really needed since this API can't be inlined (not even for embedded + * modules like TLS (we use function pointers for module APIs), and the API doesn't + * have the malloc_size attribute, but it's hard to predict how smart future compilers + * will be, so better safe than sorry. */ + return zmalloc_usable(bytes,NULL); } /* Similar to RM_Alloc, but returns NULL in case of allocation failure, instead * of panicking. */ void *RM_TryAlloc(size_t bytes) { - return ztrymalloc(bytes); + return ztrymalloc_usable(bytes,NULL); } /* Use like calloc(). Memory allocated with this function is reported in @@ -530,12 +537,12 @@ void *RM_TryAlloc(size_t bytes) { * and in general is taken into account as memory allocated by Redis. * You should avoid using calloc() directly. */ void *RM_Calloc(size_t nmemb, size_t size) { - return zcalloc(nmemb*size); + return zcalloc_usable(nmemb*size,NULL); } /* Use like realloc() for memory obtained with RedisModule_Alloc(). */ void* RM_Realloc(void *ptr, size_t bytes) { - return zrealloc(ptr,bytes); + return zrealloc_usable(ptr,bytes,NULL); } /* Use like free() for memory obtained by RedisModule_Alloc() and @@ -10704,6 +10711,9 @@ size_t RM_MallocSize(void* ptr) { /* Similar to RM_MallocSize, the difference is that RM_MallocUsableSize * returns the usable size of memory by the module. */ size_t RM_MallocUsableSize(void *ptr) { + /* It is safe to use 'zmalloc_usable_size()' to manipulate additional + * memory space, as we guarantee that the compiler can recognize this + * after 'RM_Alloc', 'RM_TryAlloc', 'RM_Realloc', or 'RM_Calloc'. */ return zmalloc_usable_size(ptr); } diff --git a/src/networking.c b/src/networking.c index 4dcdf6bc9..7fa18af48 100644 --- a/src/networking.c +++ b/src/networking.c @@ -134,7 +134,7 @@ client *createClient(connection *conn) { connSetReadHandler(conn, readQueryFromClient); connSetPrivateData(conn, c); } - c->buf = zmalloc(PROTO_REPLY_CHUNK_BYTES); + c->buf = zmalloc_usable(PROTO_REPLY_CHUNK_BYTES, &c->buf_usable_size); selectDb(c,0); uint64_t client_id; atomicGetIncr(server.next_client_id, client_id, 1); @@ -150,7 +150,6 @@ client *createClient(connection *conn) { c->lib_name = NULL; c->lib_ver = NULL; c->bufpos = 0; - c->buf_usable_size = zmalloc_usable_size(c->buf); c->buf_peak = c->buf_usable_size; c->buf_peak_last_reset_time = server.unixtime; c->ref_repl_buf_node = NULL; @@ -701,11 +700,12 @@ void trimReplyUnusedTailSpace(client *c) { if (tail->size - tail->used > tail->size / 4 && tail->used < PROTO_REPLY_CHUNK_BYTES) { + size_t usable_size; size_t old_size = tail->size; - tail = zrealloc(tail, tail->used + sizeof(clientReplyBlock)); + tail = zrealloc_usable(tail, tail->used + sizeof(clientReplyBlock), &usable_size); /* take over the allocation's internal fragmentation (at least for * memory usage tracking) */ - tail->size = zmalloc_usable_size(tail) - sizeof(clientReplyBlock); + tail->size = usable_size - sizeof(clientReplyBlock); c->reply_bytes = c->reply_bytes + tail->size - old_size; listNodeValue(ln) = tail; } @@ -785,9 +785,10 @@ void setDeferredReply(client *c, void *node, const char *s, size_t length) { listDelNode(c->reply,ln); } else { /* Create a new node */ - clientReplyBlock *buf = zmalloc(length + sizeof(clientReplyBlock)); + size_t usable_size; + clientReplyBlock *buf = zmalloc_usable(length + sizeof(clientReplyBlock), &usable_size); /* Take over the allocation's internal fragmentation */ - buf->size = zmalloc_usable_size(buf) - sizeof(clientReplyBlock); + buf->size = usable_size - sizeof(clientReplyBlock); buf->used = length; memcpy(buf->buf, s, length); listNodeValue(ln) = buf; 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; } diff --git a/src/zmalloc.h b/src/zmalloc.h index 8d4c980cc..9ed92407c 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -96,20 +96,20 @@ #define HAVE_DEFRAG #endif -__attribute__((malloc)) void *zmalloc(size_t size); -__attribute__((malloc)) void *zcalloc(size_t size); -__attribute__((malloc)) __attribute((alloc_size(1,2))) void *zcalloc_num(size_t num, size_t size); +__attribute__((malloc,alloc_size(1))) void *zmalloc(size_t size); +__attribute__((malloc,alloc_size(1))) void *zcalloc(size_t size); +__attribute__((malloc,alloc_size(1,2))) void *zcalloc_num(size_t num, size_t size); __attribute__((alloc_size(2))) void *zrealloc(void *ptr, size_t size); -__attribute__((malloc)) void *ztrymalloc(size_t size); -__attribute__((malloc)) void *ztrycalloc(size_t size); +__attribute__((malloc,alloc_size(1))) void *ztrymalloc(size_t size); +__attribute__((malloc,alloc_size(1))) void *ztrycalloc(size_t size); __attribute__((alloc_size(2))) void *ztryrealloc(void *ptr, size_t size); void zfree(void *ptr); -__attribute__((malloc)) __attribute__((alloc_size(1))) void *zmalloc_usable(size_t size, size_t *usable); -__attribute__((malloc)) __attribute__((alloc_size(1))) void *zcalloc_usable(size_t size, size_t *usable); -__attribute__((alloc_size(2))) void *zrealloc_usable(void *ptr, size_t size, size_t *usable); -__attribute__((malloc)) __attribute__((alloc_size(1))) void *ztrymalloc_usable(size_t size, size_t *usable); -__attribute__((malloc)) __attribute__((alloc_size(1))) void *ztrycalloc_usable(size_t size, size_t *usable); -__attribute__((alloc_size(2))) void *ztryrealloc_usable(void *ptr, size_t size, size_t *usable); +void *zmalloc_usable(size_t size, size_t *usable); +void *zcalloc_usable(size_t size, size_t *usable); +void *zrealloc_usable(void *ptr, size_t size, size_t *usable); +void *ztrymalloc_usable(size_t size, size_t *usable); +void *ztrycalloc_usable(size_t size, size_t *usable); +void *ztryrealloc_usable(void *ptr, size_t size, size_t *usable); void zfree_usable(void *ptr, size_t *usable); __attribute__((malloc)) char *zstrdup(const char *s); size_t zmalloc_used_memory(void); @@ -133,7 +133,22 @@ __attribute__((malloc)) void *zmalloc_no_tcache(size_t size); size_t zmalloc_size(void *ptr); size_t zmalloc_usable_size(void *ptr); #else +/* If we use 'zmalloc_usable_size()' to obtain additional available memory size + * and manipulate it, we need to call 'extend_to_usable()' afterwards to ensure + * the compiler recognizes this extra memory. However, if we use the pointer + * obtained from z[*]_usable() family functions, there is no need for this step. */ #define zmalloc_usable_size(p) zmalloc_size(p) + +/* derived from https://github.com/systemd/systemd/pull/25688 + * 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. + * We use a dummy allocator function to tell the compiler that the new size of ptr is newsize. + * The implementation returns the pointer as is; the only reason for its existence is as a conduit for the + * alloc_size 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 */ +__attribute__((alloc_size(2),noinline)) void *extend_to_usable(void *ptr, size_t size); #endif int get_proc_stat_ll(int i, long long *res);