Skip to contentSkip to conversation

SELinux: return error codes on policy load failure

PATCH In review Eric Paris 3 participants 4 messages Apr 20, 2010 SELinux security module

lore
Share & tools
lore

Eric Paris

author
#0 lore raw

PATCH 1/1 SELinux: return error codes on policy load failure

policy load failure always return EINVAL even if the failure was for some other reason (usually ENOMEM). This patch passes error codes back up the stack where they will make their way to userspace. This might help in debugging future problems with policy load.

---

 security/selinux/ss/services.c |   37 ++++++++++++++++++++++---------------
 1 files changed, 22 insertions(+), 15 deletions(-)
security/selinux/ss/services.c +22 -15
index 0b44f5a..1de60ce 100644
@@ -1760,22 +1760,28 @@ int security_load_policy(void *data, size_t len)
 
 	if (!ss_initialized) {
 		avtab_cache_init();
-		if (policydb_read(&policydb, fp)) {
+		rc = policydb_read(&policydb, fp);
+		if (rc) {
 			avtab_cache_destroy();
-			return -EINVAL;
+			return rc;
 		}
-		if (selinux_set_mapping(&policydb, secclass_map,
-					&current_mapping,
-					&current_mapping_size)) {
+
+		rc = selinux_set_mapping(&policydb, secclass_map,
+					 &current_mapping,
+					 &current_mapping_size);
+		if (rc) {
 			policydb_destroy(&policydb);
 			avtab_cache_destroy();
-			return -EINVAL;
+			return rc;
 		}
-		if (policydb_load_isids(&policydb, &sidtab)) {
+
+		rc = policydb_load_isids(&policydb, &sidtab);
+		if (rc) {
 			policydb_destroy(&policydb);
 			avtab_cache_destroy();
-			return -EINVAL;
+			return rc;
 		}
+
 		security_load_policycaps();
 		ss_initialized = 1;
 		seqno = ++latest_granting;
@@ -1791,8 +1797,9 @@ int security_load_policy(void *data, size_t len)
 	sidtab_hash_eval(&sidtab, "sids");
 #endif
 
-	if (policydb_read(&newpolicydb, fp))
-		return -EINVAL;
+	rc = policydb_read(&newpolicydb, fp);
+	if (rc)
+		return rc;
 
 	/* If switching between different policy types, log MLS status */
 	if (policydb.mls_enabled && !newpolicydb.mls_enabled)
@@ -1807,8 +1814,8 @@ int security_load_policy(void *data, size_t len)
 		return rc;
 	}
 
-	if (selinux_set_mapping(&newpolicydb, secclass_map,
-				&map, &map_size))
+	rc = selinux_set_mapping(&newpolicydb, secclass_map, &map, &map_size);
+	if (rc)
 		goto err;
 
 	rc = security_preserve_bools(&newpolicydb);
@@ -1819,10 +1826,10 @@ int security_load_policy(void *data, size_t len)
 
 	/* Clone the SID table. */
 	sidtab_shutdown(&sidtab);
-	if (sidtab_map(&sidtab, clone_sid, &newsidtab)) {
-		rc = -ENOMEM;
+
+	rc = sidtab_map(&sidtab, clone_sid, &newsidtab);
+	if (rc)
 		goto err;
-	}
 
 	/*
 	 * Convert the internal representations of contexts

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Acked-by on v1 1/1
#1 lore raw
Quoted 6 lines · Eric Paris

policy load failure always return EINVAL even if the failure was for some other reason (usually ENOMEM). This patch passes error codes back up the stack where they will make their way to userspace. This might help in debugging future problems with policy load.

Note that policydb_read() internally has the same problem on some code paths, leading to the logic on the bad: path that defaults to EINVAL if not otherwise set.

Quoted 83 lines

---

 security/selinux/ss/services.c |   37 ++++++++++++++++++++++---------------
 1 files changed, 22 insertions(+), 15 deletions(-)
security/selinux/ss/services.c +22 -15
index 0b44f5a..1de60ce 100644
@@ -1760,22 +1760,28 @@ int security_load_policy(void *data, size_t len)
 
 	if (!ss_initialized) {
 		avtab_cache_init();
-		if (policydb_read(&policydb, fp)) {
+		rc = policydb_read(&policydb, fp);
+		if (rc) {
 			avtab_cache_destroy();
-			return -EINVAL;
+			return rc;
 		}
-		if (selinux_set_mapping(&policydb, secclass_map,
-					&current_mapping,
-					&current_mapping_size)) {
+
+		rc = selinux_set_mapping(&policydb, secclass_map,
+					 &current_mapping,
+					 &current_mapping_size);
+		if (rc) {
 			policydb_destroy(&policydb);
 			avtab_cache_destroy();
-			return -EINVAL;
+			return rc;
 		}
-		if (policydb_load_isids(&policydb, &sidtab)) {
+
+		rc = policydb_load_isids(&policydb, &sidtab);
+		if (rc) {
 			policydb_destroy(&policydb);
 			avtab_cache_destroy();
-			return -EINVAL;
+			return rc;
 		}
+
 		security_load_policycaps();
 		ss_initialized = 1;
 		seqno = ++latest_granting;
@@ -1791,8 +1797,9 @@ int security_load_policy(void *data, size_t len)
 	sidtab_hash_eval(&sidtab, "sids");
 #endif
 
-	if (policydb_read(&newpolicydb, fp))
-		return -EINVAL;
+	rc = policydb_read(&newpolicydb, fp);
+	if (rc)
+		return rc;
 
 	/* If switching between different policy types, log MLS status */
 	if (policydb.mls_enabled && !newpolicydb.mls_enabled)
@@ -1807,8 +1814,8 @@ int security_load_policy(void *data, size_t len)
 		return rc;
 	}
 
-	if (selinux_set_mapping(&newpolicydb, secclass_map,
-				&map, &map_size))
+	rc = selinux_set_mapping(&newpolicydb, secclass_map, &map, &map_size);
+	if (rc)
 		goto err;
 
 	rc = security_preserve_bools(&newpolicydb);
@@ -1819,10 +1826,10 @@ int security_load_policy(void *data, size_t len)
 
 	/* Clone the SID table. */
 	sidtab_shutdown(&sidtab);
-	if (sidtab_map(&sidtab, clone_sid, &newsidtab)) {
-		rc = -ENOMEM;
+
+	rc = sidtab_map(&sidtab, clone_sid, &newsidtab);
+	if (rc)
 		goto err;
-	}
 
 	/*
 	 * Convert the internal representations of contexts
Signature · 8 lines
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.
#2 lore raw
Quoted 3 lines · Stephen Smalley

Note that policydb_read() internally has the same problem on some code paths, leading to the logic on the bad: path that defaults to EINVAL if not otherwise set.

Added to the kernel development todo list:
http://selinuxproject.org/page/Kernel_Development#To_Do_List

(which could use more love :-)

- James

Signature · 7 lines
James Morris
<[email protected]>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.
#3 lore raw
In reply to Eric Paris
Quoted 6 lines · Eric Paris

policy load failure always return EINVAL even if the failure was for some other reason (usually ENOMEM). This patch passes error codes back up the stack where they will make their way to userspace. This might help in debugging future problems with policy load.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next

Signature · 7 lines
James Morris
<[email protected]>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.