[Libguestfs] [PATCH] daemon: Fix read-file so it fails gracefully for large files (RHBZ#589039).
Richard W.M. Jones
rjones at redhat.com
Fri May 7 12:35:08 UTC 2010
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
-------------- next part --------------
>From 9a503ec7d7fa16e09abab53cc7f768b72020ce08 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Fri, 7 May 2010 13:30:42 +0100
Subject: [PATCH] daemon: Fix read-file so it fails gracefully for large files (RHBZ#589039).
Pengzhen Cao noticed that read-file would fail for files
larger than the protocol size; this is *not* the bug. However
it would also lose protocol synchronization after this.
The reason was that functions which return RBufferOut in the
generator must not 'touch' the *size_r parameter along error
return paths.
I fixed read-file and initrd-cat, and I checked that pread was
doing the right thing.
This also adds regression tests for read-file with various categories
of large file.
---
daemon/file.c | 14 ++++++++++----
daemon/initrd.c | 12 ++++++++----
src/generator.ml | 15 ++++++++++++++-
3 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/daemon/file.c b/daemon/file.c
index 7a0f8f9..2399828 100644
--- a/daemon/file.c
+++ b/daemon/file.c
@@ -336,25 +336,24 @@ do_read_file (const char *path, size_t *size_r)
return NULL;
}
- *size_r = statbuf.st_size;
/* The actual limit on messages is smaller than this. This
* check just limits the amount of memory we'll try and allocate
* here. If the message is larger than the real limit, that will
* be caught later when we try to serialize the message.
*/
- if (*size_r >= GUESTFS_MESSAGE_MAX) {
+ if (statbuf.st_size >= GUESTFS_MESSAGE_MAX) {
reply_with_error ("%s: file is too large for the protocol, use guestfs_download instead", path);
close (fd);
return NULL;
}
- r = malloc (*size_r);
+ r = malloc (statbuf.st_size);
if (r == NULL) {
reply_with_perror ("malloc");
close (fd);
return NULL;
}
- if (xread (fd, r, *size_r) == -1) {
+ if (xread (fd, r, statbuf.st_size) == -1) {
reply_with_perror ("read: %s", path);
close (fd);
free (r);
@@ -367,6 +366,10 @@ do_read_file (const char *path, size_t *size_r)
return NULL;
}
+ /* Mustn't touch *size_r until we are sure that we won't return any
+ * error (RHBZ#589039).
+ */
+ *size_r = statbuf.st_size;
return r;
}
@@ -418,6 +421,9 @@ do_pread (const char *path, int count, int64_t offset, size_t *size_r)
return NULL;
}
+ /* Mustn't touch *size_r until we are sure that we won't return any
+ * error (RHBZ#589039).
+ */
*size_r = r;
return buf;
}
diff --git a/daemon/initrd.c b/daemon/initrd.c
index addeb14..932564b 100644
--- a/daemon/initrd.c
+++ b/daemon/initrd.c
@@ -142,25 +142,24 @@ do_initrd_cat (const char *path, const char *filename, size_t *size_r)
goto cleanup;
}
- *size_r = statbuf.st_size;
/* The actual limit on messages is smaller than this. This
* check just limits the amount of memory we'll try and allocate
* here. If the message is larger than the real limit, that will
* be caught later when we try to serialize the message.
*/
- if (*size_r >= GUESTFS_MESSAGE_MAX) {
+ if (statbuf.st_size >= GUESTFS_MESSAGE_MAX) {
reply_with_error ("%s:%s: file is too large for the protocol",
path, filename);
goto cleanup;
}
- ret = malloc (*size_r);
+ ret = malloc (statbuf.st_size);
if (ret == NULL) {
reply_with_perror ("malloc");
goto cleanup;
}
- if (xread (fd, ret, *size_r) == -1) {
+ if (xread (fd, ret, statbuf.st_size) == -1) {
reply_with_perror ("read: %s:%s", path, filename);
free (ret);
ret = NULL;
@@ -175,6 +174,11 @@ do_initrd_cat (const char *path, const char *filename, size_t *size_r)
}
fd = -1;
+ /* Mustn't touch *size_r until we are sure that we won't return any
+ * error (RHBZ#589039).
+ */
+ *size_r = statbuf.st_size;
+
cleanup:
if (fd >= 0)
close (fd);
diff --git a/src/generator.ml b/src/generator.ml
index cbed94b..9dd8f8c 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -3272,7 +3272,20 @@ for full details.");
("read_file", (RBufferOut "content", [Pathname "path"]), 150, [ProtocolLimitWarning],
[InitISOFS, Always, TestOutputBuffer (
- [["read_file"; "/known-4"]], "abc\ndef\nghi")],
+ [["read_file"; "/known-4"]], "abc\ndef\nghi");
+ (* Test various near large, large and too large files (RHBZ#589039). *)
+ InitBasicFS, Always, TestLastFail (
+ [["touch"; "/a"];
+ ["truncate_size"; "/a"; "4194303"]; (* GUESTFS_MESSAGE_MAX - 1 *)
+ ["read_file"; "/a"]]);
+ InitBasicFS, Always, TestLastFail (
+ [["touch"; "/a"];
+ ["truncate_size"; "/a"; "4194304"]; (* GUESTFS_MESSAGE_MAX *)
+ ["read_file"; "/a"]]);
+ InitBasicFS, Always, TestLastFail (
+ [["touch"; "/a"];
+ ["truncate_size"; "/a"; "41943040"]; (* GUESTFS_MESSAGE_MAX * 10 *)
+ ["read_file"; "/a"]])],
"read a file",
"\
This calls returns the contents of the file C<path> as a
--
1.6.6.1
More information about the Libguestfs
mailing list