Real simple cache that removes most of the lookups in mcstrans

Real simple cache that removes most of the lookups in mcstrans

[Daniel J Walsh]

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

Basically check if the previous lookup was the same context, if yes 
return the same translation.   Otherwise do the lookup.

Also included Russells patch for avcstat.

Dan

[-- Attachment #2: libselinux-rhat.patch --]
[-- Type: text/x-patch, Size: 4515 bytes --]

diff --exclude-from=exclude -N -u -r nsalibselinux/src/init.c libselinux-1.30.7/src/init.c
--- nsalibselinux/src/init.c	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/init.c	2006-05-17 06:33:30.000000000 -0400
@@ -78,21 +78,17 @@
 }
 hidden_def(set_selinuxmnt)
  
-static void init_translations(void)
-{
-	init_context_translations();
-}
-
 static void init_lib(void) __attribute__ ((constructor));
 static void init_lib(void)
 {
 	selinux_page_size = sysconf(_SC_PAGE_SIZE);
 	init_selinuxmnt();
-	init_translations();
+	init_context_translations();
 }
 
 static void fini_lib(void) __attribute__ ((destructor));
 static void fini_lib(void)
 {
 	fini_selinuxmnt();
+	fini_context_translations();
 }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/setrans_client.c libselinux-1.30.7/src/setrans_client.c
--- nsalibselinux/src/setrans_client.c	2006-05-16 20:43:27.000000000 -0400
+++ libselinux-1.30.7/src/setrans_client.c	2006-05-17 06:37:59.000000000 -0400
@@ -16,6 +16,11 @@
 #include "selinux_internal.h"
 #include "setrans_internal.h"
 
