libsemanage: Skip policy module re-link when only setting booleans.

RFC PATCH In review Stephen Smalley 5 participants 10 messages Jul 25, 2014 – Aug 5, 2014 SELinux security module

lore

Share & tools

lore

Stephen Smalley

author

2014-07-25 17:02 UTC #0 lore raw

RFC PATCH 1/1 libsemanage: Skip policy module re-link when only setting booleans.

Motivated by:
https://bugzilla.redhat.com/show_bug.cgi?id=1098446

I believe this is always safe for booleans because we only set their value; we are never adding new ones via semanage, unlike for example users, ports, nodes, and interfaces. For the rest, I was wondering why we don't save the linked file and just reuse it on those changes rather than re-linking each time - that seems like it would be straightforward to do in libsemanage and make those operations significantly faster and less memory intensive.

Quoted 1 line

From 11f35253194036bde941042fa6a2665297853017 Mon Sep 17 00:00:00 2001

From: Stephen Smalley <[email protected]>
Date: Fri, 25 Jul 2014 12:05:43 -0400
Subject: [PATCH] Skip policy module re-link when only setting booleans.

Since booleans are only set, not added/removed, we do not need to re-link modules when setting them. We can instead just take the existing binary policy and mutate it for the new values.

Signed-off-by Stephen Smalley <[email protected]>

---
 libsemanage/src/direct_api.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

libsemanage/src/direct_api.c +18 ~~-14~~

index 64dc7d9..5b94725 100644
@@ -690,7 +690,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;
+	    preserve_tunables_modified, bools_modified;
 	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);

@@ -771,11 +771,11 @@ 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 |= 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;

@@ -891,15 +891,26 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 
 		/* ==================== Policydb-backed ================ */
 
-		/* Create new policy object, then attach to policy databases
-		 * that work with a policydb */
+		/* Create new policy object */
 		retval = semanage_expand_sandbox(sh, base, &out);
 		if (retval < 0)
 			goto cleanup;
 	
 		sepol_module_package_free(base);
 		base = NULL;
+	} 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);

