c401cc
From 53d335085c363f998ab7d81c977704babf2a30e1 Mon Sep 17 00:00:00 2001
c401cc
Message-Id: <53d335085c363f998ab7d81c977704babf2a30e1@dist-git>
c401cc
From: Eric Blake <eblake@redhat.com>
c401cc
Date: Wed, 26 Feb 2014 14:54:36 +0100
c401cc
Subject: [PATCH] storage: don't read storage volumes in nonblock mode
c401cc
c401cc
https://bugzilla.redhat.com/show_bug.cgi?id=1032370
c401cc
c401cc
Commit 348b4e2 introduced a potential problem (thankfully not
c401cc
in any release): we are attempting to use virFileReadHeaderFD()
c401cc
on a file that was opened with O_NONBLOCK.  While this
c401cc
shouldn't be a problem in practice (because O_NONBLOCK
c401cc
typically doesn't affect regular or block files, and fifos and
c401cc
sockets cannot be storage volumes), it's better to play it safe
c401cc
to avoid races from opening an unexpected file type while also
c401cc
avoiding problems with having to handle EAGAIN while read()ing.
c401cc
c401cc
Based on a report by Dan Berrange.
c401cc
c401cc
* src/storage/storage_backend.c
c401cc
(virStorageBackendVolOpenCheckMode): Fix up fd after avoiding race.
c401cc
c401cc
Signed-off-by: Eric Blake <eblake@redhat.com>
c401cc
(cherry picked from commit 655ea8dc02d88ec742ed2829fafbf6e269002adf)
c401cc
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
c401cc
---
c401cc
 src/storage/storage_backend.c | 25 +++++++++++++++++++++----
c401cc
 1 file changed, 21 insertions(+), 4 deletions(-)
c401cc
c401cc
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
c401cc
index f8c0004..ae24c74 100644
c401cc
--- a/src/storage/storage_backend.c
c401cc
+++ b/src/storage/storage_backend.c
c401cc
@@ -1150,6 +1150,12 @@ virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb,
c401cc
         return -2;
c401cc
     }
c401cc
 
c401cc
+    /* O_NONBLOCK should only matter during open() for fifos and
c401cc
+     * sockets, which we already filtered; but using it prevents a
c401cc
+     * TOCTTOU race.  However, later on we will want to read() the
c401cc
+     * header from this fd, and virFileRead* routines require a
c401cc
+     * blocking fd, so fix it up after verifying we avoided a
c401cc
+     * race.  */
c401cc
     if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) {
c401cc
         if ((errno == ENOENT || errno == ELOOP) &&
c401cc
             S_ISLNK(sb->st_mode)) {
c401cc
@@ -1171,13 +1177,13 @@ virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb,
c401cc
         return -1;
c401cc
     }
c401cc
 
c401cc
-    if (S_ISREG(sb->st_mode))
c401cc
+    if (S_ISREG(sb->st_mode)) {
c401cc
         mode = VIR_STORAGE_VOL_OPEN_REG;
c401cc
-    else if (S_ISCHR(sb->st_mode))
c401cc
+    } else if (S_ISCHR(sb->st_mode)) {
c401cc
         mode = VIR_STORAGE_VOL_OPEN_CHAR;
c401cc
-    else if (S_ISBLK(sb->st_mode))
c401cc
+    } else if (S_ISBLK(sb->st_mode)) {
c401cc
         mode = VIR_STORAGE_VOL_OPEN_BLOCK;
c401cc
-    else if (S_ISDIR(sb->st_mode)) {
c401cc
+    } else if (S_ISDIR(sb->st_mode)) {
c401cc
         mode = VIR_STORAGE_VOL_OPEN_DIR;
c401cc
 
c401cc
         if (STREQ(base, ".") ||
c401cc
@@ -1186,6 +1192,17 @@ virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb,
c401cc
             VIR_INFO("Skipping special dir '%s'", base);
c401cc
             return -2;
c401cc
         }
c401cc
+    } else {
c401cc
+        VIR_WARN("ignoring unexpected type for file '%s'", path);
c401cc
+        VIR_FORCE_CLOSE(fd);
c401cc
+        return -2;
c401cc
+    }
c401cc
+
c401cc
+    if (virSetBlocking(fd, true) < 0) {
c401cc
+        virReportSystemError(errno, _("unable to set blocking mode for '%s'"),
c401cc
+                             path);
c401cc
+        VIR_FORCE_CLOSE(fd);
c401cc
+        return -2;
c401cc
     }
c401cc
 
c401cc
     if (!(mode & flags)) {
c401cc
-- 
c401cc
1.9.0
c401cc