[RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[Stephen Smalley]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

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.

[-- Attachment #2: 0001-Skip-policy-module-re-link-when-only-setting-boolean.patch --]
[-- Type: text/x-patch, Size: 3953 bytes --]

>From 11f35253194036bde941042fa6a2665297853017 Mon Sep 17 00:00:00 2001
From: Stephen Smalley <sds@tycho.nsa.gov>
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 <sds@tycho.nsa.gov>
---
 libsemanage/src/direct_api.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 64dc7d9..5b94725 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -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);
-- 
1.9.3

Re: [RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[Daniel J Walsh]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

Nice.
On 07/25/2014 01:02 PM, Stephen Smalley wrote:
> 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
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.


[-- Attachment #2: Type: text/html, Size: 1595 bytes --]

Re: [RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[Joshua Brindle]

Stephen Smalley wrote:
> 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.

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

Re: [RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[Daniel J Walsh]

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

On 07/25/2014 03:45 PM, Joshua Brindle wrote:
> Stephen Smalley wrote:
>> 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.
>
>> to do in libsemanage and make those operations significantly faster and
>> less memory intensive.
>

Re: [RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[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 07/25/2014 03:49 PM, Daniel J Walsh wrote:
> How large is it?  Does it matter if it is compressed?
> 
> On 07/25/2014 03:45 PM, Joshua Brindle wrote:
>> Stephen Smalley wrote:
>>> 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.
>>
>>> to do in libsemanage and make those operations significantly faster and
>>> less memory intensive.
>>
> 
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
> 
> 

Re: [RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[Joshua Brindle]

Stephen Smalley wrote:
> 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.

> On 07/25/2014 03:49 PM, Daniel J Walsh wrote:
>> How large is it?  Does it matter if it is compressed?
>>
>> On 07/25/2014 03:45 PM, Joshua Brindle wrote:
>>> Stephen Smalley wrote:
>>>> 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.
>>>
>>>> to do in libsemanage and make those operations significantly faster and
>>>> less memory intensive.
>> _______________________________________________
>> Selinux mailing list
>> Selinux@tycho.nsa.gov
>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>>
>>
>

Re: [RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[Stephen Smalley]

On 07/25/2014 04:04 PM, Joshua Brindle wrote:
> Stephen Smalley wrote:
>> 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.

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.

Re: [RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[Daniel J Walsh]

Sounds like a reasonable compromise to me.

On 07/25/2014 03:55 PM, Stephen Smalley wrote:
> 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 07/25/2014 03:49 PM, Daniel J Walsh wrote:
>> How large is it?  Does it matter if it is compressed?
>>
>> On 07/25/2014 03:45 PM, Joshua Brindle wrote:
>>> Stephen Smalley wrote:
>>>> 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.
>>>
>>>> to do in libsemanage and make those operations significantly faster and
>>>> less memory intensive.
>> _______________________________________________
>> Selinux mailing list
>> Selinux@tycho.nsa.gov
>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>>
>>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>
>

Re: [RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[Steve Lawrence]

On 07/25/2014 04:12 PM, Stephen Smalley wrote:
> On 07/25/2014 04:04 PM, Joshua Brindle wrote:
>> Stephen Smalley wrote:
>>> 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.
> 
> 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

Re: [RFC] [PATCH] libsemanage: Skip policy module re-link when only setting booleans.

[Russell Coker]

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.

On 26 July 2014 5:45:49 AM AEST, Joshua Brindle <brindle@quarksecurity.com> wrote:
>Stephen Smalley wrote:
>> 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.
>
>> to do in libsemanage and make those operations significantly faster
>and
>> less memory intensive.
>
>_______________________________________________
>Selinux mailing list
>Selinux@tycho.nsa.gov
>To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>To get help, send an email containing "help" to
>Selinux-request@tycho.nsa.gov.

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