@@ -921,14 +932,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 		if (retval < 0)
 			goto cleanup;
 	} else {
-		retval = sepol_policydb_create(&out);
-		if (retval < 0)
-			goto cleanup;
-
-		retval = semanage_read_policydb(sh, out);
-		if (retval < 0)
-			goto cleanup;
-		
+		/* Changes to non-kernel policy configurations only. */
 		if (seusers_modified || users_extra_modified) {
 			retval = semanage_link_base(sh, &base);
 			if (retval < 0)

@@ -1007,7 +1011,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 	sepol_policydb_free(out);
 	out = NULL;
 
-	if (sh->do_rebuild || modified || 
+	if (sh->do_rebuild || modified || bools_modified ||
 	    seusers_modified || fcontexts_modified || users_extra_modified) {
 		retval = semanage_install_sandbox(sh);
 	}

@@ -1017,7 +1021,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 		free(mod_filenames[i]);
 	}
 
-	if (modified) {
+	if (modified || bools_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);

Signature · 1 line

1.9.3

Daniel J Walsh

2014-07-25 18:35 UTC #1 lore raw

Nice.

Quoted 17 lines · Stephen Smalley

Motivated by:
https://bugzilla.redhat.com/show_bug.cgi?id=1098446
I believe this is always safe for booleans because we only set their value; we are never adding new ones via semanage, unlike for example users, ports, nodes, and interfaces. For the rest, I was wondering why we don't save the linked file and just reuse it on those changes rather than re-linking each time - that seems like it would be straightforward to do in libsemanage and make those operations significantly faster and less memory intensive.
_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].

Joshua Brindle

2014-07-25 19:45 UTC #2 lore raw

In reply to Stephen Smalley

Quoted 8 lines · Stephen Smalley

Motivated by:
https://bugzilla.redhat.com/show_bug.cgi?id=1098446
I believe this is always safe for booleans because we only set their value; we are never adding new ones via semanage, unlike for example users, ports, nodes, and interfaces. For the rest, I was wondering why we don't save the linked file and just reuse it on those changes rather than re-linking each time - that seems like it would be straightforward

We originally kept the linked copy around and had intended to do what you are saying above but removed it when the minimal Red Hat guys complained about the size of it.

Quoted 2 lines

to do in libsemanage and make those operations significantly faster and less memory intensive.

Daniel J Walsh

2014-07-25 19:49 UTC #3 lore raw

How large is it? Does it matter if it is compressed?

Quoted 17 lines · Joshua Brindle

Quoted 8 lines · Stephen Smalley
Motivated by:
https://bugzilla.redhat.com/show_bug.cgi?id=1098446
I believe this is always safe for booleans because we only set their value; we are never adding new ones via semanage, unlike for example users, ports, nodes, and interfaces. For the rest, I was wondering why we don't save the linked file and just reuse it on those changes rather than re-linking each time - that seems like it would be straightforward
We originally kept the linked copy around and had intended to do what you are saying above but removed it when the minimal Red Hat guys complained about the size of it.
Quoted 2 lines
to do in libsemanage and make those operations significantly faster and less memory intensive.

Stephen Smalley

author

2014-07-25 19:55 UTC #4 lore raw

Effectively it would be another copy of the kernel policy file, just one that is generated before merging local customizations (booleans, users, ports, nodes, interface), so that we can take that kernel policy, read it into a policydb, and mutate it rather than having to re-link the modules to generate another one. Would allow us to avoid module re-linking on all non-module semanage changes IIUC. Could be compressed; just means you have to pay the cost of uncompressing it before using it in libsemanage.

Quoted 28 lines · Daniel J Walsh

How large is it? Does it matter if it is compressed?
Quoted 10 lines · Joshua Brindle
Quoted 8 lines · Stephen Smalley
Motivated by:
https://bugzilla.redhat.com/show_bug.cgi?id=1098446
I believe this is always safe for booleans because we only set their value; we are never adding new ones via semanage, unlike for example users, ports, nodes, and interfaces. For the rest, I was wondering why we don't save the linked file and just reuse it on those changes rather than re-linking each time - that seems like it would be straightforward
Quoted 7 lines
We originally kept the linked copy around and had intended to do what you are saying above but removed it when the minimal Red Hat guys complained about the size of it.
Quoted 2 lines
to do in libsemanage and make those operations significantly faster and less memory intensive.
_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].

Joshua Brindle

2014-07-25 20:04 UTC #5 lore raw

Quoted 9 lines · Stephen Smalley

Effectively it would be another copy of the kernel policy file, just one that is generated before merging local customizations (booleans, users, ports, nodes, interface), so that we can take that kernel policy, read it into a policydb, and mutate it rather than having to re-link the modules to generate another one. Would allow us to avoid module re-linking on all non-module semanage changes IIUC. Could be compressed; just means you have to pay the cost of uncompressing it before using it in libsemanage.

On my Fedora 20 system a linked policy is 32 meg, bzip2 linked policy is 768k.

Quoted 27 lines · Daniel J Walsh

Quoted 25 lines
How large is it? Does it matter if it is compressed?
Quoted 4 lines · Joshua Brindle
Quoted 3 lines · Stephen Smalley
Motivated by:
https://bugzilla.redhat.com/show_bug.cgi?id=1098446
Quoted 11 lines
Quoted 5 lines
I believe this is always safe for booleans because we only set their value; we are never adding new ones via semanage, unlike for example users, ports, nodes, and interfaces. For the rest, I was wondering why we don't save the linked file and just reuse it on those changes rather than re-linking each time - that seems like it would be straightforward
We originally kept the linked copy around and had intended to do what you are saying above but removed it when the minimal Red Hat guys complained about the size of it.
Quoted 2 lines
to do in libsemanage and make those operations significantly faster and less memory intensive.
_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].

Stephen Smalley

author

2014-07-25 20:12 UTC #6 lore raw

Quoted 11 lines · Joshua Brindle

Quoted 9 lines · Stephen Smalley
Effectively it would be another copy of the kernel policy file, just one that is generated before merging local customizations (booleans, users, ports, nodes, interface), so that we can take that kernel policy, read it into a policydb, and mutate it rather than having to re-link the modules to generate another one. Would allow us to avoid module re-linking on all non-module semanage changes IIUC. Could be compressed; just means you have to pay the cost of uncompressing it before using it in libsemanage.

