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