gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix config rewrite file handling to make it really atomic (#7824)

Browse Source 
Make sure we handle short writes correctly, sync to disk after writing  and use
rename to make sure the replacement is actually atomic.
In any case of failure old configuration will remain in place.

Also, add some additional logging to make it easier to diagnose rewrite problems.
This commit is contained in:
Uri Shachar 2020-09-25 12:55:45 +03:00 committed by GitHub
parent 0d62caab21
commit c30bd02c9d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 42 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
src/config.c
View File

@ -1552,60 +1552,62 @@ void rewriteConfigRemoveOrphaned(struct rewriteConfigState *state) {
dictReleaseIterator(di); dictReleaseIterator(di);
} }
/* This function overwrites the old configuration file with the new content. /* This function replaces the old configuration file with the new content
* * in an atomic manner.
* 1) The old file length is obtained.
* 2) If the new content is smaller, padding is added.
* 3) A single write(2) call is used to replace the content of the file.
* 4) Later the file is truncated to the length of the new content.
*
* This way we are sure the file is left in a consistent state even if the
* process is stopped between any of the four operations.
* *
* The function returns 0 on success, otherwise -1 is returned and errno * The function returns 0 on success, otherwise -1 is returned and errno
* set accordingly. */ * is set accordingly. */
int rewriteConfigOverwriteFile(char *configfile, sds content) { int rewriteConfigOverwriteFile(char *configfile, sds content) {
int retval = 0; int fd = -1;
int fd = open(configfile,O_RDWR|O_CREAT,0644); int retval = -1;
int content_size = sdslen(content), padding = 0; char tmp_conffile[PATH_MAX];
struct stat sb; const char *tmp_suffix = ".XXXXXX";
sds content_padded; size_t offset = 0;
ssize_t written_bytes = 0;
/* 1) Open the old file (or create a new one if it does not int tmp_path_len = snprintf(tmp_conffile, sizeof(tmp_conffile), "%s%s", configfile, tmp_suffix);
* exist), get the size. */ if (tmp_path_len <= 0 || (unsigned int)tmp_path_len >= sizeof(tmp_conffile)) {
if (fd == -1) return -1; /* errno set by open(). */ serverLog(LL_WARNING, "Config file full path is too long");
if (fstat(fd,&sb) == -1) { errno = ENAMETOOLONG;
close(fd); return retval;
return -1; /* errno set by fstat(). */
} }
/* 2) Pad the content at least match the old file size. */ #ifdef _GNU_SOURCE
content_padded = sdsdup(content); fd = mkostemp(tmp_conffile, O_CLOEXEC);
if (content_size < sb.st_size) { #else
/* If the old file was bigger, pad the content with /* There's a theoretical chance here to leak the FD if a module thread forks & execv in the middle */
* a newline plus as many "#" chars as required. */ fd = mkstemp(tmp_conffile);
padding = sb.st_size - content_size; #endif
content_padded = sdsgrowzero(content_padded,sb.st_size);
content_padded[content_size] = '\n'; if (fd == -1) {
memset(content_padded+content_size+1,'#',padding-1); serverLog(LL_WARNING, "Could not create tmp config file (%s)", strerror(errno));
return retval;
} }
/* 3) Write the new content using a single write(2). */ while (offset < sdslen(content)) {
if (write(fd,content_padded,strlen(content_padded)) == -1) { written_bytes = write(fd, content + offset, sdslen(content) - offset);
retval = -1; if (written_bytes <= 0) {
if (errno == EINTR) continue; /* FD is blocking, no other retryable errors */
serverLog(LL_WARNING, "Failed after writing (%ld) bytes to tmp config file (%s)", offset, strerror(errno));
goto cleanup; goto cleanup;
} }
offset+=written_bytes;
/* 4) Truncate the file to the right length if we used padding. */
if (padding) {
if (ftruncate(fd,content_size) == -1) {
/* Non critical error... */
} }
if (fsync(fd))
serverLog(LL_WARNING, "Could not sync tmp config file to disk (%s)", strerror(errno));
else if (fchmod(fd, 0644) == -1)
serverLog(LL_WARNING, "Could not chmod config file (%s)", strerror(errno));
else if (rename(tmp_conffile, configfile) == -1)
serverLog(LL_WARNING, "Could not rename tmp config file (%s)", strerror(errno));
else {
retval = 0;
serverLog(LL_DEBUG, "Rewritten config file (%s) successfully", configfile);
} }
cleanup: cleanup:
sdsfree(content_padded);
close(fd); close(fd);
if (retval) unlink(tmp_conffile);
return retval; return retval;
} }