From 40c140bf16ca88cc67e6262c813fde1d8ea8242f Mon Sep 17 00:00:00 2001 From: Vitah Lin Date: Thu, 26 Mar 2026 20:52:11 +0800 Subject: [PATCH] Fix heap-use-after-free in restoreCommand when module sets key metadata during notification (#14929) ### Problem When a module's keyspace notification callback calls `RedisModule_SetKeyMeta` to attach metadata for the first time, `kvobjSet` reallocates the kvobj (to add a metadata slot) and frees the old one. The local `kv` pointer in `restoreCommand` becomes dangling, and the subsequent access to `kv->type` reads freed memory. Related PR: https://github.com/redis/redis/pull/14918 ### Changed Save `kv->type` to a local variable before the notification call. --------- Co-authored-by: debing.sun --- src/cluster.c | 9 ++++++--- src/notify.c | 5 ++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index f77b068b64..e9b1a8b5eb 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -295,15 +295,18 @@ void restoreCommand(client *c) { /* Create the key and set the TTL if any */ kvobj *kv = dbAddInternal(c->db, key, &obj, NULL, &keymeta); + /* Save type: kv may be reallocated by module callbacks during notifyKeyspaceEvent below. */ + int kvtype = kv->type; + /* If minExpiredField was set, then the object is hash with expiration * on fields and need to register it in global HFE DS */ - if (kv->type == OBJ_HASH) { + if (kvtype == OBJ_HASH) { uint64_t minExpiredField = hashTypeGetMinExpire(kv, 1); if (minExpiredField != EB_EXPIRE_TIME_INVALID) estoreAdd(c->db->subexpires, getKeySlot(key->ptr), kv, minExpiredField); } - if (kv->type == OBJ_STREAM) { + if (kvtype == OBJ_STREAM) { stream *s = kv->ptr; if (s->idmp_producers != NULL) { if (dictAdd(c->db->stream_idmp_keys, key, NULL) == DICT_OK) @@ -328,7 +331,7 @@ void restoreCommand(client *c) { * destination key existed. */ if (deleted) { notifyKeyspaceEvent(NOTIFY_OVERWRITTEN, "overwritten", key, c->db->id); - if (oldtype != kv->type) { + if (oldtype != kvtype) { notifyKeyspaceEvent(NOTIFY_TYPE_CHANGED, "type_changed", key, c->db->id); } } diff --git a/src/notify.c b/src/notify.c index 84cd85551a..00dd4a0909 100644 --- a/src/notify.c +++ b/src/notify.c @@ -84,7 +84,10 @@ sds keyspaceEventsFlagsToString(int flags) { * 'type' is the notification class we define in `server.h`. * 'event' is a C string representing the event name. * 'key' is a Redis object representing the key name. - * 'dbid' is the database ID where the key lives. */ + * 'dbid' is the database ID where the key lives. + * + * NOTE: This function may invoke module notification callbacks, which may + * cause the key's kvobj to be reallocated. */ void notifyKeyspaceEvent(int type, const char *event, robj *key, int dbid) { sds chan; robj *chanobj, *eventobj;