[libvirt] [PATCH 3/3] storage: Check for invalid storage mode before opening

[Cole Robinson]

If a directory pool contains pipes or sockets, a pool start can fail or hang:

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

We already try to avoid these special files, but only attempt after
opening the path, which is where the problems lie. Unify volume opening
into a single function which runs stat() before any open() call. Directory
pools can then proceed along, ignoring the invalid files.

Signed-off-by: Cole Robinson <crobinso at redhat.com>
---
 src/storage/storage_backend.c         |   37 +++++++++++++++++++++++++++-----
 src/storage/storage_backend.h         |    1 +
 src/storage/storage_backend_fs.c      |    9 ++-----
 src/storage/storage_backend_logical.c |    9 ++-----
 src/storage/storage_backend_mpath.c   |    9 ++-----
 src/storage/storage_backend_scsi.c    |   17 +++++++--------
 6 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index f4124df..8163391 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -872,6 +872,34 @@ virStorageBackendForType(int type) {
     return NULL;
 }
 
+int
+virStorageBackendVolOpen(const char *path)
+{
+    int fd;
+    struct stat sb;
+
+    if (stat(path, &sb) < 0) {
+        virReportSystemError(errno,
+                             _("cannot stat file '%s'"), path);
+        return -1;
+    }
+
+    if (!S_ISREG(sb.st_mode) &&
+        !S_ISCHR(sb.st_mode) &&
+        !S_ISBLK(sb.st_mode)) {
+        VIR_DEBUG("Skipping volume path %s, unexpected file mode.", path);
+        return -2;
+    }
+
+    if ((fd = open(path, O_RDONLY)) < 0) {
+        virReportSystemError(errno,
+                             _("cannot open volume '%s'"),
+                             path);
+        return -1;
+    }
+
+    return fd;
+}
 
 int
 virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target,
@@ -880,13 +908,10 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target,
 {
     int ret, fd;
 
-    if ((fd = open(target->path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot open volume '%s'"),
-                             target->path);
-        return -1;
-    }
+    if ((ret = virStorageBackendVolOpen(target->path)) < 0)
+        return ret;
 
+    fd = ret;
     ret = virStorageBackendUpdateVolTargetInfoFD(target,
                                                  fd,
                                                  allocation,
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 907c4bc..ddd52a5 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -80,6 +80,7 @@ struct _virStorageBackend {
 
 virStorageBackendPtr virStorageBackendForType(int type);
 
+int virStorageBackendVolOpen(const char *path);
 
 int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
                                    int withCapacity);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index c96c4f3..1d47b2f 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -61,12 +61,9 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
     if (encryption)
         *encryption = NULL;
 
-    if ((fd = open(target->path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot open volume '%s'"),
-                             target->path);
-        return -1;
-    }
+    if ((ret = virStorageBackendVolOpen(target->path)) < 0)
+        return ret; /* Take care to propagate ret, it is not always -1 */
+    fd = ret;
 
     if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd,
                                                       allocation,
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 06238d1..616ca1a 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -559,7 +559,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
                                   virStoragePoolObjPtr pool,
                                   virStorageVolDefPtr vol)
 {
-    int fd = -1;
+    int fdret, fd = -1;
     char size[100];
     const char *cmdargvnew[] = {
         LVCREATE, "--name", vol->name, "-L", size,
@@ -602,12 +602,9 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
     if (virRun(cmdargv, NULL) < 0)
         return -1;
 
-    if ((fd = open(vol->target.path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot read path '%s'"),
-                             vol->target.path);
+    if ((fdret = virStorageBackendVolOpen(vol->target.path)) < 0)
         goto cleanup;
-    }
+    fd = fdret;
 
     /* We can only chown/grp if root */
     if (getuid() == 0) {
diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
index 735a92e..a49432e 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -44,14 +44,11 @@ virStorageBackendMpathUpdateVolTargetInfo(virStorageVolTargetPtr target,
                                           unsigned long long *capacity)
 {
     int ret = -1;
-    int fd = -1;
+    int fdret, fd = -1;
 
-    if ((fd = open(target->path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot open volume '%s'"),
-                             target->path);
+    if ((fdret = virStorageBackendVolOpen(target->path)) < 0)
         goto out;
-    }
+    fd = fdret;
 
     if (virStorageBackendUpdateVolTargetInfoFD(target,
                                                fd,
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 93aeb79..b04d581 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -135,14 +135,12 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target,
                                          unsigned long long *allocation,
                                          unsigned long long *capacity)
 {
-    int fd, ret = -1;
+    int fdret, fd = -1;
+    int ret = -1;
 
-    if ((fd = open(target->path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot open volume '%s'"),
-                             target->path);
-        return -1;
-    }
+    if ((fdret = virStorageBackendVolOpen(target->path)) < 0)
+        goto cleanup;
+    fd = fdret;
 
     if (virStorageBackendUpdateVolTargetInfoFD(target,
                                                fd,
@@ -155,8 +153,9 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target,
 
     ret = 0;
 
-  cleanup:
-    close(fd);
+cleanup:
+    if (fd >= 0)
+        close(fd);
 
     return ret;
 }
-- 
1.6.6.1