[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 13/18] security_selinux: Restore label on failed setfilecon() attempt



It's important to keep XATTRs untouched (well, in the same state
they were in when entering the function). Otherwise our
refcounting would be messed up.

Signed-off-by: Michal Privoznik <mprivozn redhat com>
---
 src/security/security_selinux.c | 40 +++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 4990d94b5f..290faba9d6 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -222,10 +222,10 @@ virSecuritySELinuxRecallLabel(const char *path,
 }
 
 
-static int virSecuritySELinuxSetFileconHelper(const char *path,
+static int virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
+                                              const char *path,
                                               const char *tcon,
                                               bool optional,
-                                              bool privileged,
                                               bool remember);
 
 
@@ -252,7 +252,6 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
 {
     virSecuritySELinuxContextListPtr list = opaque;
     virSecurityManagerMetadataLockStatePtr state;
-    bool privileged = virSecurityManagerGetPrivileged(list->manager);
     const char **paths = NULL;
     size_t npaths = 0;
     size_t i;
@@ -279,10 +278,10 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
 
         /* TODO Implement rollback */
         if (!item->restore) {
-            rv = virSecuritySELinuxSetFileconHelper(item->path,
+            rv = virSecuritySELinuxSetFileconHelper(list->manager,
+                                                    item->path,
                                                     item->tcon,
                                                     item->optional,
-                                                    privileged,
                                                     list->lock);
         } else {
             rv = virSecuritySELinuxRestoreFileLabel(list->manager,
@@ -1303,9 +1302,13 @@ virSecuritySELinuxSetFileconImpl(const char *path, const char *tcon,
 
 
 static int
-virSecuritySELinuxSetFileconHelper(const char *path, const char *tcon,
-                                   bool optional, bool privileged, bool remember)
+virSecuritySELinuxSetFileconHelper(virSecurityManagerPtr mgr,
+                                   const char *path,
+                                   const char *tcon,
+                                   bool optional,
+                                   bool remember)
 {
+    bool privileged = virSecurityManagerGetPrivileged(mgr);
     security_context_t econ = NULL;
     int rc;
     int ret = -1;
@@ -1329,8 +1332,23 @@ virSecuritySELinuxSetFileconHelper(const char *path, const char *tcon,
             goto cleanup;
     }
 
-    if (virSecuritySELinuxSetFileconImpl(path, tcon, optional, privileged) < 0)
+    if (virSecuritySELinuxSetFileconImpl(path, tcon, optional, privileged) < 0) {
+        virErrorPtr origerr;
+
+        virErrorPreserveLast(&origerr);
+        /* Try to restore the label. This is done so that XATTRs
+         * are left in the same state as when the control entered
+         * this function. However, if our attempt fails, there's
+         * not much we can do. XATTRs refcounting is fubar'ed and
+         * the only option we have is warn users. */
+        if (virSecuritySELinuxRestoreFileLabel(mgr, path, remember) < 0)
+            VIR_WARN("Unable to restore label on '%s'. "
+                     "XATTRs might have been left in inconsistent state.",
+                     path);
+
+        virErrorRestore(&origerr);
         goto cleanup;
+    }
 
     ret = 0;
  cleanup:
@@ -1343,16 +1361,14 @@ static int
 virSecuritySELinuxSetFileconOptional(virSecurityManagerPtr mgr,
                                      const char *path, const char *tcon)
 {
-    bool privileged = virSecurityManagerGetPrivileged(mgr);
-    return virSecuritySELinuxSetFileconHelper(path, tcon, true, privileged, false);
+    return virSecuritySELinuxSetFileconHelper(mgr, path, tcon, true, false);
 }
 
 static int
 virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr,
                              const char *path, const char *tcon)
 {
-    bool privileged = virSecurityManagerGetPrivileged(mgr);
-    return virSecuritySELinuxSetFileconHelper(path, tcon, false, privileged, false);
+    return virSecuritySELinuxSetFileconHelper(mgr, path, tcon, false, false);
 }
 
 static int
-- 
2.18.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]