+// Simple cache
+static	security_context_t prev_t2r_trans=NULL;
+static	security_context_t prev_t2r_raw=NULL;
+static	security_context_t prev_r2t_trans=NULL;
+static	security_context_t prev_r2t_raw=NULL;
 
 /*
  * setransd_open
@@ -194,6 +199,15 @@
 
 
 hidden int
+fini_context_translations(void)
+{
+	free(prev_r2t_trans);
+	free(prev_r2t_raw);
+	free(prev_t2r_trans);
+	free(prev_t2r_raw);
+}
+
+hidden int
 init_context_translations(void)
 {
 	int ret, fd;
@@ -225,9 +239,16 @@
 		*rawp = NULL;
 		return 0;
 	}
-
-	if (trans_to_raw_context(trans, rawp))
-		*rawp = strdup(trans);
+	if (prev_t2r_trans && strcmp(prev_t2r_trans, trans) == 0) {
+		*rawp=strdup(prev_t2r_raw);
+	} else {
+		free(prev_t2r_trans);
+		free(prev_t2r_raw);
+		if (trans_to_raw_context(trans, rawp))
+			*rawp = strdup(trans);
+		prev_t2r_trans=strdup(trans);
+		prev_t2r_raw=strdup(*rawp);
+	}
 	return *rawp ? 0 : -1;
 }
 hidden_def(selinux_trans_to_raw_context)
@@ -240,8 +261,16 @@
 		return 0;
 	}
 
-	if (raw_to_trans_context(raw, transp)) 
-		*transp = strdup(raw);
+	if (prev_r2t_raw && strcmp(prev_r2t_raw, raw) == 0) {
+		*transp=strdup(prev_r2t_trans);
+	} else {
+		free(prev_r2t_raw);
+		free(prev_r2t_trans);
+		if (raw_to_trans_context(raw, transp)) 
+			*transp = strdup(raw);
+		prev_r2t_raw=strdup(raw);
+		prev_r2t_trans=strdup(*transp);
+	}
 
 	return *transp ? 0 : -1;
 }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/setrans_internal.h libselinux-1.30.7/src/setrans_internal.h
--- nsalibselinux/src/setrans_internal.h	2006-05-16 20:43:27.000000000 -0400
+++ libselinux-1.30.7/src/setrans_internal.h	2006-05-17 06:35:09.000000000 -0400
@@ -8,3 +8,4 @@
 #define MAX_DATA_BUF			8192
 
 extern int init_context_translations(void);
+extern int fini_context_translations(void);
diff --exclude-from=exclude -N -u -r nsalibselinux/utils/avcstat.c libselinux-1.30.7/utils/avcstat.c
--- nsalibselinux/utils/avcstat.c	2006-05-15 09:43:20.000000000 -0400
+++ libselinux-1.30.7/utils/avcstat.c	2006-05-17 06:18:39.000000000 -0400
@@ -27,12 +27,12 @@
 #define HEADERS		"lookups hits misses allocations reclaims frees"
 
 struct avc_cache_stats {
-	unsigned int lookups;
-	unsigned int hits;
-	unsigned int misses;
-	unsigned int allocations;
-	unsigned int reclaims;
-	unsigned int frees;
+	unsigned long long lookups;
+	unsigned long long hits;
+	unsigned long long misses;
+	unsigned long long allocations;
+	unsigned long long reclaims;
+	unsigned long long frees;
 };
 
 static int interval;
@@ -172,7 +172,7 @@
 		while ((line = strtok(NULL, "\n"))) {
 			struct avc_cache_stats tmp;
 			
-			ret = sscanf(line, "%u %u %u %u %u %u",
+			ret = sscanf(line, "%Lu %Lu %Lu %Lu %Lu %Lu",
 				     &tmp.lookups,
 				     &tmp.hits,
 				     &tmp.misses,
@@ -195,7 +195,7 @@
 			die("unable to parse \'%s\': no data", avcstatfile);
 
 		if (cumulative || (!cumulative && !i))
-			printf("%10u %10u %10u %10u %10u %10u\n",
+			printf("%10Lu %10Lu %10Lu %10Lu %10Lu %10Lu\n",
 			       tot.lookups, tot.hits, tot.misses,
 			       tot.allocations, tot.reclaims, tot.frees);
 		else {
@@ -205,7 +205,7 @@
 			rel.allocations = tot.allocations - last.allocations;
 			rel.reclaims = tot.reclaims - last.reclaims;
 			rel.frees = tot.frees - last.frees;
-			printf("%10u %10u %10u %10u %10u %10u\n",
+			printf("%10Lu %10Lu %10Lu %10Lu %10Lu %10Lu\n",
 			       rel.lookups, rel.hits, rel.misses,
 			       rel.allocations, rel.reclaims, rel.frees);
 		}

Re: Real simple cache that removes most of the lookups in mcstrans

[Steve Grubb]

On Wednesday 17 May 2006 06:45, Daniel J Walsh wrote:
> Basically check if the previous lookup was the same context, if yes
> return the same translation.   Otherwise do the lookup.

 hidden int
+fini_context_translations(void)
+{
+       free(prev_r2t_trans);
+       free(prev_r2t_raw);
+       free(prev_t2r_trans);
+       free(prev_t2r_raw);
+}
+

This function returns an int. sb return 0?

-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Joshua Brindle]

Daniel J Walsh wrote:
> Basically check if the previous lookup was the same context, if yes 
> return the same translation.   Otherwise do the lookup.
>
> Also included Russells patch for avcstat.
Is this used on MLS? If so this cache needs to be cleared on policy 
reload for proper revocation of translation access. Further, this has no 
way of checking to see if the actual translations changed between the 
last query and this one.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Daniel J Walsh]

Joshua Brindle wrote:
> Daniel J Walsh wrote:
>> Basically check if the previous lookup was the same context, if yes 
>> return the same translation.   Otherwise do the lookup.
>>
>> Also included Russells patch for avcstat.
> Is this used on MLS? If so this cache needs to be cleared on policy 
> reload for proper revocation of translation access. Further, this has 
> no way of checking to see if the actual translations changed between 
> the last query and this one.
Yes, you mean translation change, I think.  Since I do not see where 
policy reload would effect this.  We could add some kind of timer to 
this, but refreshing the cache is a small problem, but this same problem 
existed with shared libraries.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Joshua Brindle]

Daniel J Walsh wrote:
> Joshua Brindle wrote:
>> Daniel J Walsh wrote:
>>> Basically check if the previous lookup was the same context, if yes 
>>> return the same translation.   Otherwise do the lookup.
>>>
>>> Also included Russells patch for avcstat.
>> Is this used on MLS? If so this cache needs to be cleared on policy 
>> reload for proper revocation of translation access. Further, this has 
>> no way of checking to see if the actual translations changed between 
>> the last query and this one.
> Yes, you mean translation change, I think.  Since I do not see where 
> policy reload would effect this.  We could add some kind of timer to 
> this, but refreshing the cache is a small problem, but this same 
> problem existed with shared libraries.
>
Because on an MLS system the server will be deciding whether or not you 
have permission to see a particular translation and a policy reload 
would affect that. The same problem did exist with the shared libraries 
but we aren't using them anymore :) the server should be smart enough to 
handle this. If client side caching is really desired it needs to work 
like an avc where you can get flush notifications from the server.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Daniel J Walsh]

James Antill wrote:
> On Wed, 2006-05-17 at 06:45 -0400, Daniel J Walsh wrote:
>   
>> Basically check if the previous lookup was the same context, if yes 
>> return the same translation.   Otherwise do the lookup.
>>     
>
>  Are we sure one is enough, lsof -Z does both s0 and s0-s0:c0.c255 and
> they are basically mixed (I appreciate it is much easier to code a cache
> of 1 than >1 though).
>
>  Also:
>
> +       } else {
> +               free(prev_t2r_trans);
> +               free(prev_t2r_raw);
> +               if (trans_to_raw_context(trans, rawp))
> +                       *rawp = strdup(trans);
> +               prev_t2r_trans=strdup(trans);
> +               prev_t2r_raw=strdup(*rawp);
>
> ...this is bad when trans_to_raw_context, the first strdup() fails or
> the last strdup() fails.
>
>   
Only reason strdup fails is ENOMEM.  With ENOMEM you are almost 
garanteed you are going to crash anyways.  gstrdup does a exit when it 
runs out of memory.
So we can messy up the code with a lot of checks that end up doing 
little.  Your choice.
>> Also included Russells patch for avcstat.
>>     
>
> -                       printf("%10u %10u %10u %10u %10u %10u\n",
> +                       printf("%10Lu %10Lu %10Lu %10Lu %10Lu %10Lu\n",
>
>  It's somewhat minor, but that should be %llu %Lu is a somewhat common
> extension. From man printf:
>
>        ll     (ell-ell).  A following integer conversion corresponds to a long
>               long int or unsigned long long int argument, or  a  following  n
>               conversion corresponds to a pointer to a long long int argument.
>
>        L      A following a, A, e, E, f, F, g, or G conversion corresponds  to
>               a long double argument.  (C99 allows %LF, but SUSv2 does not.)
>
>
>   


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Daniel J Walsh]

Joshua Brindle wrote:
> Daniel J Walsh wrote:
>> Joshua Brindle wrote:
>>> Daniel J Walsh wrote:
>>>> Basically check if the previous lookup was the same context, if yes 
>>>> return the same translation.   Otherwise do the lookup.
>>>>
>>>> Also included Russells patch for avcstat.
>>> Is this used on MLS? If so this cache needs to be cleared on policy 
>>> reload for proper revocation of translation access. Further, this 
>>> has no way of checking to see if the actual translations changed 
>>> between the last query and this one.
>> Yes, you mean translation change, I think.  Since I do not see where 
>> policy reload would effect this.  We could add some kind of timer to 
>> this, but refreshing the cache is a small problem, but this same 
>> problem existed with shared libraries.
>>
> Because on an MLS system the server will be deciding whether or not 
> you have permission to see a particular translation and a policy 
> reload would affect that. The same problem did exist with the shared 
> libraries but we aren't using them anymore :) the server should be 
> smart enough to handle this. If client side caching is really desired 
> it needs to work like an avc where you can get flush notifications 
> from the server.
One suggestion I have heard is to allow the administrator to turn off 
the cache, perhaps via /etc/selinux/config???



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[James Antill]

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

On Wed, 2006-05-17 at 06:45 -0400, Daniel J Walsh wrote:
> Basically check if the previous lookup was the same context, if yes 
> return the same translation.   Otherwise do the lookup.

 Are we sure one is enough, lsof -Z does both s0 and s0-s0:c0.c255 and
they are basically mixed (I appreciate it is much easier to code a cache
of 1 than >1 though).

 Also:

+       } else {
+               free(prev_t2r_trans);
+               free(prev_t2r_raw);
+               if (trans_to_raw_context(trans, rawp))
+                       *rawp = strdup(trans);
+               prev_t2r_trans=strdup(trans);
+               prev_t2r_raw=strdup(*rawp);

...this is bad when trans_to_raw_context, the first strdup() fails or
the last strdup() fails.

> Also included Russells patch for avcstat.

-                       printf("%10u %10u %10u %10u %10u %10u\n",
+                       printf("%10Lu %10Lu %10Lu %10Lu %10Lu %10Lu\n",

 It's somewhat minor, but that should be %llu %Lu is a somewhat common
extension. From man printf:

       ll     (ell-ell).  A following integer conversion corresponds to a long
              long int or unsigned long long int argument, or  a  following  n
              conversion corresponds to a pointer to a long long int argument.

       L      A following a, A, e, E, f, F, g, or G conversion corresponds  to
              a long double argument.  (C99 allows %LF, but SUSv2 does not.)

-- 
James Antill
<james.antill@redhat.com>


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: Real simple cache that removes most of the lookups in mcstrans

[James Antill]

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

On Wed, 2006-05-17 at 12:19 -0400, Daniel J Walsh wrote:

> Only reason strdup fails is ENOMEM.  With ENOMEM you are almost 
> garanteed you are going to crash anyways.  gstrdup does a exit when it 
> runs out of memory.
> So we can messy up the code with a lot of checks that end up doing 
> little.  Your choice.

 Well, it doesn't currently fail that way ... and it's relying on a side
effect that sometimes happens.
 Also the grep's I can see show the malloc/realloc/strdup failures being
handled, so without bugs in the application layer ENOMEM doesn't
currently mean a crash.

 I'd be happy to do the diffs to add an internal
xstrdup()/xmalloc()/etc. which calls abort(), if that's the route we
want to go. As you say, this would significantly reduce the failure
paths ... although it might get libselinux banned from some
applications.
 I'll also volunteer to do an audit and add the failure paths, if we
want to handle ENOMEM.

-- 
James Antill
<james.antill@redhat.com>


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: Real simple cache that removes most of the lookups in mcstrans

[Stephen Smalley]

On Wed, 2006-05-17 at 12:56 -0400, James Antill wrote:
> On Wed, 2006-05-17 at 12:19 -0400, Daniel J Walsh wrote:
> 
> > Only reason strdup fails is ENOMEM.  With ENOMEM you are almost 
> > garanteed you are going to crash anyways.  gstrdup does a exit when it 
> > runs out of memory.
> > So we can messy up the code with a lot of checks that end up doing 
> > little.  Your choice.
> 
>  Well, it doesn't currently fail that way ... and it's relying on a side
> effect that sometimes happens.
>  Also the grep's I can see show the malloc/realloc/strdup failures being
> handled, so without bugs in the application layer ENOMEM doesn't
> currently mean a crash.
> 
>  I'd be happy to do the diffs to add an internal
> xstrdup()/xmalloc()/etc. which calls abort(), if that's the route we
> want to go. As you say, this would significantly reduce the failure
> paths ... although it might get libselinux banned from some
> applications.
>  I'll also volunteer to do an audit and add the failure paths, if we
> want to handle ENOMEM.

libselinux functions shouldn't crash or abort on ENOMEM, so I'd want
them handled properly prior to merging this patch.  A more general cache
would be nice too ;)

-- 
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 majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Joshua Brindle]

Daniel J Walsh wrote:
> Joshua Brindle wrote:
>> Daniel J Walsh wrote:
>>> Joshua Brindle wrote:
>>>> Daniel J Walsh wrote:
>>>>> Basically check if the previous lookup was the same context, if 
>>>>> yes return the same translation.   Otherwise do the lookup.
>>>>>
>>>>> Also included Russells patch for avcstat.
>>>> Is this used on MLS? If so this cache needs to be cleared on policy 
>>>> reload for proper revocation of translation access. Further, this 
>>>> has no way of checking to see if the actual translations changed 
>>>> between the last query and this one.
>>> Yes, you mean translation change, I think.  Since I do not see where 
>>> policy reload would effect this.  We could add some kind of timer to 
>>> this, but refreshing the cache is a small problem, but this same 
>>> problem existed with shared libraries.
>>>
>> Because on an MLS system the server will be deciding whether or not 
>> you have permission to see a particular translation and a policy 
>> reload would affect that. The same problem did exist with the shared 
>> libraries but we aren't using them anymore :) the server should be 
>> smart enough to handle this. If client side caching is really desired 
>> it needs to work like an avc where you can get flush notifications 
>> from the server.
> One suggestion I have heard is to allow the administrator to turn off 
> the cache, perhaps via /etc/selinux/config???


That sounds good, will this be included in the next patch?

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Steve Grubb]

On Wednesday 17 May 2006 13:09, Joshua Brindle wrote:
> > One suggestion I have heard is to allow the administrator to turn off
> > the cache, perhaps via /etc/selinux/config???
>
> That sounds good, will this be included in the next patch?

I think if you have to parse /etc/selinux/config, you've lost all performance 
gains from caching. May as well not have this patch and call the daemon 
everytime.

-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Joshua Brindle]

Steve Grubb wrote:
> On Wednesday 17 May 2006 13:09, Joshua Brindle wrote:
>   
>>> One suggestion I have heard is to allow the administrator to turn off
>>> the cache, perhaps via /etc/selinux/config???
>>>       
>> That sounds good, will this be included in the next patch?
>>     
>
> I think if you have to parse /etc/selinux/config, you've lost all performance 
> gains from caching. May as well not have this patch and call the daemon 
> everytime.
>
>   
IIUC you'll only parse it on setrans_client startup so once per 
application, and you have to parse /etc/selinux/config at that time anyway.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Daniel J Walsh]

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

Updated patch with

void fini_context_translations

memory checking on strdup

CACHETRANS flag in /etc/selinux/config to turn off cacheing

Lu -> llu change

I will leave a more complete caching solution for later...  This handled 
99.9% of the cases.

[-- Attachment #2: libselinux-rhat.patch --]
[-- Type: text/x-patch, Size: 6216 bytes --]

diff --exclude-from=exclude -N -u -r nsalibselinux/src/init.c libselinux-1.30.7/src/init.c
--- nsalibselinux/src/init.c	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/init.c	2006-05-17 13:57:29.000000000 -0400
@@ -78,21 +78,17 @@
 }
 hidden_def(set_selinuxmnt)
  
-static void init_translations(void)
-{
-	init_context_translations();
-}
-
 static void init_lib(void) __attribute__ ((constructor));
 static void init_lib(void)
 {
 	selinux_page_size = sysconf(_SC_PAGE_SIZE);
 	init_selinuxmnt();
-	init_translations();
+	init_context_translations();
 }
 
 static void fini_lib(void) __attribute__ ((destructor));
 static void fini_lib(void)
 {
 	fini_selinuxmnt();
+	fini_context_translations();
 }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/selinux_config.c libselinux-1.30.7/src/selinux_config.c
--- nsalibselinux/src/selinux_config.c	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/selinux_config.c	2006-05-17 14:00:27.000000000 -0400
@@ -17,6 +17,7 @@
 #define SELINUXTAG "SELINUX="
 #define SETLOCALDEFS "SETLOCALDEFS="
 #define REQUIRESEUSERS "REQUIRESEUSERS="
+#define CACHETRANSTAG "CACHETRANS="
 
 /* Indices for file paths arrays. */
 #define BINPOLICY         0
