Re: [PATCH 2/4] Introduce virFileRewrite for safe file rewrite

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]