util: move detection of shared filesystems

The code in virstoragefile.c is getting more complex as I
consolidate backing chain handling code.  But for the setuid
virt-login-shell, we don't need to crawl backing chains.  It's
easier to audit things for setuid security if there are fewer
files involved, so this patch moves the one function that
virFileOpen() was actually relying on to also live in virfile.c.

* src/util/virstoragefile.c (virStorageFileIsSharedFS)
(virStorageFileIsSharedFSType): Move...
* src/util/virfile.c (virFileIsSharedFS, virFileIsSharedFSType):
...to here, and rename.
(virFileOpenAs): Update caller.
* src/security/security_selinux.c
(virSecuritySELinuxSetFileconHelper)
(virSecuritySELinuxSetSecurityAllLabel)
(virSecuritySELinuxRestoreSecurityImageLabelInt): Likewise.
* src/security/security_dac.c
(virSecurityDACRestoreSecurityImageLabelInt): Likewise.
* src/qemu/qemu_driver.c (qemuOpenFileAs): Likewise.
* src/qemu/qemu_migration.c (qemuMigrationIsSafe): Likewise.
* src/util/virstoragefile.h: Adjust declarations.
* src/util/virfile.h: Likewise.
* src/libvirt_private.syms (virfile.h, virstoragefile.h): Move
symbols as appropriate.

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

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index da70ee321eb5944c21e226212dc6244b09fe41c1..07d4f361e5390ce546749e6b96dd1614c1180745 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1242,6 +1242,8 @@ virFileIsDir;
 virFileIsExecutable;
 virFileIsLink;
 virFileIsMountPoint;
+virFileIsSharedFS;
+virFileIsSharedFSType;
 virFileLinkPointsTo;
 virFileLock;
 virFileLoopDeviceAssociate;
@@ -1829,8 +1831,6 @@ virStorageFileGetMetadataFromBuf;
 virStorageFileGetMetadataFromFD;
 virStorageFileGetSCSIKey;
 virStorageFileIsClusterFS;
-virStorageFileIsSharedFS;
-virStorageFileIsSharedFSType;
 virStorageFileProbeFormat;
 virStorageFileProbeFormatFromBuf;
 virStorageFileResize;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b57700e03633bf6806e37f289a70447e59821fb4..9d48a46d0dd7d9b3a604f9d770e9668ec1127562 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2790,7 +2790,7 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
     bool bypass_security = false;
     unsigned int vfoflags = 0;
     int fd = -1;