@@ -175,6 +176,10 @@
 				      sizeof(REQUIRESEUSERS)-1)) {
 			  value = buf_p + sizeof(REQUIRESEUSERS)-1;
 			  intptr = &require_seusers;
+		  } else if (!strncmp(buf_p, CACHETRANS, 
+				      sizeof(CACHETRANS)-1)) {
+			  value = buf_p + sizeof(CACHETRANS)-1;
+			  intptr = &cache_trans;
 		  } else {
 			  continue;
 		  }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/selinux_internal.h libselinux-1.30.7/src/selinux_internal.h
--- nsalibselinux/src/selinux_internal.h	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/selinux_internal.h	2006-05-17 14:05:25.000000000 -0400
@@ -70,3 +70,4 @@
 extern int load_setlocaldefs hidden;
 extern int require_seusers hidden;
 extern int selinux_page_size hidden;
+extern int cache_trans hidden;
diff --exclude-from=exclude -N -u -r nsalibselinux/src/setrans_client.c libselinux-1.30.7/src/setrans_client.c
--- nsalibselinux/src/setrans_client.c	2006-05-16 20:43:27.000000000 -0400
+++ libselinux-1.30.7/src/setrans_client.c	2006-05-17 14:15:33.000000000 -0400
@@ -16,6 +16,13 @@
 #include "selinux_internal.h"
 #include "setrans_internal.h"
 
