From 3a5049042ac06b6ed5e526f331d5378bf7c7b7ed Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Thu, 28 Jan 2021 10:33:45 +0200 Subject: [PATCH] Avoid assertions when testing arm64 cow bug. (#8405) At least in one case the arm64 cow kernel bug test triggers an assert, which is a problem because it cannot be ignored like cases where the bug is found. On older systems (Linux <4.5) madvise fails because MADV_FREE is not supported. We treat these failures as an indication the system is not affected. Fixes #8351, #8406 --- src/server.c | 101 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 33 deletions(-) diff --git a/src/server.c b/src/server.c index 763e3f5df..a0badea87 100644 --- a/src/server.c +++ b/src/server.c @@ -5200,7 +5200,7 @@ static int smapsGetSharedDirty(unsigned long addr) { FILE *f; f = fopen("/proc/self/smaps", "r"); - serverAssert(f); + if (!f) return -1; while (1) { if (!fgets(buf, sizeof(buf), f)) @@ -5211,8 +5211,8 @@ static int smapsGetSharedDirty(unsigned long addr) { in_mapping = from <= addr && addr < to; if (in_mapping && !memcmp(buf, "Shared_Dirty:", 13)) { - ret = sscanf(buf, "%*s %d", &val); - serverAssert(ret == 1); + sscanf(buf, "%*s %d", &val); + /* If parsing fails, we remain with val == -1 */ break; } } @@ -5226,23 +5226,33 @@ static int smapsGetSharedDirty(unsigned long addr) { * kernel is affected. * The bug was fixed in commit ff1712f953e27f0b0718762ec17d0adb15c9fd0b * titled: "arm64: pgtable: Ensure dirty bit is preserved across pte_wrprotect()" - * Return 1 if the kernel seems to be affected, and 0 otherwise. */ + * Return -1 on unexpected test failure, 1 if the kernel seems to be affected, + * and 0 otherwise. */ int linuxMadvFreeForkBugCheck(void) { - int ret, pipefd[2]; + int ret, pipefd[2] = { -1, -1 }; pid_t pid; - char *p, *q, bug_found = 0; - const long map_size = 3 * 4096; + char *p = NULL, *q; + int bug_found = 0; + long page_size = sysconf(_SC_PAGESIZE); + long map_size = 3 * page_size; /* Create a memory map that's in our full control (not one used by the allocator). */ p = mmap(NULL, map_size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); - serverAssert(p != MAP_FAILED); + if (p == MAP_FAILED) { + serverLog(LL_WARNING, "Failed to mmap(): %s", strerror(errno)); + return -1; + } - q = p + 4096; + q = p + page_size; /* Split the memory map in 3 pages by setting their protection as RO|RW|RO to prevent * Linux from merging this memory map with adjacent VMAs. */ - ret = mprotect(q, 4096, PROT_READ | PROT_WRITE); - serverAssert(!ret); + ret = mprotect(q, page_size, PROT_READ | PROT_WRITE); + if (ret < 0) { + serverLog(LL_WARNING, "Failed to mprotect(): %s", strerror(errno)); + bug_found = -1; + goto exit; + } /* Write to the page once to make it resident */ *(volatile char*)q = 0; @@ -5251,8 +5261,16 @@ int linuxMadvFreeForkBugCheck(void) { #ifndef MADV_FREE #define MADV_FREE 8 #endif - ret = madvise(q, 4096, MADV_FREE); - serverAssert(!ret); + ret = madvise(q, page_size, MADV_FREE); + if (ret < 0) { + /* MADV_FREE is not available on older kernels that are presumably + * not affected. */ + if (errno == EINVAL) goto exit; + + serverLog(LL_WARNING, "Failed to madvise(): %s", strerror(errno)); + bug_found = -1; + goto exit; + } /* Write to the page after being marked for freeing, this is supposed to take * ownership of that page again. */ @@ -5260,37 +5278,47 @@ int linuxMadvFreeForkBugCheck(void) { /* Create a pipe for the child to return the info to the parent. */ ret = pipe(pipefd); - serverAssert(!ret); + if (ret < 0) { + serverLog(LL_WARNING, "Failed to create pipe: %s", strerror(errno)); + bug_found = -1; + goto exit; + } /* Fork the process. */ pid = fork(); - serverAssert(pid >= 0); - if (!pid) { - /* Child: check if the page is marked as dirty, expecing 4 (kB). + if (pid < 0) { + serverLog(LL_WARNING, "Failed to fork: %s", strerror(errno)); + bug_found = -1; + goto exit; + } else if (!pid) { + /* Child: check if the page is marked as dirty, page_size in kb. * A value of 0 means the kernel is affected by the bug. */ - if (!smapsGetSharedDirty((unsigned long)q)) + ret = smapsGetSharedDirty((unsigned long) q); + if (!ret) bug_found = 1; + else if (ret == -1) /* Failed to read */ + bug_found = -1; - ret = write(pipefd[1], &bug_found, 1); - serverAssert(ret == 1); - + if (write(pipefd[1], &bug_found, sizeof(bug_found)) < 0) + serverLog(LL_WARNING, "Failed to write to parent: %s", strerror(errno)); exit(0); } else { /* Read the result from the child. */ - ret = read(pipefd[0], &bug_found, 1); - serverAssert(ret == 1); + ret = read(pipefd[0], &bug_found, sizeof(bug_found)); + if (ret < 0) { + serverLog(LL_WARNING, "Failed to read from child: %s", strerror(errno)); + bug_found = -1; + } /* Reap the child pid. */ - serverAssert(waitpid(pid, NULL, 0) == pid); + waitpid(pid, NULL, 0); } +exit: /* Cleanup */ - ret = close(pipefd[0]); - serverAssert(!ret); - ret = close(pipefd[1]); - serverAssert(!ret); - ret = munmap(p, map_size); - serverAssert(!ret); + if (pipefd[0] != -1) close(pipefd[0]); + if (pipefd[1] != -1) close(pipefd[1]); + if (p != NULL) munmap(p, map_size); return bug_found; } @@ -5901,10 +5929,17 @@ int main(int argc, char **argv) { #ifdef __linux__ linuxMemoryWarnings(); #if defined (__arm64__) - if (linuxMadvFreeForkBugCheck()) { - serverLog(LL_WARNING,"WARNING Your kernel has a bug that could lead to data corruption during background save. Please upgrade to the latest stable kernel."); + int ret; + if ((ret = linuxMadvFreeForkBugCheck())) { + if (ret == 1) + serverLog(LL_WARNING,"WARNING Your kernel has a bug that could lead to data corruption during background save. " + "Please upgrade to the latest stable kernel."); + else + serverLog(LL_WARNING, "Failed to test the kernel for a bug that could lead to data corruption during background save. " + "Your system could be affected, please report this error."); if (!checkIgnoreWarning("ARM64-COW-BUG")) { - serverLog(LL_WARNING,"Redis will now exit to prevent data corruption. Note that it is possible to suppress this warning by setting the following config: ignore-warnings ARM64-COW-BUG"); + serverLog(LL_WARNING,"Redis will now exit to prevent data corruption. " + "Note that it is possible to suppress this warning by setting the following config: ignore-warnings ARM64-COW-BUG"); exit(1); } }