From 7091b495a09b8e8b93c1f1dd257fd91fff7023fb Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Tue, 28 Feb 2023 12:02:55 +0200 Subject: Fix possible memory corruption in FLUSHALL when a client watches more than one key (#11854) Avoid calling unwatchAllKeys when running touchAllWatchedKeysInDb (which was unnecessary) This can potentially lead to use-after-free and memory corruption when the next entry pointer held by the watched keys iterator is freed when unwatching all keys of a specific client. found with address sanitizer, added a test which will not always fail (depending on the random dict hashing seed) problem introduced in #9829 (Reids 7.0) Co-authored-by: Oran Agra (cherry picked from commit 18017df7c1407bc025741c64a90f20f4a8098bd2) --- src/multi.c | 6 +++--- tests/unit/multi.tcl | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/multi.c b/src/multi.c index 5249c58bf..5acffdeae 100644 --- a/src/multi.c +++ b/src/multi.c @@ -438,9 +438,9 @@ void touchAllWatchedKeysInDb(redisDb *emptied, redisDb *replaced_with) { } client *c = wk->client; c->flags |= CLIENT_DIRTY_CAS; - /* As the client is marked as dirty, there is no point in getting here - * again for others keys (or keep the memory overhead till EXEC). */ - unwatchAllKeys(c); + /* Note - we could potentially call unwatchAllKeys for this specific client in order to reduce + * the total number of iterations. BUT this could also free the current next entry pointer + * held by the iterator and can lead to use-after-free. */ } } } diff --git a/tests/unit/multi.tcl b/tests/unit/multi.tcl index 63d85d26b..d03ec9af7 100644 --- a/tests/unit/multi.tcl +++ b/tests/unit/multi.tcl @@ -894,6 +894,14 @@ start_server {tags {"multi"}} { r readraw 1 set _ $res } {*CONFIG SET failed*} + + test "Flushall while watching several keys by one client" { + r flushall + r mset a a b b + r watch b a + r flushall + r ping + } } start_server {overrides {appendonly {yes} appendfilename {appendonly.aof} appendfsync always} tags {external:skip}} { -- cgit v1.2.1