+// Simple cache
+static	security_context_t prev_t2r_trans=NULL;
+static	security_context_t prev_t2r_raw=NULL;
+static	security_context_t prev_r2t_trans=NULL;
+static	security_context_t prev_r2t_raw=NULL;
+
+int cache_trans hidden = 1;
 
 /*
  * setransd_open
@@ -193,6 +200,17 @@
 }
 
 
+hidden void
+fini_context_translations(void)
+{
+	if (cache_trans) {
+		free(prev_r2t_trans);
+		free(prev_r2t_raw);
+		free(prev_t2r_trans);
+		free(prev_t2r_raw);
+	}
+}
+
 hidden int
 init_context_translations(void)
 {
@@ -225,9 +243,24 @@
 		*rawp = NULL;
 		return 0;
 	}
+	if (cache_trans) {
+		if (prev_t2r_trans && strcmp(prev_t2r_trans, trans) == 0) {
+			*rawp=strdup(prev_t2r_raw);
+		} else {
+			free(prev_t2r_trans); prev_t2r_trans = NULL;
+			free(prev_t2r_raw); prev_t2r_raw = NULL
+			if (trans_to_raw_context(trans, rawp))
+				*rawp = strdup(trans);
+			if (*rawp) {
+				prev_t2r_trans=strdup(trans);
+				prev_t2r_raw=strdup(*rawp);
+			}
+		}
+	}
+	else 
+		if (trans_to_raw_context(trans, rawp))
+			*rawp = strdup(trans);
 
-	if (trans_to_raw_context(trans, rawp))
-		*rawp = strdup(trans);
 	return *rawp ? 0 : -1;
 }
 hidden_def(selinux_trans_to_raw_context)
@@ -240,8 +273,23 @@
 		return 0;
 	}
 
-	if (raw_to_trans_context(raw, transp)) 
-		*transp = strdup(raw);
+	if (cache_trans) {
+		if (prev_r2t_raw && strcmp(prev_r2t_raw, raw) == 0) {
+			*transp=strdup(prev_r2t_trans);
+		} else {
+			free(prev_r2t_raw); prev_r2t_raw = NULL;
+			free(prev_r2t_trans); prev_r2t_trans = NULL;
+			if (raw_to_trans_context(raw, transp)) 
+				*transp = strdup(raw);
+			if (*transp) {
+				prev_r2t_raw=strdup(raw);
+				prev_r2t_trans=strdup(*transp);
+			}
+		}
+	} 
+	else 
+		if (raw_to_trans_context(raw, transp)) 
+			*transp = strdup(raw);
 
 	return *transp ? 0 : -1;
 }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/setrans_internal.h libselinux-1.30.7/src/setrans_internal.h
--- nsalibselinux/src/setrans_internal.h	2006-05-16 20:43:27.000000000 -0400
+++ libselinux-1.30.7/src/setrans_internal.h	2006-05-17 14:07:34.000000000 -0400
@@ -8,3 +8,4 @@
 #define MAX_DATA_BUF			8192
 
 extern int init_context_translations(void);
+extern void fini_context_translations(void);
diff --exclude-from=exclude -N -u -r nsalibselinux/utils/avcstat.c libselinux-1.30.7/utils/avcstat.c
--- nsalibselinux/utils/avcstat.c	2006-05-15 09:43:20.000000000 -0400
+++ libselinux-1.30.7/utils/avcstat.c	2006-05-17 06:18:39.000000000 -0400
@@ -27,12 +27,12 @@
 #define HEADERS		"lookups hits misses allocations reclaims frees"
 
 struct avc_cache_stats {
-	unsigned int lookups;
-	unsigned int hits;
-	unsigned int misses;
-	unsigned int allocations;
-	unsigned int reclaims;
-	unsigned int frees;
+	unsigned long long lookups;
+	unsigned long long hits;
+	unsigned long long misses;
+	unsigned long long allocations;
+	unsigned long long reclaims;
+	unsigned long long frees;
 };
 
 static int interval;
@@ -172,7 +172,7 @@
 		while ((line = strtok(NULL, "\n"))) {
 			struct avc_cache_stats tmp;
 			
-			ret = sscanf(line, "%u %u %u %u %u %u",
+			ret = sscanf(line, "%llu %llu %llu %llu %llu %llu",
 				     &tmp.lookups,
 				     &tmp.hits,
 				     &tmp.misses,
@@ -195,7 +195,7 @@
 			die("unable to parse \'%s\': no data", avcstatfile);
 
 		if (cumulative || (!cumulative && !i))
-			printf("%10u %10u %10u %10u %10u %10u\n",
+			printf("%10llu %10llu %10llu %10llu %10llu %10llu\n",
 			       tot.lookups, tot.hits, tot.misses,
 			       tot.allocations, tot.reclaims, tot.frees);
 		else {
@@ -205,7 +205,7 @@
 			rel.allocations = tot.allocations - last.allocations;
 			rel.reclaims = tot.reclaims - last.reclaims;
 			rel.frees = tot.frees - last.frees;
-			printf("%10u %10u %10u %10u %10u %10u\n",
+			printf("%10llu %10llu %10llu %10llu %10llu %10llu\n",
 			       rel.lookups, rel.hits, rel.misses,
 			       rel.allocations, rel.reclaims, rel.frees);
 		}

Resent with correct patch

[Daniel J Walsh]

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

Updated patch with

void fini_context_translations

memory checking on strdup

CACHETRANS flag in /etc/selinux/config to turn off cacheing

Lu -> llu change

I will leave a more complete caching solution for later...  This handled
99.9% of the cases.


[-- Attachment #2: libselinux-rhat.patch --]
[-- Type: text/x-patch, Size: 6208 bytes --]

diff --exclude-from=exclude -N -u -r nsalibselinux/src/init.c libselinux-1.30.7/src/init.c
--- nsalibselinux/src/init.c	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/init.c	2006-05-17 13:57:29.000000000 -0400
@@ -78,21 +78,17 @@
 }
 hidden_def(set_selinuxmnt)
  
-static void init_translations(void)
-{
-	init_context_translations();
-}
-
 static void init_lib(void) __attribute__ ((constructor));
 static void init_lib(void)
 {
 	selinux_page_size = sysconf(_SC_PAGE_SIZE);
 	init_selinuxmnt();
-	init_translations();
+	init_context_translations();
 }
 
 static void fini_lib(void) __attribute__ ((destructor));
 static void fini_lib(void)
 {
 	fini_selinuxmnt();
+	fini_context_translations();
 }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/selinux_config.c libselinux-1.30.7/src/selinux_config.c
--- nsalibselinux/src/selinux_config.c	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/selinux_config.c	2006-05-17 14:31:07.000000000 -0400
@@ -17,6 +17,7 @@
 #define SELINUXTAG "SELINUX="
 #define SETLOCALDEFS "SETLOCALDEFS="
 #define REQUIRESEUSERS "REQUIRESEUSERS="
+#define CACHETRANSTAG "CACHETRANS="
 
 /* Indices for file paths arrays. */
 #define BINPOLICY         0
