From 53d335085c363f998ab7d81c977704babf2a30e1 Mon Sep 17 00:00:00 2001

Message-Id: <53d335085c363f998ab7d81c977704babf2a30e1@dist-git>

From: Eric Blake <eblake@redhat.com>

Date: Wed, 26 Feb 2014 14:54:36 +0100

Subject: [PATCH] storage: don't read storage volumes in nonblock mode

https://bugzilla.redhat.com/show_bug.cgi?id=1032370

Commit 348b4e2 introduced a potential problem (thankfully not

in any release): we are attempting to use virFileReadHeaderFD()

on a file that was opened with O_NONBLOCK.  While this

shouldn't be a problem in practice (because O_NONBLOCK

typically doesn't affect regular or block files, and fifos and

sockets cannot be storage volumes), it's better to play it safe

to avoid races from opening an unexpected file type while also

avoiding problems with having to handle EAGAIN while read()ing.

Based on a report by Dan Berrange.

* src/storage/storage_backend.c

(virStorageBackendVolOpenCheckMode): Fix up fd after avoiding race.

Signed-off-by: Eric Blake <eblake@redhat.com>

(cherry picked from commit 655ea8dc02d88ec742ed2829fafbf6e269002adf)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/storage/storage_backend.c | 25 +++++++++++++++++++++----

 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c

index f8c0004..ae24c74 100644

--- a/src/storage/storage_backend.c

+++ b/src/storage/storage_backend.c

@@ -1150,6 +1150,12 @@ virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb,

         return -2;

     }

+    /* O_NONBLOCK should only matter during open() for fifos and

+     * sockets, which we already filtered; but using it prevents a

+     * TOCTTOU race.  However, later on we will want to read() the

+     * header from this fd, and virFileRead* routines require a

+     * blocking fd, so fix it up after verifying we avoided a

+     * race.  */

     if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) {

         if ((errno == ENOENT || errno == ELOOP) &&

             S_ISLNK(sb->st_mode)) {

@@ -1171,13 +1177,13 @@ virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb,

         return -1;

     }

-    if (S_ISREG(sb->st_mode))

+    if (S_ISREG(sb->st_mode)) {

         mode = VIR_STORAGE_VOL_OPEN_REG;

-    else if (S_ISCHR(sb->st_mode))

+    } else if (S_ISCHR(sb->st_mode)) {

         mode = VIR_STORAGE_VOL_OPEN_CHAR;

-    else if (S_ISBLK(sb->st_mode))

+    } else if (S_ISBLK(sb->st_mode)) {

         mode = VIR_STORAGE_VOL_OPEN_BLOCK;

-    else if (S_ISDIR(sb->st_mode)) {

+    } else if (S_ISDIR(sb->st_mode)) {

         mode = VIR_STORAGE_VOL_OPEN_DIR;

         if (STREQ(base, ".") ||

@@ -1186,6 +1192,17 @@ virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb,

             VIR_INFO("Skipping special dir '%s'", base);

             return -2;

         }

+    } else {

+        VIR_WARN("ignoring unexpected type for file '%s'", path);

+        VIR_FORCE_CLOSE(fd);

+        return -2;

+    }

+

+    if (virSetBlocking(fd, true) < 0) {

+        virReportSystemError(errno, _("unable to set blocking mode for '%s'"),

+                             path);

+        VIR_FORCE_CLOSE(fd);

+        return -2;

     }

     if (!(mode & flags)) {

-- 

1.9.0