Quoted 2 lines

On my Fedora 20 system a linked policy is 32 meg, bzip2 linked policy is 768k.

I wasn't going to bother with saving the current linked policy, just a copy of the kernel policy before merging local customizations. There is no linked policy in cil (on #integration) so basing anything on it is likely not a good idea, and by writing out the kernel policy before merging, we end up with something that is smaller and more readily usable on the next transaction.

Steve Lawrence

2014-07-29 13:15 UTC #7 lore raw

Quoted 15 lines · Stephen Smalley

Quoted 11 lines · Joshua Brindle
Quoted 9 lines · Stephen Smalley
Effectively it would be another copy of the kernel policy file, just one that is generated before merging local customizations (booleans, users, ports, nodes, interface), so that we can take that kernel policy, read it into a policydb, and mutate it rather than having to re-link the modules to generate another one. Would allow us to avoid module re-linking on all non-module semanage changes IIUC. Could be compressed; just means you have to pay the cost of uncompressing it before using it in libsemanage.
Quoted 2 lines
On my Fedora 20 system a linked policy is 32 meg, bzip2 linked policy is 768k.

Quoted 7 lines

I wasn't going to bother with saving the current linked policy, just a copy of the kernel policy before merging local customizations. There is no linked policy in cil (on #integration) so basing anything on it is likely not a good idea, and by writing out the kernel policy before merging, we end up with something that is smaller and more readily usable on the next transaction.

This is correct. CIL does not generate a linked policy, so in order for this change to be compatible with the CIL integration we would have to store the kernel policy.

Also, this patch looks good to me.

- Steve

Daniel J Walsh

2014-07-28 18:54 UTC #8 lore raw

In reply to Stephen Smalley

Sounds like a reasonable compromise to me.

Quoted 42 lines · Stephen Smalley

Effectively it would be another copy of the kernel policy file, just one that is generated before merging local customizations (booleans, users, ports, nodes, interface), so that we can take that kernel policy, read it into a policydb, and mutate it rather than having to re-link the modules to generate another one. Would allow us to avoid module re-linking on all non-module semanage changes IIUC. Could be compressed; just means you have to pay the cost of uncompressing it before using it in libsemanage.
Quoted 16 lines · Daniel J Walsh
How large is it? Does it matter if it is compressed?
Quoted 13 lines · Joshua Brindle
Quoted 8 lines · Stephen Smalley
Motivated by:
https://bugzilla.redhat.com/show_bug.cgi?id=1098446
I believe this is always safe for booleans because we only set their value; we are never adding new ones via semanage, unlike for example users, ports, nodes, and interfaces. For the rest, I was wondering why we don't save the linked file and just reuse it on those changes rather than re-linking each time - that seems like it would be straightforward
We originally kept the linked copy around and had intended to do what you are saying above but removed it when the minimal Red Hat guys complained about the size of it.
Quoted 9 lines
Quoted 2 lines
Quoted 2 lines
to do in libsemanage and make those operations significantly faster and less memory intensive.
_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].
_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].

Russell Coker

2014-08-05 08:30 UTC #9 lore raw

In reply to Joshua Brindle

Would be nice to have a global system setting regarding whether saving disk space is a priority. Then this could be one of many things that could work differently depending on whether the sysadmin wants to save space.

Quoted 27 lines · Joshua Brindle

Quoted 6 lines · Stephen Smalley
Motivated by:
https://bugzilla.redhat.com/show_bug.cgi?id=1098446
I believe this is always safe for booleans because we only set their value; we are never adding new ones via semanage, unlike for example users, ports, nodes, and interfaces. For the rest, I was wondering
why
Quoted 1 line
we don't save the linked file and just reuse it on those changes
rather
Quoted 1 line
than re-linking each time - that seems like it would be
straightforward
We originally kept the linked copy around and had intended to do what you are saying above but removed it when the minimal Red Hat guys complained about the size of it.
Quoted 1 line
to do in libsemanage and make those operations significantly faster
and
Quoted 1 line
less memory intensive.
_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected]. To get help, send an email containing "help" to [email protected].

Signature · 1 line

Sent from my Samsung Galaxy Note 2 with K-9 Mail.