@@ -175,6 +176,10 @@
 				      sizeof(REQUIRESEUSERS)-1)) {
 			  value = buf_p + sizeof(REQUIRESEUSERS)-1;
 			  intptr = &require_seusers;
+		  } else if (!strncmp(buf_p, CACHETRANSTAG, 
+				      sizeof(CACHETRANSTAG)-1)) {
+			  value = buf_p + sizeof(CACHETRANSTAG)-1;
+			  intptr = &cache_trans;
 		  } else {
 			  continue;
 		  }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/selinux_internal.h libselinux-1.30.7/src/selinux_internal.h
--- nsalibselinux/src/selinux_internal.h	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/selinux_internal.h	2006-05-17 14:05:25.000000000 -0400
@@ -70,3 +70,4 @@
 extern int load_setlocaldefs hidden;
 extern int require_seusers hidden;
 extern int selinux_page_size hidden;
+extern int cache_trans hidden;
diff --exclude-from=exclude -N -u -r nsalibselinux/src/setrans_client.c libselinux-1.30.7/src/setrans_client.c
--- nsalibselinux/src/setrans_client.c	2006-05-16 20:43:27.000000000 -0400
+++ libselinux-1.30.7/src/setrans_client.c	2006-05-17 14:32:31.000000000 -0400
@@ -16,6 +16,13 @@
 #include "selinux_internal.h"
 #include "setrans_internal.h"
 
+// Simple cache
+static	security_context_t prev_t2r_trans=NULL;
+static	security_context_t prev_t2r_raw=NULL;
+static	security_context_t prev_r2t_trans=NULL;
+static	security_context_t prev_r2t_raw=NULL;
+
+int cache_trans hidden = 1;
 
 /*
  * setransd_open
@@ -193,6 +200,17 @@
 }
 
 
+hidden void
+fini_context_translations(void)
+{
+	if (cache_trans) {
+		free(prev_r2t_trans);
+		free(prev_r2t_raw);
+		free(prev_t2r_trans);
+		free(prev_t2r_raw);
+	}
+}
+
 hidden int
 init_context_translations(void)
 {
@@ -225,9 +243,24 @@
 		*rawp = NULL;
 		return 0;
 	}
+	if (cache_trans) {
+		if (prev_t2r_trans && strcmp(prev_t2r_trans, trans) == 0) {
+			*rawp=strdup(prev_t2r_raw);
+		} else {
+			free(prev_t2r_trans); prev_t2r_trans = NULL;
+			free(prev_t2r_raw); prev_t2r_raw = NULL;
+			if (trans_to_raw_context(trans, rawp))
+				*rawp = strdup(trans);
+			if (*rawp) {
+				prev_t2r_trans=strdup(trans);
+				prev_t2r_raw=strdup(*rawp);
+			}
+		}
+	}
+	else 
+		if (trans_to_raw_context(trans, rawp))
+			*rawp = strdup(trans);
 
-	if (trans_to_raw_context(trans, rawp))
-		*rawp = strdup(trans);
 	return *rawp ? 0 : -1;
 }
 hidden_def(selinux_trans_to_raw_context)
@@ -240,8 +273,23 @@
 		return 0;
 	}
 
-	if (raw_to_trans_context(raw, transp)) 
-		*transp = strdup(raw);
+	if (cache_trans) {
+		if (prev_r2t_raw && strcmp(prev_r2t_raw, raw) == 0) {
+			*transp=strdup(prev_r2t_trans);
+		} else {
+			free(prev_r2t_raw); prev_r2t_raw = NULL;
+			free(prev_r2t_trans); prev_r2t_trans = NULL;
+			if (raw_to_trans_context(raw, transp)) 
+				*transp = strdup(raw);
+			if (*transp) {
+				prev_r2t_raw=strdup(raw);
+				prev_r2t_trans=strdup(*transp);
+			}
+		}
+	} 
+	else 
+		if (raw_to_trans_context(raw, transp)) 
+			*transp = strdup(raw);
 
 	return *transp ? 0 : -1;
 }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/setrans_internal.h libselinux-1.30.7/src/setrans_internal.h
--- nsalibselinux/src/setrans_internal.h	2006-05-16 20:43:27.000000000 -0400
+++ libselinux-1.30.7/src/setrans_internal.h	2006-05-17 14:07:34.000000000 -0400
@@ -8,3 +8,4 @@
 #define MAX_DATA_BUF			8192
 
 extern int init_context_translations(void);
+extern void fini_context_translations(void);
diff --exclude-from=exclude -N -u -r nsalibselinux/utils/avcstat.c libselinux-1.30.7/utils/avcstat.c
--- nsalibselinux/utils/avcstat.c	2006-05-15 09:43:20.000000000 -0400
+++ libselinux-1.30.7/utils/avcstat.c	2006-05-17 06:18:39.000000000 -0400
@@ -27,12 +27,12 @@
 #define HEADERS		"lookups hits misses allocations reclaims frees"
 
 struct avc_cache_stats {
-	unsigned int lookups;
-	unsigned int hits;
-	unsigned int misses;
-	unsigned int allocations;
-	unsigned int reclaims;
-	unsigned int frees;
+	unsigned long long lookups;
+	unsigned long long hits;
+	unsigned long long misses;
+	unsigned long long allocations;
+	unsigned long long reclaims;
+	unsigned long long frees;
 };
 
 static int interval;
@@ -172,7 +172,7 @@
 		while ((line = strtok(NULL, "\n"))) {
 			struct avc_cache_stats tmp;
 			
-			ret = sscanf(line, "%u %u %u %u %u %u",
+			ret = sscanf(line, "%Lu %Lu %Lu %Lu %Lu %Lu",
 				     &tmp.lookups,
 				     &tmp.hits,
 				     &tmp.misses,
@@ -195,7 +195,7 @@
 			die("unable to parse \'%s\': no data", avcstatfile);
 
 		if (cumulative || (!cumulative && !i))
-			printf("%10u %10u %10u %10u %10u %10u\n",
+			printf("%10Lu %10Lu %10Lu %10Lu %10Lu %10Lu\n",
 			       tot.lookups, tot.hits, tot.misses,
 			       tot.allocations, tot.reclaims, tot.frees);
 		else {
@@ -205,7 +205,7 @@
 			rel.allocations = tot.allocations - last.allocations;
 			rel.reclaims = tot.reclaims - last.reclaims;
 			rel.frees = tot.frees - last.frees;
-			printf("%10u %10u %10u %10u %10u %10u\n",
+			printf("%10Lu %10Lu %10Lu %10Lu %10Lu %10Lu\n",
 			       rel.lookups, rel.hits, rel.misses,
 			       rel.allocations, rel.reclaims, rel.frees);
 		}

Re: Resent with correct patch

[Stephen Smalley]

On Wed, 2006-05-17 at 14:35 -0400, Daniel J Walsh wrote:
> Updated patch with
> 
> void fini_context_translations
> 
> memory checking on strdup
> 
> CACHETRANS flag in /etc/selinux/config to turn off cacheing
> 
> Lu -> llu change
> 
> I will leave a more complete caching solution for later...  This handled
> 99.9% of the cases.

Could be a problem for multi-threaded apps, and those could easily end
up calling functions like lgetfilecon.

-- 
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 majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Daniel J Walsh]

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

Stephen Smalley wrote:
> On Wed, 2006-05-17 at 12:56 -0400, James Antill wrote:
>   
>> On Wed, 2006-05-17 at 12:19 -0400, Daniel J Walsh wrote:
>>
>>     
>>> Only reason strdup fails is ENOMEM.  With ENOMEM you are almost 
>>> garanteed you are going to crash anyways.  gstrdup does a exit when it 
>>> runs out of memory.
>>> So we can messy up the code with a lot of checks that end up doing 
>>> little.  Your choice.
>>>       
>>  Well, it doesn't currently fail that way ... and it's relying on a side
>> effect that sometimes happens.
>>  Also the grep's I can see show the malloc/realloc/strdup failures being
>> handled, so without bugs in the application layer ENOMEM doesn't
>> currently mean a crash.
>>
>>  I'd be happy to do the diffs to add an internal
>> xstrdup()/xmalloc()/etc. which calls abort(), if that's the route we
>> want to go. As you say, this would significantly reduce the failure
>> paths ... although it might get libselinux banned from some
>> applications.
>>  I'll also volunteer to do an audit and add the failure paths, if we
>> want to handle ENOMEM.
>>     
>
> libselinux functions shouldn't crash or abort on ENOMEM, so I'd want
> them handled properly prior to merging this patch.  A more general cache
> would be nice too ;)
>
>   


As for thread safe, I added
+static __thread security_context_t prev_t2r_trans=NULL;
+static __thread security_context_t prev_t2r_raw=NULL;
+static __thread security_context_t prev_r2t_trans=NULL;
+static __thread security_context_t prev_r2t_raw=NULL;

After talking we James Antill we feel that we should enhance this a 
little to allow a linked list of previous translated security context.
ps,lsof, netstat all would benefit.

But for  now this improves the ls -Z performance significantly.


[-- Attachment #2: libselinux-rhat.patch --]
[-- Type: text/x-patch, Size: 6244 bytes --]

diff --exclude-from=exclude -N -u -r nsalibselinux/src/init.c libselinux-1.30.7/src/init.c
--- nsalibselinux/src/init.c	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/init.c	2006-05-17 13:57:29.000000000 -0400
@@ -78,21 +78,17 @@
 }
 hidden_def(set_selinuxmnt)
  
-static void init_translations(void)
-{
-	init_context_translations();
-}
-
 static void init_lib(void) __attribute__ ((constructor));
 static void init_lib(void)
 {
 	selinux_page_size = sysconf(_SC_PAGE_SIZE);
 	init_selinuxmnt();
-	init_translations();
+	init_context_translations();
 }
 
 static void fini_lib(void) __attribute__ ((destructor));
 static void fini_lib(void)
 {
 	fini_selinuxmnt();
+	fini_context_translations();
 }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/selinux_config.c libselinux-1.30.7/src/selinux_config.c
--- nsalibselinux/src/selinux_config.c	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/selinux_config.c	2006-05-17 14:31:07.000000000 -0400
@@ -17,6 +17,7 @@
 #define SELINUXTAG "SELINUX="
 #define SETLOCALDEFS "SETLOCALDEFS="
 #define REQUIRESEUSERS "REQUIRESEUSERS="
+#define CACHETRANSTAG "CACHETRANS="
 
 /* Indices for file paths arrays. */
 #define BINPOLICY         0
