From 644d94558a739656f81343f3b8a12205d53eeeb7 Mon Sep 17 00:00:00 2001 From: Joe Hu Date: Mon, 17 Apr 2023 14:05:36 -0400 Subject: [PATCH] Fix RDB check regression caused by PR 12022 (#12051) The nightly tests showed that the recent PR #12022 caused random failures in aof.tcl on checking RDB preamble inside an AOF file. Root cause: When checking RDB preamble in an AOF file, what's passed into redis_check_rdb is aof_filename, not aof_filepath. The newly introduced isFifo function does not check return status of the stat call and hence uses the uninitailized stat_p object. Fix: 1. Fix isFifo by checking stat call's return code. 2. Pass aof_filepath instead of aof_filename to redis_check_rdb. 3. move the FIFO check to rdb.c since the limitation is the re-opening of the file, and not anything specific about redis-check-rdb. --- src/anet.c | 6 ++++++ src/anet.h | 1 + src/rdb.c | 5 +++++ src/redis-check-aof.c | 2 +- src/redis-check-rdb.c | 13 +------------ 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/anet.c b/src/anet.c index 00c30b83d..790ea7e0a 100644 --- a/src/anet.c +++ b/src/anet.c @@ -697,3 +697,9 @@ int anetSetSockMarkId(char *err, int fd, uint32_t id) { return ANET_OK; #endif } + +int anetIsFifo(char *filepath) { + struct stat sb; + if (stat(filepath, &sb) == -1) return 0; + return S_ISFIFO(sb.st_mode); +} diff --git a/src/anet.h b/src/anet.h index b571e52c1..b13c14f77 100644 --- a/src/anet.h +++ b/src/anet.h @@ -70,5 +70,6 @@ int anetFormatAddr(char *fmt, size_t fmt_len, char *ip, int port); int anetPipe(int fds[2], int read_flags, int write_flags); int anetSetSockMarkId(char *err, int fd, uint32_t id); int anetGetError(int fd); +int anetIsFifo(char *filepath); #endif diff --git a/src/rdb.c b/src/rdb.c index 6ebc0751e..c52d8faf7 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -88,6 +88,11 @@ void rdbReportError(int corruption_error, int linenum, char *reason, ...) { /* If we're loading an rdb file form disk, run rdb check (and exit) */ serverLog(LL_WARNING, "%s", msg); char *argv[2] = {"",rdbFileBeingLoaded}; + if (anetIsFifo(argv[1])) { + /* Cannot check RDB FIFO because we cannot reopen the FIFO and check already streamed data. */ + rdbCheckError("Cannot check RDB that is a FIFO: %s", argv[1]); + return; + } redis_check_rdb_main(2,argv,NULL); } else if (corruption_error) { /* In diskless loading, in case of corrupt file, log and exit. */ diff --git a/src/redis-check-aof.c b/src/redis-check-aof.c index 54d70ae50..616177a8b 100644 --- a/src/redis-check-aof.c +++ b/src/redis-check-aof.c @@ -242,7 +242,7 @@ int checkSingleAof(char *aof_filename, char *aof_filepath, int last_file, int fi } if (preamble) { - char *argv[2] = {NULL, aof_filename}; + char *argv[2] = {NULL, aof_filepath}; if (redis_check_rdb_main(2, argv, fp) == C_ERR) { printf("RDB preamble of AOF file is not sane, aborting.\n"); exit(1); diff --git a/src/redis-check-rdb.c b/src/redis-check-rdb.c index 15d183092..682135e55 100644 --- a/src/redis-check-rdb.c +++ b/src/redis-check-rdb.c @@ -186,15 +186,9 @@ void rdbCheckSetupSignals(void) { sigaction(SIGABRT, &act, NULL); } -static int isFifo(char *filename) { - struct stat stat_p; - stat(filename, &stat_p); - return S_ISFIFO(stat_p.st_mode); -} - /* Check the specified RDB file. Return 0 if the RDB looks sane, otherwise * 1 is returned. - * The file is specified as a filename in 'rdbfilename' if 'fp' is not NULL, + * The file is specified as a filename in 'rdbfilename' if 'fp' is NULL, * otherwise the already open file 'fp' is checked. */ int redis_check_rdb(char *rdbfilename, FILE *fp) { uint64_t dbid; @@ -205,11 +199,6 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) { static rio rdb; /* Pointed by global struct riostate. */ struct stat sb; - if (isFifo(rdbfilename)) { - /* Cannot check RDB over named pipe because fopen blocks until another process opens the FIFO for writing. */ - return 1; - } - int closefile = (fp == NULL); if (fp == NULL && (fp = fopen(rdbfilename,"r")) == NULL) return 1;