On 10/19/2011 11:26 AM, Jiri Denemark wrote:
When saving config files we just overwrite old content of the file. In case something fails during that process (e.g. disk gets full) we lose both old and new content. This patch makes the process more robust by writing the new content into a separate file and only if that succeeds the original file is atomically replaced with the new one. --- src/libvirt_private.syms | 1 + src/util/virfile.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 6 +++++ 3 files changed, 63 insertions(+), 0 deletions(-)
+virFileRewrite(const char *path, + mode_t mode, + virFileRewriteFunc rewrite, + void *opaque) +{ + char *newfile = NULL; + int fd = -1; + int ret = -1; + + if (virAsprintf(&newfile, "%s.new", path)< 0) { + virReportOOMError(); + goto cleanup; + } + + if ((fd = open(newfile, O_WRONLY | O_CREAT | O_TRUNC, mode))< 0) {
Should we be using O_EXCL instead of O_TRUNC, and insisting that no .new file already exists, so as to protect ourselves from symlink attacks where someone installs a symlink and tricks us into truncating a random file?
+ + if (rename(newfile, path)< 0) { + virReportSystemError(errno, _("cannot rename file '%s' as '%s'"), + newfile, path); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FORCE_CLOSE(fd); + if (newfile) { + unlink(newfile);
This fails if the rename() succeeds, or worse, it succeeds at unlinking an unrelated file that someone created in the window between our rename and this unlink. We should probably only attempt the unlink if ret < 0.
-- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list