@@ -175,6 +176,10 @@
 				      sizeof(REQUIRESEUSERS)-1)) {
 			  value = buf_p + sizeof(REQUIRESEUSERS)-1;
 			  intptr = &require_seusers;
+		  } else if (!strncmp(buf_p, CACHETRANSTAG, 
+				      sizeof(CACHETRANSTAG)-1)) {
+			  value = buf_p + sizeof(CACHETRANSTAG)-1;
+			  intptr = &cache_trans;
 		  } else {
 			  continue;
 		  }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/selinux_internal.h libselinux-1.30.7/src/selinux_internal.h
--- nsalibselinux/src/selinux_internal.h	2006-05-15 09:43:24.000000000 -0400
+++ libselinux-1.30.7/src/selinux_internal.h	2006-05-17 14:05:25.000000000 -0400
@@ -70,3 +70,4 @@
 extern int load_setlocaldefs hidden;
 extern int require_seusers hidden;
 extern int selinux_page_size hidden;
+extern int cache_trans hidden;
diff --exclude-from=exclude -N -u -r nsalibselinux/src/setrans_client.c libselinux-1.30.7/src/setrans_client.c
--- nsalibselinux/src/setrans_client.c	2006-05-16 20:43:27.000000000 -0400
+++ libselinux-1.30.7/src/setrans_client.c	2006-05-17 18:17:41.000000000 -0400
@@ -16,6 +16,13 @@
 #include "selinux_internal.h"
 #include "setrans_internal.h"
 
