Fix undefined behavior defined by ASAN (#1451)

Asan now supports making sure you are passing in the correct pointer
type, which seems useful but we can't support it since we pass in an
incorrect pointer in several places. This is most commonly done with
generic free functions, where we simply cast it to the correct type.

It's not a lot of code to clean up, so it seems appropriate to cleanup
instead of disabling the check.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>

src/acl.c

@@ -297,11 +297,6 @@ int ACLListMatchSds(void *a, void *b) {
     return sdscmp(a, b) == 0;
 }
 
-/* Method to free list elements from ACL users password/patterns lists. */
-void ACLListFreeSds(void *item) {
-    sdsfree(item);
-}
-
 /* Method to duplicate list elements from ACL users password/patterns lists. */
 void *ACLListDupSds(void *item) {
     return sdsdup(item);
@@ -374,7 +369,7 @@ aclSelector *ACLCreateSelector(int flags) {
     listSetFreeMethod(selector->patterns, ACLListFreeKeyPattern);
     listSetDupMethod(selector->patterns, ACLListDupKeyPattern);
     listSetMatchMethod(selector->channels, ACLListMatchSds);
-    listSetFreeMethod(selector->channels, ACLListFreeSds);
+    listSetFreeMethod(selector->channels, sdsfreeVoid);
     listSetDupMethod(selector->channels, ACLListDupSds);
     memset(selector->allowed_commands, 0, sizeof(selector->allowed_commands));
 
@@ -445,7 +440,7 @@ user *ACLCreateUser(const char *name, size_t namelen) {
     u->passwords = listCreate();
     u->acl_string = NULL;
     listSetMatchMethod(u->passwords, ACLListMatchSds);
-    listSetFreeMethod(u->passwords, ACLListFreeSds);
+    listSetFreeMethod(u->passwords, sdsfreeVoid);
     listSetDupMethod(u->passwords, ACLListDupSds);
 
     u->selectors = listCreate();
@@ -489,6 +484,11 @@ void ACLFreeUser(user *u) {
     zfree(u);
 }
 
+/* Used for generic free functions. */
+static void ACLFreeUserVoid(void *u) {
+    ACLFreeUser(u);
+}
+
 /* When a user is deleted we need to cycle the active
  * connections in order to kill all the pending ones that
  * are authenticated with such user. */
@@ -2445,12 +2445,12 @@ sds ACLLoadFromFile(const char *filename) {
             c->user = new_user;
         }
 
-        if (user_channels) raxFreeWithCallback(user_channels, (void (*)(void *))listRelease);
-        raxFreeWithCallback(old_users, (void (*)(void *))ACLFreeUser);
+        if (user_channels) raxFreeWithCallback(user_channels, listReleaseVoid);
+        raxFreeWithCallback(old_users, ACLFreeUserVoid);
         sdsfree(errors);
         return NULL;
     } else {
-        raxFreeWithCallback(Users, (void (*)(void *))ACLFreeUser);
+        raxFreeWithCallback(Users, ACLFreeUserVoid);
         Users = old_users;
         errors =
             sdscat(errors, "WARNING: ACL errors detected, no change to the previously active ACL rules was performed");

src/adlist.c

@@ -77,6 +77,12 @@ void listRelease(list *list) {
     zfree(list);
 }
 
+/* Just like listRelease, but takes the list as a (void *).
+ * Useful as generic free callback. */
+void listReleaseVoid(void *l) {
+    listRelease((list *)l);
+}
+
 /* Add a new node to the list, to head, containing the specified 'value'
  * pointer as value.
  *

src/adlist.h

@@ -72,6 +72,7 @@ typedef struct list {
 /* Prototypes */
 list *listCreate(void);
 void listRelease(list *list);
+void listReleaseVoid(void *list);
 void listEmpty(list *list);
 list *listAddNodeHead(list *list, void *value);
 list *listAddNodeTail(list *list, void *value);

src/call_reply.c

@@ -559,7 +559,7 @@ CallReply *callReplyCreateError(sds reply, void *private_data) {
         sdsfree(reply);
     }
     list *deferred_error_list = listCreate();
-    listSetFreeMethod(deferred_error_list, (void (*)(void *))sdsfree);
+    listSetFreeMethod(deferred_error_list, sdsfreeVoid);
     listAddNodeTail(deferred_error_list, sdsnew(err_buff));
     return callReplyCreate(err_buff, deferred_error_list, private_data);
 }

src/db.c

@@ -1193,7 +1193,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
      * are deep copied temporary strings. We must not free them if they are just
      * a shallow copy - a pointer to the actual data in the data structure */
     if (!shallow_copied_list_items) {
-        listSetFreeMethod(keys, (void (*)(void *))sdsfree);
+        listSetFreeMethod(keys, sdsfreeVoid);
     }
 
     /* For main hash table scan or scannable data structure. */

src/defrag.c

@@ -421,7 +421,7 @@ static void activeDefragQuickListNodes(quicklist *ql) {
 static void defragLater(robj *obj) {
     if (!defrag_later) {
         defrag_later = listCreate();
-        listSetFreeMethod(defrag_later, (void (*)(void *))sdsfree);
+        listSetFreeMethod(defrag_later, sdsfreeVoid);
         defrag_later_cursor = 0;
     }
     sds key = sdsdup(objectGetKey(obj));

src/eval.c

@@ -204,7 +204,7 @@ void scriptingInit(int setup) {
      * and we need to free them respectively. */
     lctx.lua_scripts = dictCreate(&shaScriptObjectDictType);
     lctx.lua_scripts_lru_list = listCreate();
-    listSetFreeMethod(lctx.lua_scripts_lru_list, (void (*)(void *))sdsfree);
+    listSetFreeMethod(lctx.lua_scripts_lru_list, sdsfreeVoid);
     lctx.lua_scripts_mem = 0;
 
     luaRegisterServerAPI(lua);
@@ -777,7 +777,7 @@ void ldbInit(void) {
     ldb.conn = NULL;
     ldb.active = 0;
     ldb.logs = listCreate();
-    listSetFreeMethod(ldb.logs, (void (*)(void *))sdsfree);
+    listSetFreeMethod(ldb.logs, sdsfreeVoid);
     ldb.children = listCreate();
     ldb.src = NULL;
     ldb.lines = 0;

src/functions.c

@@ -348,7 +348,7 @@ libraryJoin(functionsLibCtx *functions_lib_ctx_dst, functionsLibCtx *functions_l
             } else {
                 if (!old_libraries_list) {
                     old_libraries_list = listCreate();
-                    listSetFreeMethod(old_libraries_list, (void (*)(void *))engineLibraryFree);
+                    listSetFreeMethod(old_libraries_list, engineLibraryDispose);
                 }
                 libraryUnlink(functions_lib_ctx_dst, old_li);
                 listAddNodeTail(old_libraries_list, old_li);

src/listpack.c

@@ -250,6 +250,12 @@ void lpFree(unsigned char *lp) {
     lp_free(lp);
 }
 
+/* Same as lpFree, but useful for when you are passing the listpack
+ * into a generic free function that expects (void *) */
+void lpFreeVoid(void *lp) {
+    lp_free((unsigned char *)lp);
+}
+
 /* Shrink the memory to fit. */
 unsigned char *lpShrinkToFit(unsigned char *lp) {
     size_t size = lpGetTotalBytes(lp);

src/listpack.h

@@ -56,6 +56,7 @@ typedef struct {
 
 unsigned char *lpNew(size_t capacity);
 void lpFree(unsigned char *lp);
+void lpFreeVoid(void *lp);
 unsigned char *lpShrinkToFit(unsigned char *lp);
 unsigned char *
 lpInsertString(unsigned char *lp, unsigned char *s, uint32_t slen, unsigned char *p, int where, unsigned char **newp);

src/module.c

@@ -10399,7 +10399,7 @@ ValkeyModuleServerInfoData *VM_GetServerInfo(ValkeyModuleCtx *ctx, const char *s
  * context instead of passing NULL. */
 void VM_FreeServerInfo(ValkeyModuleCtx *ctx, ValkeyModuleServerInfoData *data) {
     if (ctx != NULL) autoMemoryFreed(ctx, VALKEYMODULE_AM_INFO, data);
-    raxFreeWithCallback(data->rax, (void (*)(void *))sdsfree);
+    raxFreeWithCallback(data->rax, sdsfreeVoid);
     zfree(data);
 }
 

src/networking.c

@@ -556,7 +556,7 @@ void afterErrorReply(client *c, const char *s, size_t len, int flags) {
     if (c->flag.module) {
         if (!c->deferred_reply_errors) {
             c->deferred_reply_errors = listCreate();
-            listSetFreeMethod(c->deferred_reply_errors, (void (*)(void *))sdsfree);
+            listSetFreeMethod(c->deferred_reply_errors, sdsfreeVoid);
         }
         listAddNodeTail(c->deferred_reply_errors, sdsnewlen(s, len));
         return;

src/replication.c

@@ -282,7 +282,7 @@ void removeReplicaFromPsyncWait(client *replica_main_client) {
 void resetReplicationBuffer(void) {
     server.repl_buffer_mem = 0;
     server.repl_buffer_blocks = listCreate();
-    listSetFreeMethod(server.repl_buffer_blocks, (void (*)(void *))zfree);
+    listSetFreeMethod(server.repl_buffer_blocks, zfree);
 }
 
 int canFeedReplicaReplBuffer(client *replica) {

src/t_stream.c

@@ -54,6 +54,7 @@
 #define STREAM_LISTPACK_MAX_SIZE (1 << 30)
 
 void streamFreeCG(streamCG *cg);
+void streamFreeCGVoid(void *cg);
 void streamFreeNACK(streamNACK *na);
 size_t streamReplyWithRangeFromConsumerPEL(client *c,
                                            stream *s,
@@ -86,8 +87,8 @@ stream *streamNew(void) {
 
 /* Free a stream, including the listpacks stored inside the radix tree. */
 void freeStream(stream *s) {
-    raxFreeWithCallback(s->rax, (void (*)(void *))lpFree);
-    if (s->cgroups) raxFreeWithCallback(s->cgroups, (void (*)(void *))streamFreeCG);
+    raxFreeWithCallback(s->rax, lpFreeVoid);
+    if (s->cgroups) raxFreeWithCallback(s->cgroups, streamFreeCGVoid);
     zfree(s);
 }
 
@@ -2454,6 +2455,11 @@ void streamFreeConsumer(streamConsumer *sc) {
     zfree(sc);
 }
 
+/* Used for generic free functions. */
+static void streamFreeConsumerVoid(void *sc) {
+    streamFreeConsumer((streamConsumer *)sc);
+}
+
 /* Create a new consumer group in the context of the stream 's', having the
  * specified name, last server ID and reads counter. If a consumer group with
  * the same name already exists NULL is returned, otherwise the pointer to the
@@ -2473,11 +2479,16 @@ streamCG *streamCreateCG(stream *s, char *name, size_t namelen, streamID *id, lo
 
 /* Free a consumer group and all its associated data. */
 void streamFreeCG(streamCG *cg) {
-    raxFreeWithCallback(cg->pel, (void (*)(void *))streamFreeNACK);
-    raxFreeWithCallback(cg->consumers, (void (*)(void *))streamFreeConsumer);
+    raxFreeWithCallback(cg->pel, zfree);
+    raxFreeWithCallback(cg->consumers, streamFreeConsumerVoid);
     zfree(cg);
 }
 
+/* Used for generic free functions. */
+void streamFreeCGVoid(void *cg) {
+    streamFreeCG((streamCG *)cg);
+}
+
 /* Lookup the consumer group in the specified stream and returns its
  * pointer, otherwise if there is no such group, NULL is returned. */
 streamCG *streamLookupCG(stream *s, sds groupname) {

src/unit/test_listpack.c

@@ -1184,7 +1184,7 @@ int test_listpackStressWithRandom(int argc, char **argv, int flags) {
     for (i = 0; i < iteration; i++) {
         lp = lpNew(0);
         ref = listCreate();
-        listSetFreeMethod(ref, (void (*)(void *))sdsfree);
+        listSetFreeMethod(ref, sdsfreeVoid);
         len = rand() % 256;
 
         /* Create lists */

src/unit/test_ziplist.c

@@ -645,7 +645,7 @@ int test_ziplistStressWithRandomPayloadsOfDifferentEncoding(int argc, char **arg
     for (i = 0; i < iteration; i++) {
         zl = ziplistNew();
         ref = listCreate();
-        listSetFreeMethod(ref, (void (*)(void *))sdsfree);
+        listSetFreeMethod(ref, sdsfreeVoid);
         len = rand() % 256;
 
         /* Create lists */