Redis There may be bug in sdsnewlen

99: sh = s_malloc(hdrlen+initlen+1); 100: if (init==SDS_NOINIT) 101: init = NULL; 102: else if (!init) 103: memset(sh, 0, hdrlen+initlen+1); 104: if (sh == NULL) return NULL;

if s_malloc return null and init = null, memset will coredump

it should be: 99: sh = s_malloc(hdrlen+initlen+1); 104: if (sh == NULL) return NULL; 100: if (init==SDS_NOINIT) 101: init = NULL; 102: else if (!init) 103: memset(sh, 0, hdrlen+initlen+1);

Comment From: trevor211

I don't think it is a bug. s_malloc is ensured to return a non-null pointer. s_malloc is defined as zmalloc. https://github.com/redis/redis/blob/47637bea6d12d899f69a6b384ee7b024177006de/src/sdsalloc.h#L43 Inside zmalloc, it first calls malloc, if it returns NULL, zmalloc_oom_handler would be called. https://github.com/redis/redis/blob/47637bea6d12d899f69a6b384ee7b024177006de/src/zmalloc.c#L92 zmalloc_oom_handler would call abort() as you can see. https://github.com/redis/redis/blob/47637bea6d12d899f69a6b384ee7b024177006de/src/zmalloc.c#L80-L87

Comment From: oranagra

This is indeed not a problem due to what @trevor211 said (allocations never return NULL in redis). BTW, note that in redis-server, the zmalloc_default_oom isn't actually used.

zmalloc_set_oom_handler(redisOutOfMemoryHandler);

anyway, since this issue was repeatedly reported so many times, we decided to fix it in sds.c anyway, just to stop more issues being opened (already fixed in 6.0).

thanks.