+// Simple cache
+static	__thread security_context_t prev_t2r_trans=NULL;
+static	__thread security_context_t prev_t2r_raw=NULL;
+static	__thread security_context_t prev_r2t_trans=NULL;
+static	__thread security_context_t prev_r2t_raw=NULL;
+
+int cache_trans hidden = 1;
 
 /*
  * setransd_open
@@ -193,6 +200,17 @@
 }
 
 
+hidden void
+fini_context_translations(void)
+{
+	if (cache_trans) {
+		free(prev_r2t_trans);
+		free(prev_r2t_raw);
+		free(prev_t2r_trans);
+		free(prev_t2r_raw);
+	}
+}
+
 hidden int
 init_context_translations(void)
 {
@@ -225,9 +243,24 @@
 		*rawp = NULL;
 		return 0;
 	}
+	if (cache_trans) {
+		if (prev_t2r_trans && strcmp(prev_t2r_trans, trans) == 0) {
+			*rawp=strdup(prev_t2r_raw);
+		} else {
+			free(prev_t2r_trans); prev_t2r_trans = NULL;
+			free(prev_t2r_raw); prev_t2r_raw = NULL;
+			if (trans_to_raw_context(trans, rawp))
+				*rawp = strdup(trans);
+			if (*rawp) {
+				prev_t2r_trans=strdup(trans);
+				prev_t2r_raw=strdup(*rawp);
+			}
+		}
+	}
+	else 
+		if (trans_to_raw_context(trans, rawp))
+			*rawp = strdup(trans);
 
-	if (trans_to_raw_context(trans, rawp))
-		*rawp = strdup(trans);
 	return *rawp ? 0 : -1;
 }
 hidden_def(selinux_trans_to_raw_context)
@@ -240,8 +273,23 @@
 		return 0;
 	}
 
-	if (raw_to_trans_context(raw, transp)) 
-		*transp = strdup(raw);
+	if (cache_trans) {
+		if (prev_r2t_raw && strcmp(prev_r2t_raw, raw) == 0) {
+			*transp=strdup(prev_r2t_trans);
+		} else {
+			free(prev_r2t_raw); prev_r2t_raw = NULL;
+			free(prev_r2t_trans); prev_r2t_trans = NULL;
+			if (raw_to_trans_context(raw, transp)) 
+				*transp = strdup(raw);
+			if (*transp) {
+				prev_r2t_raw=strdup(raw);
+				prev_r2t_trans=strdup(*transp);
+			}
+		}
+	} 
+	else 
+		if (raw_to_trans_context(raw, transp)) 
+			*transp = strdup(raw);
 
 	return *transp ? 0 : -1;
 }
diff --exclude-from=exclude -N -u -r nsalibselinux/src/setrans_internal.h libselinux-1.30.7/src/setrans_internal.h
--- nsalibselinux/src/setrans_internal.h	2006-05-16 20:43:27.000000000 -0400
+++ libselinux-1.30.7/src/setrans_internal.h	2006-05-17 14:07:34.000000000 -0400
@@ -8,3 +8,4 @@
 #define MAX_DATA_BUF			8192
 
 extern int init_context_translations(void);
+extern void fini_context_translations(void);
diff --exclude-from=exclude -N -u -r nsalibselinux/utils/avcstat.c libselinux-1.30.7/utils/avcstat.c
--- nsalibselinux/utils/avcstat.c	2006-05-15 09:43:20.000000000 -0400
+++ libselinux-1.30.7/utils/avcstat.c	2006-05-17 06:18:39.000000000 -0400
@@ -27,12 +27,12 @@
 #define HEADERS		"lookups hits misses allocations reclaims frees"
 
 struct avc_cache_stats {
-	unsigned int lookups;
-	unsigned int hits;
-	unsigned int misses;
-	unsigned int allocations;
-	unsigned int reclaims;
-	unsigned int frees;
+	unsigned long long lookups;
+	unsigned long long hits;
+	unsigned long long misses;
+	unsigned long long allocations;
+	unsigned long long reclaims;
+	unsigned long long frees;
 };
 
 static int interval;
@@ -172,7 +172,7 @@
 		while ((line = strtok(NULL, "\n"))) {
 			struct avc_cache_stats tmp;
 			
-			ret = sscanf(line, "%u %u %u %u %u %u",
+			ret = sscanf(line, "%Lu %Lu %Lu %Lu %Lu %Lu",
 				     &tmp.lookups,
 				     &tmp.hits,
 				     &tmp.misses,
@@ -195,7 +195,7 @@
 			die("unable to parse \'%s\': no data", avcstatfile);
 
 		if (cumulative || (!cumulative && !i))
-			printf("%10u %10u %10u %10u %10u %10u\n",
+			printf("%10Lu %10Lu %10Lu %10Lu %10Lu %10Lu\n",
 			       tot.lookups, tot.hits, tot.misses,
 			       tot.allocations, tot.reclaims, tot.frees);
 		else {
@@ -205,7 +205,7 @@
 			rel.allocations = tot.allocations - last.allocations;
 			rel.reclaims = tot.reclaims - last.reclaims;
 			rel.frees = tot.frees - last.frees;
-			printf("%10u %10u %10u %10u %10u %10u\n",
+			printf("%10Lu %10Lu %10Lu %10Lu %10Lu %10Lu\n",
 			       rel.lookups, rel.hits, rel.misses,
 			       rel.allocations, rel.reclaims, rel.frees);
 		}

Re: Real simple cache that removes most of the lookups in mcstrans

[Russell Coker]

On Thursday 18 May 2006 01:15, Joshua Brindle <method@gentoo.org> wrote:
> > Yes, you mean translation change, I think.  Since I do not see where
> > policy reload would effect this.  We could add some kind of timer to
> > this, but refreshing the cache is a small problem, but this same
> > problem existed with shared libraries.
>
> Because on an MLS system the server will be deciding whether or not you
> have permission to see a particular translation and a policy reload
> would affect that. The same problem did exist with the shared libraries
> but we aren't using them anymore :) the server should be smart enough to
> handle this. If client side caching is really desired it needs to work
> like an avc where you can get flush notifications from the server.

Client side caching is not required to work the same way as server-side.

The idea behind SE Linux is that the kernel enforces access control in almost 
all situations, and that applications which enforce access control are the 
exception (this means crond, login programs, the X server for SE-X, and not 
much else).

If an application such as "ls" gets a copy of the translation data then there 
is no point in writing the library code such that the data can not be 
(easily) displayed a second time.  The information once available is presumed 
to have already been displayed to the user or stored in a way such that it 
can be obtained in another manner.

-- 
http://www.coker.com.au/selinux/   My NSA Security Enhanced Linux packages
http://www.coker.com.au/bonnie++/  Bonnie++ hard drive benchmark
http://www.coker.com.au/postal/    Postal SMTP/POP benchmark
http://www.coker.com.au/~russell/  My home page

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Joshua Brindle]

Russell Coker wrote:
> On Thursday 18 May 2006 01:15, Joshua Brindle <method@gentoo.org> wrote:
>   
>>> Yes, you mean translation change, I think.  Since I do not see where
>>> policy reload would effect this.  We could add some kind of timer to
>>> this, but refreshing the cache is a small problem, but this same
>>> problem existed with shared libraries.
>>>       
>> Because on an MLS system the server will be deciding whether or not you
>> have permission to see a particular translation and a policy reload
>> would affect that. The same problem did exist with the shared libraries
>> but we aren't using them anymore :) the server should be smart enough to
>> handle this. If client side caching is really desired it needs to work
>> like an avc where you can get flush notifications from the server.
>>     
>
> Client side caching is not required to work the same way as server-side.
>
> The idea behind SE Linux is that the kernel enforces access control in almost 
> all situations, and that applications which enforce access control are the 
> exception (this means crond, login programs, the X server for SE-X, and not 
> much else).
>
> If an application such as "ls" gets a copy of the translation data then there 
> is no point in writing the library code such that the data can not be 
> (easily) displayed a second time.  The information once available is presumed 
> to have already been displayed to the user or stored in a way such that it 
> can be obtained in another manner.
>   
If that were true we wouldn't revoke access to file descriptors (or 
files) after a policy reload. And you are still ignoring the case where 
the translation actually changes. My worry isn't ls (which is one-shot, 
and executes quickly most of the time), _anything_ that uses this 
library code now caches the answer and if its a long running process 
then it has no way of knowing when translations have changed or access 
has been revoked.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Stephen Smalley]

On Fri, 2006-05-19 at 09:51 -0400, Joshua Brindle wrote:
> Russell Coker wrote:
> > If an application such as "ls" gets a copy of the translation data then there 
> > is no point in writing the library code such that the data can not be 
> > (easily) displayed a second time.  The information once available is presumed 
> > to have already been displayed to the user or stored in a way such that it 
> > can be obtained in another manner.
> >   
> If that were true we wouldn't revoke access to file descriptors (or 
> files) after a policy reload.

Not the same thing.  Revoking data != revoking object capabilities.  The
data has already leaked.  Revoking the object capability prevents
reading of _new_ data from the object, and writing of data to the
object.  But the already read data has already migrated into the state
of the client.

>  And you are still ignoring the case where 
> the translation actually changes. My worry isn't ls (which is one-shot, 
> and executes quickly most of the time), _anything_ that uses this 
> library code now caches the answer and if its a long running process 
> then it has no way of knowing when translations have changed or access 
> has been revoked.

Notifications of changes to the translations make sense for correctness.
Not so much of an access control issue.

-- 
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 majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Russell Coker]

On Saturday 20 May 2006 00:07, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> >  And you are still ignoring the case where
> > the translation actually changes. My worry isn't ls (which is one-shot,
> > and executes quickly most of the time), _anything_ that uses this
> > library code now caches the answer and if its a long running process
> > then it has no way of knowing when translations have changed or access
> > has been revoked.
>
> Notifications of changes to the translations make sense for correctness.
> Not so much of an access control issue.

Agreed in concept (although I doubt that you can do anything that is really 
correct in the face of translation changes during runtime).  Maybe the timing 
solution is the one to go for?

I think that it would be expected that when a translation is changed some 
programs will not display the new data instantly (note that there may be a 
delay of arbitrary length from the time the application gets the translation 
to the time it displays it, sorting output and many other things may occur).

For ls I don't think that we even need to have a time check, it usually 
doesn't run for a long period of time and does not support more than one 
operation per execution.  If a translation is changed while ls is executing 
then returning the initial version of the result is OK (no operation other 
than restarting execution of the program will give entirely consistent 
results in the face of such a change).

For ps unless there is something wrong it won't run for long enough to have 
any issues.

Another option might be to enable the cache on request.  So ls and ps can 
request the cache while other programs might not do so.

-- 
http://www.coker.com.au/selinux/   My NSA Security Enhanced Linux packages
http://www.coker.com.au/bonnie++/  Bonnie++ hard drive benchmark
http://www.coker.com.au/postal/    Postal SMTP/POP benchmark
http://www.coker.com.au/~russell/  My home page

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Joshua Brindle]

Stephen Smalley wrote:
> On Fri, 2006-05-19 at 09:51 -0400, Joshua Brindle wrote:
>   
>> Russell Coker wrote:
>>     
>>> If an application such as "ls" gets a copy of the translation data then there 
>>> is no point in writing the library code such that the data can not be 
>>> (easily) displayed a second time.  The information once available is presumed 
>>> to have already been displayed to the user or stored in a way such that it 
>>> can be obtained in another manner.
>>>   
>>>       
>> If that were true we wouldn't revoke access to file descriptors (or 
>> files) after a policy reload.
>>     
>
> Not the same thing.  Revoking data != revoking object capabilities.  The
> data has already leaked.  Revoking the object capability prevents
> reading of _new_ data from the object, and writing of data to the
> object.  But the already read data has already migrated into the state
> of the client.
>   
That is a tricky distinction. AFAIK the servers that do access control 
for translations essentially treat the translation itself as the object 
(by using the untranslated context as the target object with a specific 
object class). I understand that you can't revoke access to the data in 
a file that a program read, only further access to the file. And I know 
that the client can cache the information outside of the library. My 
point is that if the access changes the client potentially won't know 
because the library is transparently caching the information and not 
consulting the server.
>   
>>  And you are still ignoring the case where 
>> the translation actually changes. My worry isn't ls (which is one-shot, 
>> and executes quickly most of the time), _anything_ that uses this 
>> library code now caches the answer and if its a long running process 
>> then it has no way of knowing when translations have changed or access 
>> has been revoked.
>>     
>
> Notifications of changes to the translations make sense for correctness.
> Not so much of an access control issue
Understood, however, if the client is providing a translation service 
for a user (or other application) it should get notification if that 
user/app should no longer be able to see the translation, right? The 
translations are treated like objects in this scheme, unless I'm 
misunderstanding something.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Russell Coker]

On Sunday 21 May 2006 07:47, Joshua Brindle <method@gentoo.org> wrote:
> a file that a program read, only further access to the file. And I know
> that the client can cache the information outside of the library. My
> point is that if the access changes the client potentially won't know
> because the library is transparently caching the information and not
> consulting the server.

That depends on the client.  I believe that the operation of ls will not 
become any less correct in the face of translation caching and changes to 
translations during the run-time of ls.  Having a listing of files in a 
directory (or tree) that is translated in different ways is not a good 
result, the only way for ls to have a consistent result in such situations is 
to have more caching such that a translation will never change for the 
duration of the process!

> Understood, however, if the client is providing a translation service
> for a user (or other application) it should get notification if that
> user/app should no longer be able to see the translation, right? The
> translations are treated like objects in this scheme, unless I'm
> misunderstanding something.

That might be suitable for some programs, but is not required for ls or ps.

What if we have a cache that defaults to on but which can be turned off on 
demand by programs that have special requirements in that regard?

-- 
http://www.coker.com.au/selinux/   My NSA Security Enhanced Linux packages
http://www.coker.com.au/bonnie++/  Bonnie++ hard drive benchmark
http://www.coker.com.au/postal/    Postal SMTP/POP benchmark
http://www.coker.com.au/~russell/  My home page

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Stephen Smalley]

On Sat, 2006-05-20 at 17:47 -0400, Joshua Brindle wrote:
> Understood, however, if the client is providing a translation service 
> for a user (or other application) it should get notification if that 
> user/app should no longer be able to see the translation, right? The 
> translations are treated like objects in this scheme, unless I'm 
> misunderstanding something.

If the client were providing a translation service for some third party
(as opposed to that third party directly interacting with the
translation daemon), then the client (now a server itself) would be an
object manager in its own right and would need to perform security
checks based on the third party (the client of the client, so to speak).
At which point it is using the AVC itself and has the associated support
for getting notifications of policy reloads.  In the more likely case,
the client is getting data for its own use, and the translation daemon
is checking access based on the client's credentials (which may encode
information about the user and application and even the entire call
chain).

-- 
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 majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Real simple cache that removes most of the lookups in mcstrans

[Stephen Smalley]

On Wed, 2006-05-17 at 14:22 -0400, Daniel J Walsh wrote:
> Updated patch with
> 
> void fini_context_translations
> 
> memory checking on strdup
> 
> CACHETRANS flag in /etc/selinux/config to turn off cacheing
> 
> Lu -> llu change
> 
> I will leave a more complete caching solution for later...  This handled 
> 99.9% of the cases.

Hmmm..I never seemed to receive a version of this patch that actually
checked the strdup calls and used %llu rather than %Lu, so I made those
changes by hand.  Merged with those modifications.

-- 
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 majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.