summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorOran Agra <oran@redislabs.com>2022-07-13 09:14:38 +0300
committerGitHub <noreply@github.com>2022-07-13 09:14:38 +0300
commit599e59ebc57283f52c60a8de56ec5f44d053109a (patch)
tree6745143949ea51b00b884c5cf2282f0363882a12 /src
parent20af95a99f333d242e13ffae86e89d6c1167d36f (diff)
downloadredis-599e59ebc57283f52c60a8de56ec5f44d053109a.tar.gz
Avoid valgrind fishy value warning on corrupt restore payloads (#10937)
The corrupt dump fuzzer uncovered a valgrind warning saying: ``` ==76370== Argument 'size' of function malloc has a fishy (possibly negative) value: -3744781444216323815 ``` This allocation would have failed (returning NULL) and being handled properly by redis (even before this change), but we also want to silence the valgrind warnings (which are checking that casting to ssize_t produces a non-negative value). The solution i opted for is to explicitly fail these allocations (returning NULL), before even reaching `malloc` (which would have failed and return NULL too). The implication is that we will not be able to support a single allocation of more than 2GB on a 32bit system (which i don't think is a realistic scenario). i.e. i do think we could be facing cases were redis consumes more than 2gb on a 32bit system, but not in a single allocation. The byproduct of this, is that i dropped the overflow assertions, since these will now lead to the same OOM panic we have for failed allocations.
Diffstat (limited to 'src')
-rw-r--r--src/zmalloc.c18
1 files changed, 12 insertions, 6 deletions
diff --git a/src/zmalloc.c b/src/zmalloc.c
index 857baa8ac..dc9d7cbc4 100644
--- a/src/zmalloc.c
+++ b/src/zmalloc.c
@@ -59,14 +59,12 @@ void zlibc_free(void *ptr) {
#ifdef HAVE_MALLOC_SIZE
#define PREFIX_SIZE (0)
-#define ASSERT_NO_SIZE_OVERFLOW(sz)
#else
#if defined(__sun) || defined(__sparc) || defined(__sparc__)
#define PREFIX_SIZE (sizeof(long long))
#else
#define PREFIX_SIZE (sizeof(size_t))
#endif
-#define ASSERT_NO_SIZE_OVERFLOW(sz) assert((sz) + PREFIX_SIZE > (sz))
#endif
/* When using the libc allocator, use a minimum allocation size to match the
@@ -106,7 +104,8 @@ static void (*zmalloc_oom_handler)(size_t) = zmalloc_default_oom;
/* 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) {
- ASSERT_NO_SIZE_OVERFLOW(size);
+ /* 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);
if (!ptr) return NULL;
@@ -149,7 +148,7 @@ void *zmalloc_usable(size_t size, size_t *usable) {
* Currently implemented only for jemalloc. Used for online defragmentation. */
#ifdef HAVE_DEFRAG
void *zmalloc_no_tcache(size_t size) {
- ASSERT_NO_SIZE_OVERFLOW(size);
+ if (size >= SIZE_MAX/2) zmalloc_oom_handler(size);
void *ptr = mallocx(size+PREFIX_SIZE, MALLOCX_TCACHE_NONE);
if (!ptr) zmalloc_oom_handler(size);
update_zmalloc_stat_alloc(zmalloc_size(ptr));
@@ -166,7 +165,8 @@ 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) {
- ASSERT_NO_SIZE_OVERFLOW(size);
+ /* 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);
if (ptr == NULL) return NULL;
@@ -221,7 +221,6 @@ void *zcalloc_usable(size_t size, size_t *usable) {
/* 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) {
- ASSERT_NO_SIZE_OVERFLOW(size);
#ifndef HAVE_MALLOC_SIZE
void *realptr;
#endif
@@ -238,6 +237,13 @@ void *ztryrealloc_usable(void *ptr, size_t size, size_t *usable) {
if (ptr == NULL)
return ztrymalloc_usable(size, usable);
+ /* Possible overflow, return NULL, so that the caller can panic or handle a failed allocation. */
+ if (size >= SIZE_MAX/2) {
+ zfree(ptr);
+ if (usable) *usable = 0;
+ return NULL;
+ }
+
#ifdef HAVE_MALLOC_SIZE
oldsize = zmalloc_size(ptr);
newptr = realloc(ptr,size);