From 8deb6182c692904733544568479035b31fe1007e Mon Sep 17 00:00:00 2001 From: dequis Date: Sun, 8 Apr 2018 15:52:09 -0300 Subject: [PATCH] Make config_write more atomic to prevent truncation when out of space This is modeled after glib's g_file_set_contents. It doesn't use that function directly because the writing is done with GIOChannel streaming-like writes and g_file_set_contents expects the whole thing to be in-memory. Main differences with g_file_set_contents: - complete lack of win32 special casing (cygwin/WSL should work though) - no fallocate() (linux only, but we don't know the size upfront, anyway) - always calls fsync (glib skips it on btrfs or when not overwriting) Other than that, it's the same old mkstemp + fsync + rename. --- src/lib-config/write.c | 45 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/lib-config/write.c b/src/lib-config/write.c index 37e51f09..ab2c6975 100644 --- a/src/lib-config/write.c +++ b/src/lib-config/write.c @@ -301,30 +301,67 @@ int config_write(CONFIG_REC *rec, const char *fname, int create_mode) { int ret; int fd; + int save_errno; + char *tmp_name; + const char *dest_name; g_return_val_if_fail(rec != NULL, -1); g_return_val_if_fail(fname != NULL || rec->fname != NULL, -1); g_return_val_if_fail(create_mode != -1 || rec->create_mode != -1, -1); - fd = open(fname != NULL ? fname : rec->fname, + dest_name = fname != NULL ? fname : rec->fname; + tmp_name = g_strdup_printf("%s.XXXXXX", dest_name); + + fd = g_mkstemp_full(tmp_name, O_WRONLY | O_TRUNC | O_CREAT, create_mode != -1 ? create_mode : rec->create_mode); - if (fd == -1) - return config_error(rec, g_strerror(errno)); + if (fd == -1) { + config_error(rec, g_strerror(errno)); + ret = -1; + goto out; + } rec->handle = g_io_channel_unix_new(fd); g_io_channel_set_encoding(rec->handle, NULL, NULL); g_io_channel_set_close_on_unref(rec->handle, TRUE); + rec->tmp_indent_level = 0; rec->tmp_last_lf = TRUE; ret = config_write_block(rec, rec->mainnode, FALSE, TRUE); + save_errno = errno; + if (ret == -1) { /* write error */ - config_error(rec, errno == 0 ? "bug" : g_strerror(errno)); + unlink(tmp_name); + config_error(rec, save_errno == 0 ? "bug" : g_strerror(save_errno)); + goto out; + } + + ret = fsync(fd); + save_errno = errno; + + if (ret == -1) { + unlink(tmp_name); + config_error(rec, g_strerror(errno)); + goto out; } g_io_channel_unref(rec->handle); rec->handle = NULL; + if (rename(tmp_name, dest_name) == -1) { + unlink(tmp_name); + config_error(rec, g_strerror(errno)); + goto out; + } + +out: + if (rec->handle) { + g_io_channel_unref(rec->handle); + rec->handle = NULL; + } + + g_free(tmp_name); + return ret; }