From a7179e75705aab90b155d0512d7e8e0fffd60383 Mon Sep 17 00:00:00 2001 From: Moti Cohen Date: Wed, 23 Feb 2022 17:59:22 +0200 Subject: [PATCH] Sentinel: fix a free-after-use issue re-registering Sentinels. (#10333) In case HELLO message received from another sentinel, with same address like another instance registered in the past but with different runid. Then there was cumbersome logic to modify the instance the port to 0 to in order to mark as invalid and later on to delete it. But the deletion is happening during update of instances in such a way that we might end up accessing an instance that was deleted just before. Didn't find a good reason why to postpone the deletion action of an obsolete instance (deletion is taking place instantly, for other cases ) -> Lets delete at once There is a mixture of logic of Sentinel address update with the logic of deletion of Sentinels that match a given Address -> Split to two! --- src/sentinel.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/sentinel.c b/src/sentinel.c index 2342c0c00..89aad023d 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -1190,10 +1190,6 @@ int sentinelUpdateSentinelAddressInAllMasters(sentinelRedisInstance *ri) { if (match->link->pc != NULL) instanceLinkCloseConnection(match->link,match->link->pc); - /* Remove any sentinel with port number set to 0 */ - if (match->addr->port == 0) - dictDelete(master->sentinels,match->name); - if (match == ri) continue; /* Address already updated for it. */ /* Update the address of the matching Sentinel by copying the address @@ -2878,9 +2874,22 @@ void sentinelProcessHelloMessage(char *hello, int hello_len) { getSentinelRedisInstanceByAddrAndRunID( master->sentinels, token[0],port,NULL); if (other) { + /* If there is already other sentinel with same address (but + * different runid) then remove the old one across all masters */ sentinelEvent(LL_NOTICE,"+sentinel-invalid-addr",other,"%@"); - other->addr->port = 0; /* It means: invalid address. */ - sentinelUpdateSentinelAddressInAllMasters(other); + dictIterator *di; + dictEntry *de; + + /* Keep a copy of runid. 'other' about to be deleted in loop. */ + sds runid_obsolete = sdsnew(other->runid); + + di = dictGetIterator(sentinel.masters); + while((de = dictNext(di)) != NULL) { + sentinelRedisInstance *master = dictGetVal(de); + removeMatchingSentinelFromMaster(master, runid_obsolete); + } + dictReleaseIterator(di); + sdsfree(runid_obsolete); } }