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

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



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


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