-    int path_shared = virStorageFileIsSharedFS(path);
+    int path_shared = virFileIsSharedFS(path);
     uid_t uid = geteuid();
     gid_t gid = getegid();
 

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b389d0a85a719e8fea104a59c02d91ad5284ff96..593d2d3bd65d110359b1006c4fa9f86bc4f1f014 100644 (file)
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1534,7 +1534,7 @@ qemuMigrationIsSafe(virDomainDefPtr def)
             int rc;
 
             if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE) {
-                if ((rc = virStorageFileIsSharedFS(src)) < 0)
+                if ((rc = virFileIsSharedFS(src)) < 0)
                     return false;
                 else if (rc == 0)
                     continue;

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 00c81d0e567af4de5babc41a83d1899b9fa47369..851276796fa43a5a64f46aed4f0681f554d7a1ad 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -402,7 +402,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
      * VM's I/O attempts :-)
      */
     if (migrated) {
-        int rc = virStorageFileIsSharedFS(src);
+        int rc = virFileIsSharedFS(src);
         if (rc < 0)
             return -1;
         if (rc == 1) {

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index aa21a223a7bb807e6281dd01d6354a75c02759b7..bfac11c7dfb234a340c1c4daeb24a98250f7e6a3 100644 (file)
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -920,8 +920,7 @@ virSecuritySELinuxSetFileconHelper(const char *path, char *tcon, bool optional)
                 return -1;
         } else {
             const char *msg;
-            if ((virStorageFileIsSharedFSType(path,
-                                              VIR_STORAGE_FILE_SHFS_NFS) == 1) &&
+            if (virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS) == 1 &&
                 security_get_boolean_active("virt_use_nfs") != 1) {
                 msg = _("Setting security context '%s' on '%s' not supported. "
                         "Consider setting virt_use_nfs");
@@ -1172,7 +1171,7 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
      * VM's I/O attempts :-)
      */
     if (migrated) {
-        int rc = virStorageFileIsSharedFS(src);
+        int rc = virFileIsSharedFS(src);
         if (rc < 0)
             return -1;
         if (rc == 1) {
@@ -2323,8 +2322,7 @@ virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr,
 
     if (stdin_path) {
         if (virSecuritySELinuxSetFilecon(stdin_path, data->content_context) < 0 &&
-            virStorageFileIsSharedFSType(stdin_path,
-                                         VIR_STORAGE_FILE_SHFS_NFS) != 1)
+            virFileIsSharedFSType(stdin_path, VIR_FILE_SHFS_NFS) != 1)
             return -1;
     }
 

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 6debbb0ce08758176ffe182aec3bb2d2526c3761..fcc65b1b45ece9618f17d05d194a57bf844f4cea 100644 (file)
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -43,6 +43,13 @@
 # include <sys/mman.h>
 #endif
 
+#ifdef __linux__
+# if HAVE_LINUX_MAGIC_H
+#  include <linux/magic.h>
+# endif
+# include <sys/statfs.h>
+#endif
+
 #if defined(__linux__) && HAVE_DECL_LO_FLAGS_AUTOCLEAR
 # include <linux/loop.h>
 # include <sys/ioctl.h>
@@ -2050,7 +2057,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode,
 
             /* On Linux we can also verify the FS-type of the
              * directory.  (this is a NOP on other platforms). */
-            if (virStorageFileIsSharedFS(path) <= 0)
+            if (virFileIsSharedFS(path) <= 0)
                 goto error;
         }
 
@@ -2646,3 +2653,116 @@ int virFilePrintf(FILE *fp, const char *msg, ...)
 
     return ret;
 }
+
+
+#ifdef __linux__
+
+# ifndef NFS_SUPER_MAGIC
+#  define NFS_SUPER_MAGIC 0x6969
+# endif
+# ifndef OCFS2_SUPER_MAGIC
+#  define OCFS2_SUPER_MAGIC 0x7461636f
+# endif
+# ifndef GFS2_MAGIC
+#  define GFS2_MAGIC 0x01161970
+# endif
+# ifndef AFS_FS_MAGIC
+#  define AFS_FS_MAGIC 0x6B414653
+# endif
+# ifndef SMB_SUPER_MAGIC
+#  define SMB_SUPER_MAGIC 0x517B
+# endif
+# ifndef CIFS_SUPER_MAGIC
+#  define CIFS_SUPER_MAGIC 0xFF534D42
+# endif
+
+int
+virFileIsSharedFSType(const char *path,
+                      int fstypes)
+{
+    char *dirpath, *p;
+    struct statfs sb;
+    int statfs_ret;
+
+    if (VIR_STRDUP(dirpath, path) < 0)
+        return -1;
+
+    do {
+
+        /* Try less and less of the path until we get to a
+         * directory we can stat. Even if we don't have 'x'
+         * permission on any directory in the path on the NFS
+         * server (assuming it's NFS), we will be able to stat the
+         * mount point, and that will properly tell us if the
+         * fstype is NFS.
+         */
+
+        if ((p = strrchr(dirpath, '/')) == NULL) {
+            virReportSystemError(EINVAL,
+                         _("Invalid relative path '%s'"), path);
+            VIR_FREE(dirpath);
+            return -1;
+        }
+
+        if (p == dirpath)
+            *(p+1) = '\0';
+        else
+            *p = '\0';
+
+        statfs_ret = statfs(dirpath, &sb);
+
+    } while ((statfs_ret < 0) && (p != dirpath));
+
+    VIR_FREE(dirpath);
+
+    if (statfs_ret < 0) {
+        virReportSystemError(errno,
+                             _("cannot determine filesystem for '%s'"),
+                             path);
+        return -1;
+    }
+
+    VIR_DEBUG("Check if path %s with FS magic %lld is shared",
+              path, (long long int)sb.f_type);
+
+    if ((fstypes & VIR_FILE_SHFS_NFS) &&
+        (sb.f_type == NFS_SUPER_MAGIC))
+        return 1;
+
+    if ((fstypes & VIR_FILE_SHFS_GFS2) &&
+        (sb.f_type == GFS2_MAGIC))
+        return 1;
+    if ((fstypes & VIR_FILE_SHFS_OCFS) &&
+        (sb.f_type == OCFS2_SUPER_MAGIC))
+        return 1;
+    if ((fstypes & VIR_FILE_SHFS_AFS) &&
+        (sb.f_type == AFS_FS_MAGIC))
+        return 1;
+    if ((fstypes & VIR_FILE_SHFS_SMB) &&
+        (sb.f_type == SMB_SUPER_MAGIC))
+        return 1;
+    if ((fstypes & VIR_FILE_SHFS_CIFS) &&
+        (sb.f_type == CIFS_SUPER_MAGIC))
+        return 1;
+
+    return 0;
+}
+#else
+int virFileIsSharedFSType(const char *path ATTRIBUTE_UNUSED,
+                          int fstypes ATTRIBUTE_UNUSED)
+{
+    /* XXX implement me :-) */
+    return 0;
+}
+#endif
+
+int virFileIsSharedFS(const char *path)
+{
+    return virFileIsSharedFSType(path,
+                                 VIR_FILE_SHFS_NFS |
+                                 VIR_FILE_SHFS_GFS2 |
+                                 VIR_FILE_SHFS_OCFS |
+                                 VIR_FILE_SHFS_AFS |
+                                 VIR_FILE_SHFS_SMB |
+                                 VIR_FILE_SHFS_CIFS);
+}

diff --git a/src/util/virfile.h b/src/util/virfile.h
index 302b74ffe78f8a85f9e5dfb882475b9e9cc3cab3..638378a2af884c07c0d380f66c5ce25ee4c97184 100644 (file)
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -1,7 +1,7 @@
 /*
  * virfile.h: safer file handling
  *
- * Copyright (C) 2010-2011, 2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
  * Copyright (C) 2010 IBM Corporation
  * Copyright (C) 2010 Stefan Berger
  * Copyright (C) 2010 Eric Blake
@@ -159,6 +159,17 @@ bool virFileIsDir (const char *file) ATTRIBUTE_NONNULL(1);
 bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1);
 bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
 
+enum {
+    VIR_FILE_SHFS_NFS = (1 << 0),
+    VIR_FILE_SHFS_GFS2 = (1 << 1),
+    VIR_FILE_SHFS_OCFS = (1 << 2),
+    VIR_FILE_SHFS_AFS = (1 << 3),
+    VIR_FILE_SHFS_SMB = (1 << 4),
+    VIR_FILE_SHFS_CIFS = (1 << 5),
+};
+
+int virFileIsSharedFSType(const char *path, int fstypes) ATTRIBUTE_NONNULL(1);
+int virFileIsSharedFS(const char *path) ATTRIBUTE_NONNULL(1);
 int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1);
 
 int virFileGetMountSubtree(const char *mtabpath,

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 72f114e1fce2603dbcd94aca4520895b52d8e6aa..1b2a81048dc3e9a0b52930b711cede5f286407fe 100644 (file)
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -28,12 +28,6 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdlib.h>
-#ifdef __linux__
-# if HAVE_LINUX_MAGIC_H
-#  include <linux/magic.h>
-# endif
-# include <sys/statfs.h>
-#endif
 #include "dirname.h"
 #include "viralloc.h"
 #include "virerror.h"
@@ -1283,126 +1277,15 @@ virStorageFileResize(const char *path,
     return ret;
 }
 
-#ifdef __linux__
-
-# ifndef NFS_SUPER_MAGIC
-#  define NFS_SUPER_MAGIC 0x6969
-# endif
-# ifndef OCFS2_SUPER_MAGIC
-#  define OCFS2_SUPER_MAGIC 0x7461636f
-# endif
-# ifndef GFS2_MAGIC
-#  define GFS2_MAGIC 0x01161970
-# endif
-# ifndef AFS_FS_MAGIC
-#  define AFS_FS_MAGIC 0x6B414653
-# endif
-# ifndef SMB_SUPER_MAGIC
-#  define SMB_SUPER_MAGIC 0x517B
-# endif
-# ifndef CIFS_SUPER_MAGIC
-#  define CIFS_SUPER_MAGIC 0xFF534D42
-# endif
-
-
-int virStorageFileIsSharedFSType(const char *path,
-                                 int fstypes)
-{
-    char *dirpath, *p;
-    struct statfs sb;
-    int statfs_ret;
-
-    if (VIR_STRDUP(dirpath, path) < 0)
-        return -1;
-
-    do {
-
-        /* Try less and less of the path until we get to a
-         * directory we can stat. Even if we don't have 'x'
-         * permission on any directory in the path on the NFS
-         * server (assuming it's NFS), we will be able to stat the
-         * mount point, and that will properly tell us if the
-         * fstype is NFS.
-         */
-
-        if ((p = strrchr(dirpath, '/')) == NULL) {
-            virReportSystemError(EINVAL,
-                         _("Invalid relative path '%s'"), path);
-            VIR_FREE(dirpath);
-            return -1;
-        }
-
-        if (p == dirpath)
-            *(p+1) = '\0';
-        else
-            *p = '\0';
-
-        statfs_ret = statfs(dirpath, &sb);
-
-    } while ((statfs_ret < 0) && (p != dirpath));
-
-    VIR_FREE(dirpath);
-
-    if (statfs_ret < 0) {
-        virReportSystemError(errno,
-                             _("cannot determine filesystem for '%s'"),
-                             path);
-        return -1;
-    }
-
-    VIR_DEBUG("Check if path %s with FS magic %lld is shared",
-              path, (long long int)sb.f_type);
-
-    if ((fstypes & VIR_STORAGE_FILE_SHFS_NFS) &&
-        (sb.f_type == NFS_SUPER_MAGIC))
-        return 1;
-
-    if ((fstypes & VIR_STORAGE_FILE_SHFS_GFS2) &&
-        (sb.f_type == GFS2_MAGIC))
-        return 1;
-    if ((fstypes & VIR_STORAGE_FILE_SHFS_OCFS) &&
-        (sb.f_type == OCFS2_SUPER_MAGIC))
-        return 1;
-    if ((fstypes & VIR_STORAGE_FILE_SHFS_AFS) &&
-        (sb.f_type == AFS_FS_MAGIC))
-        return 1;
-    if ((fstypes & VIR_STORAGE_FILE_SHFS_SMB) &&
-        (sb.f_type == SMB_SUPER_MAGIC))
-        return 1;
-    if ((fstypes & VIR_STORAGE_FILE_SHFS_CIFS) &&
-        (sb.f_type == CIFS_SUPER_MAGIC))
-        return 1;
-
-    return 0;
-}
-#else
-int virStorageFileIsSharedFSType(const char *path ATTRIBUTE_UNUSED,
-                                 int fstypes ATTRIBUTE_UNUSED)
-{
-    /* XXX implement me :-) */
-    return 0;
-}
-#endif
-
-int virStorageFileIsSharedFS(const char *path)
-{
-    return virStorageFileIsSharedFSType(path,
-                                        VIR_STORAGE_FILE_SHFS_NFS |
-                                        VIR_STORAGE_FILE_SHFS_GFS2 |
-                                        VIR_STORAGE_FILE_SHFS_OCFS |
-                                        VIR_STORAGE_FILE_SHFS_AFS |
-                                        VIR_STORAGE_FILE_SHFS_SMB |
-                                        VIR_STORAGE_FILE_SHFS_CIFS);
-}
 
 int virStorageFileIsClusterFS(const char *path)
 {
     /* These are coherent cluster filesystems known to be safe for
      * migration with cache != none
      */
-    return virStorageFileIsSharedFSType(path,
-                                        VIR_STORAGE_FILE_SHFS_GFS2 |
-                                        VIR_STORAGE_FILE_SHFS_OCFS);
+    return virFileIsSharedFSType(path,
+                                 VIR_FILE_SHFS_GFS2 |
+                                 VIR_FILE_SHFS_OCFS);
 }
 
 #ifdef LVS

diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index a6dcfa46ecc398ec4db8aa967ff655b7400b970a..18db09edb583a778f6fc69c58fc8fec280fd01ae 100644 (file)
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -250,19 +250,7 @@ int virStorageFileResize(const char *path,
                          unsigned long long orig_capacity,
                          bool pre_allocate);
 
-enum {
-    VIR_STORAGE_FILE_SHFS_NFS = (1 << 0),
-    VIR_STORAGE_FILE_SHFS_GFS2 = (1 << 1),
-    VIR_STORAGE_FILE_SHFS_OCFS = (1 << 2),
-    VIR_STORAGE_FILE_SHFS_AFS = (1 << 3),
-    VIR_STORAGE_FILE_SHFS_SMB = (1 << 4),
-    VIR_STORAGE_FILE_SHFS_CIFS = (1 << 5),
-};
-
-int virStorageFileIsSharedFS(const char *path);
 int virStorageFileIsClusterFS(const char *path);
-int virStorageFileIsSharedFSType(const char *path,
-                                 int fstypes);
 
 int virStorageFileGetLVMKey(const char *path,
                             char **key);