[PATCH 1/2] libsepol: do not seg fault on sepol_*_key_free(NULL)

[PATCH 1/2] libsepol: do not seg fault on sepol_*_key_free(NULL)

[Stephen Smalley]

sepol_*_key_free(NULL) should just be a no-op just like
free(NULL).  Fix several instances that did not handle this
correctly and would seg fault if called with NULL.

Test: setsebool -P zebra_write_config=1 while non-root

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 libsepol/src/boolean_record.c | 2 ++
 libsepol/src/iface_record.c   | 2 ++
 libsepol/src/user_record.c    | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/libsepol/src/boolean_record.c b/libsepol/src/boolean_record.c
index ebef7f1..a194704 100644
--- a/libsepol/src/boolean_record.c
+++ b/libsepol/src/boolean_record.c
@@ -67,6 +67,8 @@ int sepol_bool_key_extract(sepol_handle_t * handle,
 
 void sepol_bool_key_free(sepol_bool_key_t * key)
 {
+	if (!key)
+		return;
 	free(key->name);
 	free(key);
 }
diff --git a/libsepol/src/iface_record.c b/libsepol/src/iface_record.c
index c8b977c..6d56835 100644
--- a/libsepol/src/iface_record.c
+++ b/libsepol/src/iface_record.c
@@ -73,6 +73,8 @@ int sepol_iface_key_extract(sepol_handle_t * handle,
 
 void sepol_iface_key_free(sepol_iface_key_t * key)
 {
+	if (!key)
+		return;
 	free(key->name);
 	free(key);
 }
diff --git a/libsepol/src/user_record.c b/libsepol/src/user_record.c
index ed5b048..fa95f2d 100644
--- a/libsepol/src/user_record.c
+++ b/libsepol/src/user_record.c
@@ -76,6 +76,8 @@ int sepol_user_key_extract(sepol_handle_t * handle,
 
 void sepol_user_key_free(sepol_user_key_t * key)
 {
+	if (!key)
+		return;
 	free(key->name);
 	free(key);
 }
-- 
2.9.3

[PATCH 2/2] libsemanage: revert "Skip policy module re-link when only setting booleans."

[Stephen Smalley]

commit e5aaa01f81afa278cce79bd59ebfdb80a32e4e5a ("Skip policy module
re-link when only setting booleans.") can lead to duplicate entries
(e.g. portcon entries) being added into the kernel policy because the
existing linked policy already includes the local customizations.
Revert this commit until we can come up with an approach that handles
this properly.  This means that setsebool -P triggers a full policy
rebuild.

>From the original bug report:
I've noticed a strange interaction with custom ports and booleans.
After setting a boolean, the list of ports for a particular type
(which has been customized) shows duplicate entries.

Example:

    $ semanage port -a -t http_port_t -p tcp 12345
    $ semanage port -l | grep http_port_t
    http_port_t                    tcp      12345, 80, 81, ...
    $ setsebool -P zebra_write_config false
    $ semanage port -l | grep http_port_t
    http_port_t                    tcp      12345, 12345, 80, 81, ...
    $ setsebool -P zebra_write_config false
    $ semanage port -l | grep http_port_t
    http_port_t                    tcp      12345, 12345, 12345, 80, 81, ...

As can be seen, each time a boolean is set persistently (it doesn't
matter which boolean or which state), the custom port 12345 is
duplicated. Running "semodule -B" clears the duplicates.

However, if only the local customizations are listed, the port is
always listed only once:

    $ semanage port -l -C
    SELinux Port Type              Proto    Port Number
    http_port_t                    tcp      12345

Resolves: https://github.com/SELinuxProject/selinux/issues/50
Reported-by: Carlos Rodrigues <cefrodrigues@gmail.com>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 libsemanage/src/direct_api.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 5687323..ee2f9e7 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -1104,8 +1104,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 	/* Declare some variables */
 	int modified = 0, fcontexts_modified, ports_modified,
 	    seusers_modified, users_extra_modified, dontaudit_modified,
-	    preserve_tunables_modified, bools_modified = 0,
-		disable_dontaudit, preserve_tunables;
+	    preserve_tunables_modified, disable_dontaudit, preserve_tunables;
 	dbase_config_t *users = semanage_user_dbase_local(sh);
 	dbase_config_t *users_base = semanage_user_base_dbase_local(sh);
 	dbase_config_t *pusers_base = semanage_user_base_dbase_policy(sh);
@@ -1186,13 +1185,13 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 	users_extra_modified =
 	    users_extra->dtable->is_modified(users_extra->dbase);
 	ports_modified = ports->dtable->is_modified(ports->dbase);
-	bools_modified = bools->dtable->is_modified(bools->dbase);
 
 	modified = sh->modules_modified;
 	modified |= seusers_modified;
 	modified |= users_extra_modified;
 	modified |= ports_modified;
 	modified |= users->dtable->is_modified(users_base->dbase);
+	modified |= bools->dtable->is_modified(bools->dbase);
 	modified |= ifaces->dtable->is_modified(ifaces->dbase);
 	modified |= nodes->dtable->is_modified(nodes->dbase);
 	modified |= dontaudit_modified;
@@ -1316,19 +1315,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 			goto cleanup;
 
 		cil_db_destroy(&cildb);
-	
-	} else {
-		/* Load already linked policy */
-		retval = sepol_policydb_create(&out);
-		if (retval < 0)
-			goto cleanup;
-
-		retval = semanage_read_policydb(sh, out);
-		if (retval < 0)
-			goto cleanup;
-	}
 
-	if (sh->do_rebuild || modified || bools_modified) {
 		/* Attach to policy databases that work with a policydb. */
 		dbase_policydb_attach((dbase_policydb_t *) pusers_base->dbase, out);
 		dbase_policydb_attach((dbase_policydb_t *) pports->dbase, out);
@@ -1350,6 +1337,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 		if (retval < 0)
 			goto cleanup;
 	} else {
+		/* Load already linked policy */
+		retval = sepol_policydb_create(&out);
+		if (retval < 0)
+			goto cleanup;
+
+		retval = semanage_read_policydb(sh, out);
+		if (retval < 0)
+			goto cleanup;
+
 		retval = semanage_base_merge_components(sh);
 		if (retval < 0)
 			goto cleanup;
@@ -1444,7 +1440,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 	sepol_policydb_free(out);
 	out = NULL;
 
-	if (sh->do_rebuild || modified || bools_modified || fcontexts_modified) {
+	if (sh->do_rebuild || modified || fcontexts_modified) {
 		retval = semanage_install_sandbox(sh);
 	}
 
@@ -1458,7 +1454,7 @@ cleanup:
 		free(mod_filenames[i]);
 	}
 
-	if (modified || bools_modified) {
+	if (modified) {
 		/* Detach from policydb, so it can be freed */
 		dbase_policydb_detach((dbase_policydb_t *) pusers_base->dbase);
 		dbase_policydb_detach((dbase_policydb_t *) pports->dbase);
-- 
2.9.3