mirrors
/
redis
mirror of https://mirror.osredm.com/root/redis.git
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix PSYNC crash with wrong offset (#10243) 

`PSYNC replicationid str_offset` will crash the server.

The reason is in `masterTryPartialResynchronization`,
we will call `getLongLongFromObjectOrReply` check the
offset. With a wrong offset, it will add a reply and
then trigger a full SYNC and the client become a replica.

So crash in `c->bufpos == 0 && listLength(c->reply) == 0`.
In this commit, we check the psync_offset before entering
function `masterTryPartialResynchronization`, and return.

Regardless of that crash, accepting the sync, but also replying
with an error would have corrupt the replication stream.
This commit is contained in:
Binbin 2022-02-06 19:13:56 +08:00 committed by GitHub
parent 6ebb679f06
commit 344e41c922
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 10 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/replication.c
View File

@ -705,18 +705,12 @@ int replicationSetupSlaveForFullResync(client *slave, long long offset) {
* *
* On success return C_OK, otherwise C_ERR is returned and we proceed * On success return C_OK, otherwise C_ERR is returned and we proceed
* with the usual full resync. */ * with the usual full resync. */
int masterTryPartialResynchronization(client *c) { int masterTryPartialResynchronization(client *c, long long psync_offset) {
long long psync_offset, psync_len; long long psync_len;
char *master_replid = c->argv[1]->ptr; char *master_replid = c->argv[1]->ptr;
char buf[128]; char buf[128];
int buflen; int buflen;
/* Parse the replication offset asked by the slave. Go to full sync
* on parse error: this should never happen but we try to handle
* it in a robust way compared to aborting. */
if (getLongLongFromObjectOrReply(c,c->argv[2],&psync_offset,NULL) !=
C_OK) goto need_full_resync;
/* Is the replication ID of this master the same advertised by the wannabe /* Is the replication ID of this master the same advertised by the wannabe
* slave via PSYNC? If the replication ID changed this master has a * slave via PSYNC? If the replication ID changed this master has a
* different replication history, and there is no way to continue. * different replication history, and there is no way to continue.
@ -977,7 +971,14 @@ void syncCommand(client *c) {
* So the slave knows the new replid and offset to try a PSYNC later * So the slave knows the new replid and offset to try a PSYNC later
* if the connection with the master is lost. */ * if the connection with the master is lost. */
if (!strcasecmp(c->argv[0]->ptr,"psync")) { if (!strcasecmp(c->argv[0]->ptr,"psync")) {
if (masterTryPartialResynchronization(c) == C_OK) { long long psync_offset;
if (getLongLongFromObjectOrReply(c, c->argv[2], &psync_offset, NULL) != C_OK) {
serverLog(LL_WARNING, "Replica %s asks for synchronization but with a wrong offset",
replicationGetSlaveName(c));
return;
}
if (masterTryPartialResynchronization(c, psync_offset) == C_OK) {
server.stat_sync_partial_ok++; server.stat_sync_partial_ok++;
return; /* No full resync needed, return. */ return; /* No full resync needed, return. */
} else { } else {