[Qemu-devel] [PULL for-1.5] qemu-ga CVE-2013-2007 addenda

[Qemu-devel] [PULL for-1.5] qemu-ga CVE-2013-2007 addenda

[Michael Roth]

Hi Anthony,

These are fix-ups for Laszlo's CVE-2013-2007 fix:

http://www.mail-archive.com/qemu-devel@nongnu.org/msg170944.html

The main effect is to avoid cluttering filesystems with empty files if
we hit an error path in the open/create/chmod path.

I'm unable to confirm whether or not these error paths can actually be
triggered in 1.5 or are just theoretical, but I plan to apply these to
1.4.2 to be sure and so I'm also submitting this for 1.5.

If you think it's too late in the cycle to warrant these for 1.5 I can
also cherry-pick them from my QGA tree for 1.4.2 instead.

The following changes since commit 38ebb396c955ceb2ef7e246248ceb7f8bfe1b774:

  target-i386: ROR r8/r16 imm instruction fix (2013-05-10 19:59:54 +0200)

are available in the git repository at:

  http://github.com/mdroth/qemu qga-pull-2013-05-13

for you to fetch changes up to 2b720018060179b394f8ce736983373ab80dd37c:

  qga: unlink just created guest-file if fchmod() or fdopen() fails on it (2013-05-13 09:45:49 -0500)

----------------------------------------------------------------
Laszlo Ersek (2):
      qga: distinguish binary modes in "guest_file_open_modes" map
      qga: unlink just created guest-file if fchmod() or fdopen() fails on it

 qga/commands-posix.c |   25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

[Qemu-devel] [PATCH 1/2] qga: distinguish binary modes in "guest_file_open_modes" map

[Michael Roth]

From: Laszlo Ersek <lersek@redhat.com>

In Windows guests this may make a difference.

Since the original patch (commit c689b4f1) sought to be pedantic and to
consider theoretical corner cases of portability, we should fix it up
where it failed to come through in that pursuit.

Suggested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 04c6951..2eec712 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -242,17 +242,27 @@ static GuestFileHandle *guest_file_handle_find(int64_t id, Error **err)
 
 typedef const char * const ccpc;
 
+#ifndef O_BINARY
+#define O_BINARY 0
+#endif
+
 /* http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html */
 static const struct {
     ccpc *forms;
     int oflag_base;
 } guest_file_open_modes[] = {
-    { (ccpc[]){ "r",  "rb",         NULL }, O_RDONLY                      },
-    { (ccpc[]){ "w",  "wb",         NULL }, O_WRONLY | O_CREAT | O_TRUNC  },
-    { (ccpc[]){ "a",  "ab",         NULL }, O_WRONLY | O_CREAT | O_APPEND },
-    { (ccpc[]){ "r+", "rb+", "r+b", NULL }, O_RDWR                        },
-    { (ccpc[]){ "w+", "wb+", "w+b", NULL }, O_RDWR   | O_CREAT | O_TRUNC  },
-    { (ccpc[]){ "a+", "ab+", "a+b", NULL }, O_RDWR   | O_CREAT | O_APPEND }
+    { (ccpc[]){ "r",          NULL }, O_RDONLY                                 },
+    { (ccpc[]){ "rb",         NULL }, O_RDONLY                      | O_BINARY },
+    { (ccpc[]){ "w",          NULL }, O_WRONLY | O_CREAT | O_TRUNC             },
+    { (ccpc[]){ "wb",         NULL }, O_WRONLY | O_CREAT | O_TRUNC  | O_BINARY },
+    { (ccpc[]){ "a",          NULL }, O_WRONLY | O_CREAT | O_APPEND            },
+    { (ccpc[]){ "ab",         NULL }, O_WRONLY | O_CREAT | O_APPEND | O_BINARY },
+    { (ccpc[]){ "r+",         NULL }, O_RDWR                                   },
+    { (ccpc[]){ "rb+", "r+b", NULL }, O_RDWR                        | O_BINARY },
+    { (ccpc[]){ "w+",         NULL }, O_RDWR   | O_CREAT | O_TRUNC             },
+    { (ccpc[]){ "wb+", "w+b", NULL }, O_RDWR   | O_CREAT | O_TRUNC  | O_BINARY },
+    { (ccpc[]){ "a+",         NULL }, O_RDWR   | O_CREAT | O_APPEND            },
+    { (ccpc[]){ "ab+", "a+b", NULL }, O_RDWR   | O_CREAT | O_APPEND | O_BINARY }
 };
 
 static int
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/2] qga: unlink just created guest-file if fchmod() or fdopen() fails on it

[Michael Roth]

From: Laszlo Ersek <lersek@redhat.com>

We shouldn't allow guest filesystem pollution on error paths.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 2eec712..e199738 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -355,6 +355,9 @@ safe_open_or_create(const char *path, const char *mode, Error **err)
             }
 
             close(fd);
+            if (oflag & O_CREAT) {
+                unlink(path);
+            }
         }
     }
 
-- 
1.7.9.5