All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libselinux: disable refcounting in the userspace AVC
@ 2009-09-02  3:35 Eamon Walsh
  2009-09-02 12:00 ` Stephen Smalley
  0 siblings, 1 reply; 10+ messages in thread
From: Eamon Walsh @ 2009-09-02  3:35 UTC (permalink / raw)
  To: SELinux List; +Cc: Joshua Brindle

The userspace AVC currently has refcounted SID's.  This patch strips out 
the refcounting under the following justifications:

1.  Managing the refcounts by calling sidput() and sidget() as 
appropriate is a difficult and bug-prone task for users of the library.

2.  The userspace AVC doesn't currently make use of the refcounts to 
reclaim unused SID's unless avc_cleanup() is explicitly called.

3.  The kernel itself no longer uses refcounting for it's own SID's.

The implication of this change is that SID's (basically malloc'ed copies 
of security contexts) will persist in the AVC's SID table until the next 
call to avc_destroy().  This presents the potential for increased memory 
usage, but in practice I don't believe this will be an issue.  ABI 
compatibility is preserved: the avc_cleanup(), sidput(), and sidget() 
calls are changed to no-ops.

Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
---

  avc.c          |  122 +++++++--------------------------------------------------
  avc_internal.h |   11 -----
  avc_sidtab.c   |   42 -------------------
  avc_sidtab.h   |    2
  4 files changed, 16 insertions(+), 161 deletions(-)


diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
index f0e2d33..1c62fa3 100644
--- a/libselinux/src/avc.c
+++ b/libselinux/src/avc.c
@@ -71,8 +71,6 @@ int avc_context_to_sid_raw(security_context_t ctx, security_id_t * sid)
  	int rc;
  	avc_get_lock(avc_lock);
  	rc = sidtab_context_to_sid(&avc_sidtab, ctx, sid);
-	if (!rc)
-		(*sid)->refcnt++;
  	avc_release_lock(avc_lock);
  	return rc;
  }
@@ -97,13 +95,8 @@ int avc_sid_to_context_raw(security_id_t sid, security_context_t * ctx)
  	int rc;
  	*ctx = NULL;
  	avc_get_lock(avc_lock);
-	if (sid->refcnt>  0) {
-		*ctx = strdup(sid->ctx);	/* caller must free via freecon */
-		rc = *ctx ? 0 : -1;
-	} else {
-		errno = EINVAL;	/* bad reference count */
-		rc = -1;
-	}
+	*ctx = strdup(sid->ctx);	/* caller must free via freecon */
+	rc = *ctx ? 0 : -1;
  	avc_release_lock(avc_lock);
  	return rc;
  }
@@ -123,24 +116,14 @@ int avc_sid_to_context(security_id_t sid, security_context_t * ctx)
  	return ret;
  }

-int sidget(security_id_t sid)
+int sidget(security_id_t sid __attribute__((unused)))
  {
-	int rc;
-	avc_get_lock(avc_lock);
-	rc = sid_inc_refcnt(sid);
-	avc_release_lock(avc_lock);
-	return rc;
+	return 1;
  }

-int sidput(security_id_t sid)
+int sidput(security_id_t sid __attribute__((unused)))
  {
-	int rc;
-	if (!sid)
-	    return 0;
-	avc_get_lock(avc_lock);
-	rc = sid_dec_refcnt(sid);
-	avc_release_lock(avc_lock);
-	return rc;
+	return 1;
  }

  int avc_get_initial_sid(const char * name, security_id_t * sid)
@@ -505,57 +488,8 @@ static int avc_insert(security_id_t ssid, security_id_t tsid,
  	return rc;
  }

-/**
- * avc_remove - Remove AVC and sidtab entries for SID.
- * @sid: security identifier to be removed
- *
- * Remove all AVC entries containing @sid as source, target, or
- * create_sid, and remove @sid from the SID table.
- * Free the memory allocated for the structure corresponding
- * to @sid.  After this function has been called, @sid must
- * not be used until another call to avc_context_to_sid() has
- * been made for this SID.
- */
-static void avc_remove(security_id_t sid)
-{
-	struct avc_node *prev, *cur, *tmp;
-	int i;
-
-	for (i = 0; i<  AVC_CACHE_SLOTS; i++) {
-		cur = avc_cache.slots[i];
-		prev = NULL;
-		while (cur) {
-			if (sid == cur->ae.ssid || sid == cur->ae.tsid ||
-			    sid == cur->ae.create_sid) {
-				if (prev)
-					prev->next = cur->next;
-				else
-					avc_cache.slots[i] = cur->next;
-				tmp = cur;
-				cur = cur->next;
-				avc_clear_avc_entry(&tmp->ae);
-				tmp->next = avc_node_freelist;
-				avc_node_freelist = tmp;
-				avc_cache.active_nodes--;
-			} else {
-				prev = cur;
-				cur = cur->next;
-			}
-		}
-	}
-	sidtab_remove(&avc_sidtab, sid);
-}
-
  void avc_cleanup(void)
  {
-	security_id_t sid;
-
-	avc_get_lock(avc_lock);
-
-	while (NULL != (sid = sidtab_claim_sid(&avc_sidtab)))
-		avc_remove(sid);
-
-	avc_release_lock(avc_lock);
  }

  hidden_def(avc_cleanup)
@@ -745,15 +679,8 @@ static void avc_dump_query(security_id_t ssid, security_id_t tsid,
  {
  	avc_get_lock(avc_lock);

-	if (ssid->refcnt>  0)
-		log_append(avc_audit_buf, "scontext=%s", ssid->ctx);
-	else
-		log_append(avc_audit_buf, "ssid=%p", ssid);
-
-	if (tsid->refcnt>  0)
-		log_append(avc_audit_buf, " tcontext=%s", tsid->ctx);
-	else
-		log_append(avc_audit_buf, " tsid=%p", tsid);
+	log_append(avc_audit_buf, "scontext=%s tcontext=%s",
+		   ssid->ctx, tsid->ctx);

  	avc_release_lock(avc_lock);
  	log_append(avc_audit_buf, " tclass=%s",
@@ -844,11 +771,6 @@ int avc_has_perm_noaudit(security_id_t ssid,
  		avc_cache_stats_incr(entry_misses);
  		rc = avc_lookup(ssid, tsid, tclass, requested, aeref);
  		if (rc) {
-			if ((ssid->refcnt<= 0) || (tsid->refcnt<= 0)) {
-				errno = EINVAL;
-				rc = -1;
-				goto out;
-			}
  			rc = security_compute_av_flags_raw(ssid->ctx, tsid->ctx,
  							   tclass, requested,
  							&entry.avd);
@@ -911,11 +833,6 @@ int avc_compute_create(security_id_t ssid,  security_id_t tsid,
  	avc_entry_ref_init(&aeref);

  	avc_get_lock(avc_lock);
-	if (ssid->refcnt<= 0 || tsid->refcnt<= 0) {
-		errno = EINVAL;	/* bad reference count */
-		rc = -1;
-		goto out;
-	}

  	/* check for a cached entry */
  	rc = avc_lookup(ssid, tsid, tclass, 0,&aeref);
@@ -950,8 +867,6 @@ int avc_compute_create(security_id_t ssid,  security_id_t tsid,

  	rc = 0;
  out:
-	if (*newsid)
-		(*newsid)->refcnt++;
  	avc_release_lock(avc_lock);
  	return rc;
  }
@@ -960,22 +875,15 @@ int avc_compute_member(security_id_t ssid,  security_id_t tsid,
  		       security_class_t tclass, security_id_t *newsid)
  {
  	int rc;
+	security_context_t ctx = NULL;
  	*newsid = NULL;
  	avc_get_lock(avc_lock);
-	if (ssid->refcnt>  0&&  tsid->refcnt>  0) {
-		security_context_t ctx = NULL;
-		rc = security_compute_member_raw(ssid->ctx, tsid->ctx, tclass,
-						&ctx);
-		if (rc)
-			goto out;
-		rc = sidtab_context_to_sid(&avc_sidtab, ctx, newsid);
-		if (!rc)
-			(*newsid)->refcnt++;
-		freecon(ctx);
-	} else {
-		errno = EINVAL;	/* bad reference count */
-		rc = -1;
-	}
+
+	rc = security_compute_member_raw(ssid->ctx, tsid->ctx, tclass,&ctx);
+	if (rc)
+		goto out;
+	rc = sidtab_context_to_sid(&avc_sidtab, ctx, newsid);
+	freecon(ctx);
  out:
  	avc_release_lock(avc_lock);
  	return rc;
diff --git a/libselinux/src/avc_internal.h b/libselinux/src/avc_internal.h
index e9e5772..53610e8 100644
--- a/libselinux/src/avc_internal.h
+++ b/libselinux/src/avc_internal.h
@@ -16,17 +16,6 @@
  #include "callbacks.h"
  #include "dso.h"

-/* SID reference counter manipulation */
-static inline int sid_inc_refcnt(security_id_t sid)
-{
-	return sid->refcnt = (sid->refcnt>  0) ? sid->refcnt + 1 : 0;
-}
-
-static inline int sid_dec_refcnt(security_id_t sid)
-{
-	return sid->refcnt = (sid->refcnt>  0) ? sid->refcnt - 1 : 0;
-}
-
  /* callback pointers */
  extern void *(*avc_func_malloc) (size_t) hidden;
  extern void (*avc_func_free) (void *)hidden;
diff --git a/libselinux/src/avc_sidtab.c b/libselinux/src/avc_sidtab.c
index dab5c4e..3ca1d1f 100644
--- a/libselinux/src/avc_sidtab.c
+++ b/libselinux/src/avc_sidtab.c
@@ -67,53 +67,13 @@ int sidtab_insert(struct sidtab *s, security_context_t ctx)
  	hvalue = sidtab_hash(newctx);
  	newnode->next = s->htable[hvalue];
  	newnode->sid_s.ctx = newctx;
-	newnode->sid_s.refcnt = 0;	/* caller should increment */
+	newnode->sid_s.refcnt = 1;	/* unused */
  	s->htable[hvalue] = newnode;
  	s->nel++;
        out:
  	return rc;
  }

-void sidtab_remove(struct sidtab *s, security_id_t sid)
-{
-	int hvalue;
-	struct sidtab_node *cur, *prev;
-
-	hvalue = sidtab_hash(sid->ctx);
-	cur = s->htable[hvalue];
-	prev = NULL;
-	while (cur) {
-		if (sid ==&cur->sid_s) {
-			if (prev)
-				prev->next = cur->next;
-			else
-				s->htable[hvalue] = cur->next;
-			avc_free(cur);
-			s->nel--;
-			return;
-		} else {
-			prev = cur;
-			cur = cur->next;
-		}
-	}
-}
-
-security_id_t sidtab_claim_sid(struct sidtab *s)
-{
-	int i;
-	struct sidtab_node *cur;
-
-	for (i = 0; i<  SIDTAB_SIZE; i++) {
-		cur = s->htable[i];
-		while (cur) {
-			if (!cur->sid_s.refcnt)
-				return&cur->sid_s;
-			cur = cur->next;
-		}
-	}
-	return NULL;
-}
-
  int
  sidtab_context_to_sid(struct sidtab *s,
  		      security_context_t ctx, security_id_t * sid)
diff --git a/libselinux/src/avc_sidtab.h b/libselinux/src/avc_sidtab.h
index 620a335..29b5d8b 100644
--- a/libselinux/src/avc_sidtab.h
+++ b/libselinux/src/avc_sidtab.h
@@ -26,8 +26,6 @@ struct sidtab {

  int sidtab_init(struct sidtab *s) hidden;
  int sidtab_insert(struct sidtab *s, security_context_t ctx) hidden;
-void sidtab_remove(struct sidtab *s, security_id_t sid) hidden;
-security_id_t sidtab_claim_sid(struct sidtab *s) hidden;

  int sidtab_context_to_sid(struct sidtab *s,
  			  security_context_t ctx, security_id_t * sid) hidden;



-- 
Eamon Walsh<ewalsh@tycho.nsa.gov>
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.

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] libselinux: disable refcounting in the userspace AVC
  2009-09-02  3:35 [PATCH] libselinux: disable refcounting in the userspace AVC Eamon Walsh
@ 2009-09-02 12:00 ` Stephen Smalley
  2009-09-02 13:35   ` Joshua Brindle
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Smalley @ 2009-09-02 12:00 UTC (permalink / raw)
  To: Eamon Walsh; +Cc: SELinux List, Joshua Brindle

On Tue, 2009-09-01 at 23:35 -0400, Eamon Walsh wrote:
> The userspace AVC currently has refcounted SID's.  This patch strips out 
> the refcounting under the following justifications:
> 
> 1.  Managing the refcounts by calling sidput() and sidget() as 
> appropriate is a difficult and bug-prone task for users of the library.
> 
> 2.  The userspace AVC doesn't currently make use of the refcounts to 
> reclaim unused SID's unless avc_cleanup() is explicitly called.
> 
> 3.  The kernel itself no longer uses refcounting for it's own SID's.

Just to clarify on this point:  Early on we had discussed introducing
refcounting of kernel SIDs to enable reclaiming unused ones, but this
became less important once the user/kernel interface was converted from
passing SIDs to passing security contexts.  In that case we were dealing
with a global (wrt a system) resource and kernel memory, whereas in the
userspace AVC we are only dealing with a per-object-manager SID table
and the application's virtual memory.

> The implication of this change is that SID's (basically malloc'ed copies 
> of security contexts) will persist in the AVC's SID table until the next 
> call to avc_destroy().  This presents the potential for increased memory 
> usage, but in practice I don't believe this will be an issue.  ABI 
> compatibility is preserved: the avc_cleanup(), sidput(), and sidget() 
> calls are changed to no-ops.
> 
> Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

> ---
> 
>   avc.c          |  122 +++++++--------------------------------------------------
>   avc_internal.h |   11 -----
>   avc_sidtab.c   |   42 -------------------
>   avc_sidtab.h   |    2
>   4 files changed, 16 insertions(+), 161 deletions(-)
> 
> 
> diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
> index f0e2d33..1c62fa3 100644
> --- a/libselinux/src/avc.c
> +++ b/libselinux/src/avc.c
> @@ -71,8 +71,6 @@ int avc_context_to_sid_raw(security_context_t ctx, security_id_t * sid)
>   	int rc;
>   	avc_get_lock(avc_lock);
>   	rc = sidtab_context_to_sid(&avc_sidtab, ctx, sid);
> -	if (!rc)
> -		(*sid)->refcnt++;
>   	avc_release_lock(avc_lock);
>   	return rc;
>   }
> @@ -97,13 +95,8 @@ int avc_sid_to_context_raw(security_id_t sid, security_context_t * ctx)
>   	int rc;
>   	*ctx = NULL;
>   	avc_get_lock(avc_lock);
> -	if (sid->refcnt>  0) {
> -		*ctx = strdup(sid->ctx);	/* caller must free via freecon */
> -		rc = *ctx ? 0 : -1;
> -	} else {
> -		errno = EINVAL;	/* bad reference count */
> -		rc = -1;
> -	}
> +	*ctx = strdup(sid->ctx);	/* caller must free via freecon */
> +	rc = *ctx ? 0 : -1;
>   	avc_release_lock(avc_lock);
>   	return rc;
>   }
> @@ -123,24 +116,14 @@ int avc_sid_to_context(security_id_t sid, security_context_t * ctx)
>   	return ret;
>   }
> 
> -int sidget(security_id_t sid)
> +int sidget(security_id_t sid __attribute__((unused)))
>   {
> -	int rc;
> -	avc_get_lock(avc_lock);
> -	rc = sid_inc_refcnt(sid);
> -	avc_release_lock(avc_lock);
> -	return rc;
> +	return 1;
>   }
> 
> -int sidput(security_id_t sid)
> +int sidput(security_id_t sid __attribute__((unused)))
>   {
> -	int rc;
> -	if (!sid)
> -	    return 0;
> -	avc_get_lock(avc_lock);
> -	rc = sid_dec_refcnt(sid);
> -	avc_release_lock(avc_lock);
> -	return rc;
> +	return 1;
>   }
> 
>   int avc_get_initial_sid(const char * name, security_id_t * sid)
> @@ -505,57 +488,8 @@ static int avc_insert(security_id_t ssid, security_id_t tsid,
>   	return rc;
>   }
> 
> -/**
> - * avc_remove - Remove AVC and sidtab entries for SID.
> - * @sid: security identifier to be removed
> - *
> - * Remove all AVC entries containing @sid as source, target, or
> - * create_sid, and remove @sid from the SID table.
> - * Free the memory allocated for the structure corresponding
> - * to @sid.  After this function has been called, @sid must
> - * not be used until another call to avc_context_to_sid() has
> - * been made for this SID.
> - */
> -static void avc_remove(security_id_t sid)
> -{
> -	struct avc_node *prev, *cur, *tmp;
> -	int i;
> -
> -	for (i = 0; i<  AVC_CACHE_SLOTS; i++) {
> -		cur = avc_cache.slots[i];
> -		prev = NULL;
> -		while (cur) {
> -			if (sid == cur->ae.ssid || sid == cur->ae.tsid ||
> -			    sid == cur->ae.create_sid) {
> -				if (prev)
> -					prev->next = cur->next;
> -				else
> -					avc_cache.slots[i] = cur->next;
> -				tmp = cur;
> -				cur = cur->next;
> -				avc_clear_avc_entry(&tmp->ae);
> -				tmp->next = avc_node_freelist;
> -				avc_node_freelist = tmp;
> -				avc_cache.active_nodes--;
> -			} else {
> -				prev = cur;
> -				cur = cur->next;
> -			}
> -		}
> -	}
> -	sidtab_remove(&avc_sidtab, sid);
> -}
> -
>   void avc_cleanup(void)
>   {
> -	security_id_t sid;
> -
> -	avc_get_lock(avc_lock);
> -
> -	while (NULL != (sid = sidtab_claim_sid(&avc_sidtab)))
> -		avc_remove(sid);
> -
> -	avc_release_lock(avc_lock);
>   }
> 
>   hidden_def(avc_cleanup)
> @@ -745,15 +679,8 @@ static void avc_dump_query(security_id_t ssid, security_id_t tsid,
>   {
>   	avc_get_lock(avc_lock);
> 
> -	if (ssid->refcnt>  0)
> -		log_append(avc_audit_buf, "scontext=%s", ssid->ctx);
> -	else
> -		log_append(avc_audit_buf, "ssid=%p", ssid);
> -
> -	if (tsid->refcnt>  0)
> -		log_append(avc_audit_buf, " tcontext=%s", tsid->ctx);
> -	else
> -		log_append(avc_audit_buf, " tsid=%p", tsid);
> +	log_append(avc_audit_buf, "scontext=%s tcontext=%s",
> +		   ssid->ctx, tsid->ctx);
> 
>   	avc_release_lock(avc_lock);
>   	log_append(avc_audit_buf, " tclass=%s",
> @@ -844,11 +771,6 @@ int avc_has_perm_noaudit(security_id_t ssid,
>   		avc_cache_stats_incr(entry_misses);
>   		rc = avc_lookup(ssid, tsid, tclass, requested, aeref);
>   		if (rc) {
> -			if ((ssid->refcnt<= 0) || (tsid->refcnt<= 0)) {
> -				errno = EINVAL;
> -				rc = -1;
> -				goto out;
> -			}
>   			rc = security_compute_av_flags_raw(ssid->ctx, tsid->ctx,
>   							   tclass, requested,
>   							&entry.avd);
> @@ -911,11 +833,6 @@ int avc_compute_create(security_id_t ssid,  security_id_t tsid,
>   	avc_entry_ref_init(&aeref);
> 
>   	avc_get_lock(avc_lock);
> -	if (ssid->refcnt<= 0 || tsid->refcnt<= 0) {
> -		errno = EINVAL;	/* bad reference count */
> -		rc = -1;
> -		goto out;
> -	}
> 
>   	/* check for a cached entry */
>   	rc = avc_lookup(ssid, tsid, tclass, 0,&aeref);
> @@ -950,8 +867,6 @@ int avc_compute_create(security_id_t ssid,  security_id_t tsid,
> 
>   	rc = 0;
>   out:
> -	if (*newsid)
> -		(*newsid)->refcnt++;
>   	avc_release_lock(avc_lock);
>   	return rc;
>   }
> @@ -960,22 +875,15 @@ int avc_compute_member(security_id_t ssid,  security_id_t tsid,
>   		       security_class_t tclass, security_id_t *newsid)
>   {
>   	int rc;
> +	security_context_t ctx = NULL;
>   	*newsid = NULL;
>   	avc_get_lock(avc_lock);
> -	if (ssid->refcnt>  0&&  tsid->refcnt>  0) {
> -		security_context_t ctx = NULL;
> -		rc = security_compute_member_raw(ssid->ctx, tsid->ctx, tclass,
> -						&ctx);
> -		if (rc)
> -			goto out;
> -		rc = sidtab_context_to_sid(&avc_sidtab, ctx, newsid);
> -		if (!rc)
> -			(*newsid)->refcnt++;
> -		freecon(ctx);
> -	} else {
> -		errno = EINVAL;	/* bad reference count */
> -		rc = -1;
> -	}
> +
> +	rc = security_compute_member_raw(ssid->ctx, tsid->ctx, tclass,&ctx);
> +	if (rc)
> +		goto out;
> +	rc = sidtab_context_to_sid(&avc_sidtab, ctx, newsid);
> +	freecon(ctx);
>   out:
>   	avc_release_lock(avc_lock);
>   	return rc;
> diff --git a/libselinux/src/avc_internal.h b/libselinux/src/avc_internal.h
> index e9e5772..53610e8 100644
> --- a/libselinux/src/avc_internal.h
> +++ b/libselinux/src/avc_internal.h
> @@ -16,17 +16,6 @@
>   #include "callbacks.h"
>   #include "dso.h"
> 
> -/* SID reference counter manipulation */
> -static inline int sid_inc_refcnt(security_id_t sid)
> -{
> -	return sid->refcnt = (sid->refcnt>  0) ? sid->refcnt + 1 : 0;
> -}
> -
> -static inline int sid_dec_refcnt(security_id_t sid)
> -{
> -	return sid->refcnt = (sid->refcnt>  0) ? sid->refcnt - 1 : 0;
> -}
> -
>   /* callback pointers */
>   extern void *(*avc_func_malloc) (size_t) hidden;
>   extern void (*avc_func_free) (void *)hidden;
> diff --git a/libselinux/src/avc_sidtab.c b/libselinux/src/avc_sidtab.c
> index dab5c4e..3ca1d1f 100644
> --- a/libselinux/src/avc_sidtab.c
> +++ b/libselinux/src/avc_sidtab.c
> @@ -67,53 +67,13 @@ int sidtab_insert(struct sidtab *s, security_context_t ctx)
>   	hvalue = sidtab_hash(newctx);
>   	newnode->next = s->htable[hvalue];
>   	newnode->sid_s.ctx = newctx;
> -	newnode->sid_s.refcnt = 0;	/* caller should increment */
> +	newnode->sid_s.refcnt = 1;	/* unused */
>   	s->htable[hvalue] = newnode;
>   	s->nel++;
>         out:
>   	return rc;
>   }
> 
> -void sidtab_remove(struct sidtab *s, security_id_t sid)
> -{
> -	int hvalue;
> -	struct sidtab_node *cur, *prev;
> -
> -	hvalue = sidtab_hash(sid->ctx);
> -	cur = s->htable[hvalue];
> -	prev = NULL;
> -	while (cur) {
> -		if (sid ==&cur->sid_s) {
> -			if (prev)
> -				prev->next = cur->next;
> -			else
> -				s->htable[hvalue] = cur->next;
> -			avc_free(cur);
> -			s->nel--;
> -			return;
> -		} else {
> -			prev = cur;
> -			cur = cur->next;
> -		}
> -	}
> -}
> -
> -security_id_t sidtab_claim_sid(struct sidtab *s)
> -{
> -	int i;
> -	struct sidtab_node *cur;
> -
> -	for (i = 0; i<  SIDTAB_SIZE; i++) {
> -		cur = s->htable[i];
> -		while (cur) {
> -			if (!cur->sid_s.refcnt)
> -				return&cur->sid_s;
> -			cur = cur->next;
> -		}
> -	}
> -	return NULL;
> -}
> -
>   int
>   sidtab_context_to_sid(struct sidtab *s,
>   		      security_context_t ctx, security_id_t * sid)
> diff --git a/libselinux/src/avc_sidtab.h b/libselinux/src/avc_sidtab.h
> index 620a335..29b5d8b 100644
> --- a/libselinux/src/avc_sidtab.h
> +++ b/libselinux/src/avc_sidtab.h
> @@ -26,8 +26,6 @@ struct sidtab {
> 
>   int sidtab_init(struct sidtab *s) hidden;
>   int sidtab_insert(struct sidtab *s, security_context_t ctx) hidden;
> -void sidtab_remove(struct sidtab *s, security_id_t sid) hidden;
> -security_id_t sidtab_claim_sid(struct sidtab *s) hidden;
> 
>   int sidtab_context_to_sid(struct sidtab *s,
>   			  security_context_t ctx, security_id_t * sid) hidden;
> 
> 
> 
-- 
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH] libselinux: disable refcounting in the userspace AVC
  2009-09-02 12:00 ` Stephen Smalley
@ 2009-09-02 13:35   ` Joshua Brindle
  2009-09-02 13:54     ` Stephen Smalley
  0 siblings, 1 reply; 10+ messages in thread
From: Joshua Brindle @ 2009-09-02 13:35 UTC (permalink / raw)
  To: Stephen Smalley, Eamon Walsh; +Cc: SELinux List

> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> 
> On Tue, 2009-09-01 at 23:35 -0400, Eamon Walsh wrote:
> > The userspace AVC currently has refcounted SID's.  This patch strips
> out
> > the refcounting under the following justifications:
> >
> > 1.  Managing the refcounts by calling sidput() and sidget() as
> > appropriate is a difficult and bug-prone task for users of the
> library.
> >
> > 2.  The userspace AVC doesn't currently make use of the refcounts to
> > reclaim unused SID's unless avc_cleanup() is explicitly called.
> >
> > 3.  The kernel itself no longer uses refcounting for it's own SID's.
> 
> Just to clarify on this point:  Early on we had discussed introducing
> refcounting of kernel SIDs to enable reclaiming unused ones, but this
> became less important once the user/kernel interface was converted
from
> passing SIDs to passing security contexts.  In that case we were
> dealing
> with a global (wrt a system) resource and kernel memory, whereas in
the
> userspace AVC we are only dealing with a per-object-manager SID table
> and the application's virtual memory.
> 

So is the purpose of the patch just to remove unnecessary complication
in the userspace sidtab? Will this affect multithreaded applications
that share a sidtab? 

IIRC sepostgres reimplemented its sidtab and avc because our current one
wasn't very friendly to multithread/multiprocess object managers.

> > The implication of this change is that SID's (basically malloc'ed
> copies
> > of security contexts) will persist in the AVC's SID table until the
> next
> > call to avc_destroy().  This presents the potential for increased
> memory
> > usage, but in practice I don't believe this will be an issue.  ABI
> > compatibility is preserved: the avc_cleanup(), sidput(), and
sidget()
> > calls are changed to no-ops.
> >
> > Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
> 
> Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>
> 
> > ---
> >
> >   avc.c          |  122
+++++++--------------------------------------
> ------------
> >   avc_internal.h |   11 -----
> >   avc_sidtab.c   |   42 -------------------
> >   avc_sidtab.h   |    2
> >   4 files changed, 16 insertions(+), 161 deletions(-)
> >
> >
> > diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
> > index f0e2d33..1c62fa3 100644
> > --- a/libselinux/src/avc.c
> > +++ b/libselinux/src/avc.c
> > @@ -71,8 +71,6 @@ int avc_context_to_sid_raw(security_context_t ctx,
> security_id_t * sid)
> >   	int rc;
> >   	avc_get_lock(avc_lock);
> >   	rc = sidtab_context_to_sid(&avc_sidtab, ctx, sid);
> > -	if (!rc)
> > -		(*sid)->refcnt++;
> >   	avc_release_lock(avc_lock);
> >   	return rc;
> >   }
> > @@ -97,13 +95,8 @@ int avc_sid_to_context_raw(security_id_t sid,
> security_context_t * ctx)
> >   	int rc;
> >   	*ctx = NULL;
> >   	avc_get_lock(avc_lock);
> > -	if (sid->refcnt>  0) {
> > -		*ctx = strdup(sid->ctx);	/* caller must free via
> freecon */
> > -		rc = *ctx ? 0 : -1;
> > -	} else {
> > -		errno = EINVAL;	/* bad reference count */
> > -		rc = -1;
> > -	}
> > +	*ctx = strdup(sid->ctx);	/* caller must free via freecon
*/
> > +	rc = *ctx ? 0 : -1;
> >   	avc_release_lock(avc_lock);
> >   	return rc;
> >   }
> > @@ -123,24 +116,14 @@ int avc_sid_to_context(security_id_t sid,
> security_context_t * ctx)
> >   	return ret;
> >   }
> >
> > -int sidget(security_id_t sid)
> > +int sidget(security_id_t sid __attribute__((unused)))
> >   {
> > -	int rc;
> > -	avc_get_lock(avc_lock);
> > -	rc = sid_inc_refcnt(sid);
> > -	avc_release_lock(avc_lock);
> > -	return rc;
> > +	return 1;
> >   }
> >
> > -int sidput(security_id_t sid)
> > +int sidput(security_id_t sid __attribute__((unused)))
> >   {
> > -	int rc;
> > -	if (!sid)
> > -	    return 0;
> > -	avc_get_lock(avc_lock);
> > -	rc = sid_dec_refcnt(sid);
> > -	avc_release_lock(avc_lock);
> > -	return rc;
> > +	return 1;
> >   }
> >
> >   int avc_get_initial_sid(const char * name, security_id_t * sid)
> > @@ -505,57 +488,8 @@ static int avc_insert(security_id_t ssid,
> security_id_t tsid,
> >   	return rc;
> >   }
> >
> > -/**
> > - * avc_remove - Remove AVC and sidtab entries for SID.
> > - * @sid: security identifier to be removed
> > - *
> > - * Remove all AVC entries containing @sid as source, target, or
> > - * create_sid, and remove @sid from the SID table.
> > - * Free the memory allocated for the structure corresponding
> > - * to @sid.  After this function has been called, @sid must
> > - * not be used until another call to avc_context_to_sid() has
> > - * been made for this SID.
> > - */
> > -static void avc_remove(security_id_t sid)
> > -{
> > -	struct avc_node *prev, *cur, *tmp;
> > -	int i;
> > -
> > -	for (i = 0; i<  AVC_CACHE_SLOTS; i++) {
> > -		cur = avc_cache.slots[i];
> > -		prev = NULL;
> > -		while (cur) {
> > -			if (sid == cur->ae.ssid || sid == cur->ae.tsid
||
> > -			    sid == cur->ae.create_sid) {
> > -				if (prev)
> > -					prev->next = cur->next;
> > -				else
> > -					avc_cache.slots[i] = cur->next;
> > -				tmp = cur;
> > -				cur = cur->next;
> > -				avc_clear_avc_entry(&tmp->ae);
> > -				tmp->next = avc_node_freelist;
> > -				avc_node_freelist = tmp;
> > -				avc_cache.active_nodes--;
> > -			} else {
> > -				prev = cur;
> > -				cur = cur->next;
> > -			}
> > -		}
> > -	}
> > -	sidtab_remove(&avc_sidtab, sid);
> > -}
> > -
> >   void avc_cleanup(void)
> >   {
> > -	security_id_t sid;
> > -
> > -	avc_get_lock(avc_lock);
> > -
> > -	while (NULL != (sid = sidtab_claim_sid(&avc_sidtab)))
> > -		avc_remove(sid);
> > -
> > -	avc_release_lock(avc_lock);
> >   }
> >
> >   hidden_def(avc_cleanup)
> > @@ -745,15 +679,8 @@ static void avc_dump_query(security_id_t ssid,
> security_id_t tsid,
> >   {
> >   	avc_get_lock(avc_lock);
> >
> > -	if (ssid->refcnt>  0)
> > -		log_append(avc_audit_buf, "scontext=%s", ssid->ctx);
> > -	else
> > -		log_append(avc_audit_buf, "ssid=%p", ssid);
> > -
> > -	if (tsid->refcnt>  0)
> > -		log_append(avc_audit_buf, " tcontext=%s", tsid->ctx);
> > -	else
> > -		log_append(avc_audit_buf, " tsid=%p", tsid);
> > +	log_append(avc_audit_buf, "scontext=%s tcontext=%s",
> > +		   ssid->ctx, tsid->ctx);
> >
> >   	avc_release_lock(avc_lock);
> >   	log_append(avc_audit_buf, " tclass=%s",
> > @@ -844,11 +771,6 @@ int avc_has_perm_noaudit(security_id_t ssid,
> >   		avc_cache_stats_incr(entry_misses);
> >   		rc = avc_lookup(ssid, tsid, tclass, requested, aeref);
> >   		if (rc) {
> > -			if ((ssid->refcnt<= 0) || (tsid->refcnt<= 0)) {
> > -				errno = EINVAL;
> > -				rc = -1;
> > -				goto out;
> > -			}
> >   			rc = security_compute_av_flags_raw(ssid->ctx,
tsid-
> >ctx,
> >   							   tclass,
requested,
> >   							&entry.avd);
> > @@ -911,11 +833,6 @@ int avc_compute_create(security_id_t ssid,
> security_id_t tsid,
> >   	avc_entry_ref_init(&aeref);
> >
> >   	avc_get_lock(avc_lock);
> > -	if (ssid->refcnt<= 0 || tsid->refcnt<= 0) {
> > -		errno = EINVAL;	/* bad reference count */
> > -		rc = -1;
> > -		goto out;
> > -	}
> >
> >   	/* check for a cached entry */
> >   	rc = avc_lookup(ssid, tsid, tclass, 0,&aeref);
> > @@ -950,8 +867,6 @@ int avc_compute_create(security_id_t ssid,
> security_id_t tsid,
> >
> >   	rc = 0;
> >   out:
> > -	if (*newsid)
> > -		(*newsid)->refcnt++;
> >   	avc_release_lock(avc_lock);
> >   	return rc;
> >   }
> > @@ -960,22 +875,15 @@ int avc_compute_member(security_id_t ssid,
> security_id_t tsid,
> >   		       security_class_t tclass, security_id_t *newsid)
> >   {
> >   	int rc;
> > +	security_context_t ctx = NULL;
> >   	*newsid = NULL;
> >   	avc_get_lock(avc_lock);
> > -	if (ssid->refcnt>  0&&  tsid->refcnt>  0) {
> > -		security_context_t ctx = NULL;
> > -		rc = security_compute_member_raw(ssid->ctx, tsid->ctx,
> tclass,
> > -						&ctx);
> > -		if (rc)
> > -			goto out;
> > -		rc = sidtab_context_to_sid(&avc_sidtab, ctx, newsid);
> > -		if (!rc)
> > -			(*newsid)->refcnt++;
> > -		freecon(ctx);
> > -	} else {
> > -		errno = EINVAL;	/* bad reference count */
> > -		rc = -1;
> > -	}
> > +
> > +	rc = security_compute_member_raw(ssid->ctx, tsid->ctx,
> tclass,&ctx);
> > +	if (rc)
> > +		goto out;
> > +	rc = sidtab_context_to_sid(&avc_sidtab, ctx, newsid);
> > +	freecon(ctx);
> >   out:
> >   	avc_release_lock(avc_lock);
> >   	return rc;
> > diff --git a/libselinux/src/avc_internal.h
> b/libselinux/src/avc_internal.h
> > index e9e5772..53610e8 100644
> > --- a/libselinux/src/avc_internal.h
> > +++ b/libselinux/src/avc_internal.h
> > @@ -16,17 +16,6 @@
> >   #include "callbacks.h"
> >   #include "dso.h"
> >
> > -/* SID reference counter manipulation */
> > -static inline int sid_inc_refcnt(security_id_t sid)
> > -{
> > -	return sid->refcnt = (sid->refcnt>  0) ? sid->refcnt + 1 : 0;
> > -}
> > -
> > -static inline int sid_dec_refcnt(security_id_t sid)
> > -{
> > -	return sid->refcnt = (sid->refcnt>  0) ? sid->refcnt - 1 : 0;
> > -}
> > -
> >   /* callback pointers */
> >   extern void *(*avc_func_malloc) (size_t) hidden;
> >   extern void (*avc_func_free) (void *)hidden;
> > diff --git a/libselinux/src/avc_sidtab.c
> b/libselinux/src/avc_sidtab.c
> > index dab5c4e..3ca1d1f 100644
> > --- a/libselinux/src/avc_sidtab.c
> > +++ b/libselinux/src/avc_sidtab.c
> > @@ -67,53 +67,13 @@ int sidtab_insert(struct sidtab *s,
> security_context_t ctx)
> >   	hvalue = sidtab_hash(newctx);
> >   	newnode->next = s->htable[hvalue];
> >   	newnode->sid_s.ctx = newctx;
> > -	newnode->sid_s.refcnt = 0;	/* caller should increment */
> > +	newnode->sid_s.refcnt = 1;	/* unused */
> >   	s->htable[hvalue] = newnode;
> >   	s->nel++;
> >         out:
> >   	return rc;
> >   }
> >
> > -void sidtab_remove(struct sidtab *s, security_id_t sid)
> > -{
> > -	int hvalue;
> > -	struct sidtab_node *cur, *prev;
> > -
> > -	hvalue = sidtab_hash(sid->ctx);
> > -	cur = s->htable[hvalue];
> > -	prev = NULL;
> > -	while (cur) {
> > -		if (sid ==&cur->sid_s) {
> > -			if (prev)
> > -				prev->next = cur->next;
> > -			else
> > -				s->htable[hvalue] = cur->next;
> > -			avc_free(cur);
> > -			s->nel--;
> > -			return;
> > -		} else {
> > -			prev = cur;
> > -			cur = cur->next;
> > -		}
> > -	}
> > -}
> > -
> > -security_id_t sidtab_claim_sid(struct sidtab *s)
> > -{
> > -	int i;
> > -	struct sidtab_node *cur;
> > -
> > -	for (i = 0; i<  SIDTAB_SIZE; i++) {
> > -		cur = s->htable[i];
> > -		while (cur) {
> > -			if (!cur->sid_s.refcnt)
> > -				return&cur->sid_s;
> > -			cur = cur->next;
> > -		}
> > -	}
> > -	return NULL;
> > -}
> > -
> >   int
> >   sidtab_context_to_sid(struct sidtab *s,
> >   		      security_context_t ctx, security_id_t * sid)
> > diff --git a/libselinux/src/avc_sidtab.h
> b/libselinux/src/avc_sidtab.h
> > index 620a335..29b5d8b 100644
> > --- a/libselinux/src/avc_sidtab.h
> > +++ b/libselinux/src/avc_sidtab.h
> > @@ -26,8 +26,6 @@ struct sidtab {
> >
> >   int sidtab_init(struct sidtab *s) hidden;
> >   int sidtab_insert(struct sidtab *s, security_context_t ctx)
hidden;
> > -void sidtab_remove(struct sidtab *s, security_id_t sid) hidden;
> > -security_id_t sidtab_claim_sid(struct sidtab *s) hidden;
> >
> >   int sidtab_context_to_sid(struct sidtab *s,
> >   			  security_context_t ctx, security_id_t * sid)
> hidden;
> >
> >
> >
> --
> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH] libselinux: disable refcounting in the userspace AVC
  2009-09-02 13:35   ` Joshua Brindle
@ 2009-09-02 13:54     ` Stephen Smalley
  2009-09-02 16:35       ` Joshua Brindle
  2009-09-03  2:07       ` KaiGai Kohei
  0 siblings, 2 replies; 10+ messages in thread
From: Stephen Smalley @ 2009-09-02 13:54 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: Eamon Walsh, SELinux List, KaiGai Kohei

On Wed, 2009-09-02 at 09:35 -0400, Joshua Brindle wrote:
> > From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> > 
> > On Tue, 2009-09-01 at 23:35 -0400, Eamon Walsh wrote:
> > > The userspace AVC currently has refcounted SID's.  This patch strips
> > out
> > > the refcounting under the following justifications:
> > >
> > > 1.  Managing the refcounts by calling sidput() and sidget() as
> > > appropriate is a difficult and bug-prone task for users of the
> > library.
> > >
> > > 2.  The userspace AVC doesn't currently make use of the refcounts to
> > > reclaim unused SID's unless avc_cleanup() is explicitly called.
> > >
> > > 3.  The kernel itself no longer uses refcounting for it's own SID's.
> > 
> > Just to clarify on this point:  Early on we had discussed introducing
> > refcounting of kernel SIDs to enable reclaiming unused ones, but this
> > became less important once the user/kernel interface was converted
> from
> > passing SIDs to passing security contexts.  In that case we were
> > dealing
> > with a global (wrt a system) resource and kernel memory, whereas in
> the
> > userspace AVC we are only dealing with a per-object-manager SID table
> > and the application's virtual memory.
> > 
> 
> So is the purpose of the patch just to remove unnecessary complication
> in the userspace sidtab? Will this affect multithreaded applications
> that share a sidtab? 

Unnecessary complication for the userspace object managers - they no
longer have to keep SID refcounts accurate via sidget() and sidput()
calls when storing SIDs in their data structures.

No affect on multithreaded apps; they will still work (locking is
unaffected).

> IIRC sepostgres reimplemented its sidtab and avc because our current one
> wasn't very friendly to multithread/multiprocess object managers.

Looking back, I think KaiGai's reasons were:
- he didn't see benefit to the indirection of the AVC SID table since
PostgreSQL has to directly manage security contexts for persistent
labeling,
- he wanted to enable sharing of the AVC (via shared memory) and the
netlink thread among all PostgreSQL instances (processes, not just
threads).

So I don't think this makes any difference there.

-- 
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH] libselinux: disable refcounting in the userspace AVC
  2009-09-02 13:54     ` Stephen Smalley
@ 2009-09-02 16:35       ` Joshua Brindle
  2009-09-02 19:37         ` SELinux Packages and Library interactions Hasan Rezaul-CHR010
  2009-09-03  1:19         ` [PATCH] libselinux: disable refcounting in the userspace AVC Eamon Walsh
  2009-09-03  2:07       ` KaiGai Kohei
  1 sibling, 2 replies; 10+ messages in thread
From: Joshua Brindle @ 2009-09-02 16:35 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Eamon Walsh, SELinux List, KaiGai Kohei

> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> 
> On Wed, 2009-09-02 at 09:35 -0400, Joshua Brindle wrote:
> > > From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> > >
> > > On Tue, 2009-09-01 at 23:35 -0400, Eamon Walsh wrote:
> > > > The userspace AVC currently has refcounted SID's.  This patch
> strips
> > > out
> > > > the refcounting under the following justifications:
> > > >
> > > > 1.  Managing the refcounts by calling sidput() and sidget() as
> > > > appropriate is a difficult and bug-prone task for users of the
> > > library.
> > > >
> > > > 2.  The userspace AVC doesn't currently make use of the
refcounts
> to
> > > > reclaim unused SID's unless avc_cleanup() is explicitly called.
> > > >
> > > > 3.  The kernel itself no longer uses refcounting for it's own
> SID's.
> > >
> > > Just to clarify on this point:  Early on we had discussed
> introducing
> > > refcounting of kernel SIDs to enable reclaiming unused ones, but
> this
> > > became less important once the user/kernel interface was converted
> > from
> > > passing SIDs to passing security contexts.  In that case we were
> > > dealing
> > > with a global (wrt a system) resource and kernel memory, whereas
in
> > the
> > > userspace AVC we are only dealing with a per-object-manager SID
> table
> > > and the application's virtual memory.
> > >
> >
> > So is the purpose of the patch just to remove unnecessary
> complication
> > in the userspace sidtab? Will this affect multithreaded applications
> > that share a sidtab?
> 
> Unnecessary complication for the userspace object managers - they no
> longer have to keep SID refcounts accurate via sidget() and sidput()
> calls when storing SIDs in their data structures.
> 
> No affect on multithreaded apps; they will still work (locking is
> unaffected).
> 
> > IIRC sepostgres reimplemented its sidtab and avc because our current
> one
> > wasn't very friendly to multithread/multiprocess object managers.
> 
> Looking back, I think KaiGai's reasons were:
> - he didn't see benefit to the indirection of the AVC SID table since
> PostgreSQL has to directly manage security contexts for persistent
> labeling,
> - he wanted to enable sharing of the AVC (via shared memory) and the
> netlink thread among all PostgreSQL instances (processes, not just
> threads).
> 
> So I don't think this makes any difference there.
> 

Ok, seems fair.


--
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* SELinux Packages and Library interactions...
  2009-09-02 16:35       ` Joshua Brindle
@ 2009-09-02 19:37         ` Hasan Rezaul-CHR010
  2009-09-03 12:52           ` Stephen Smalley
  2009-09-03  1:19         ` [PATCH] libselinux: disable refcounting in the userspace AVC Eamon Walsh
  1 sibling, 1 reply; 10+ messages in thread
From: Hasan Rezaul-CHR010 @ 2009-09-02 19:37 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: SELinux List

Hi All,

Can someone point me to a nice diagram that describes the detailed
interaction of all the SELinux packages and libraries (policycoreutils,
checkpolicy, libselinux, libsepol, libsemanage, etc).

I am just trying to better understands what piece does what and how the
pieces interact with each other. 

Thanks in advance.


--
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libselinux: disable refcounting in the userspace AVC
  2009-09-02 16:35       ` Joshua Brindle
  2009-09-02 19:37         ` SELinux Packages and Library interactions Hasan Rezaul-CHR010
@ 2009-09-03  1:19         ` Eamon Walsh
  1 sibling, 0 replies; 10+ messages in thread
From: Eamon Walsh @ 2009-09-03  1:19 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: Stephen Smalley, SELinux List

On 09/02/2009 12:35 PM, Joshua Brindle wrote:
>
> Ok, seems fair.
>    

Pushed to libselinux 2.0.86, with manpage updates.  As Steve noted, this 
doesn't affect multithread or locking.   It just reduces complexity 
(especially on the caller's side) at the cost of memory usage from SID's 
potentially building up in the sidtab.

-- 
Eamon Walsh<ewalsh@tycho.nsa.gov>
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libselinux: disable refcounting in the userspace AVC
  2009-09-02 13:54     ` Stephen Smalley
  2009-09-02 16:35       ` Joshua Brindle
@ 2009-09-03  2:07       ` KaiGai Kohei
       [not found]         ` <3660A45FD9BC4D52AA7737D1CF2F1F5E@CIMSLAB.NL>
  1 sibling, 1 reply; 10+ messages in thread
From: KaiGai Kohei @ 2009-09-03  2:07 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Joshua Brindle, Eamon Walsh, SELinux List, KaiGai Kohei

Stephen Smalley wrote:
>> IIRC sepostgres reimplemented its sidtab and avc because our current one
>> wasn't very friendly to multithread/multiprocess object managers.
> 
> Looking back, I think KaiGai's reasons were:
> - he didn't see benefit to the indirection of the AVC SID table since
> PostgreSQL has to directly manage security contexts for persistent
> labeling,

Yes, it uses object identifier which is associated with a certain
text representation of security context as a key to lookup its
userspace avc.

> - he wanted to enable sharing of the AVC (via shared memory) and the
> netlink thread among all PostgreSQL instances (processes, not just
> threads).

The pgsql-hackers didn't like an idea to deploy userspace avc on the
shared memory segment, so I had to change its implementation.
Now, it put only a flag to notice avc invalidation for all the server
processes. A background netlink process updates the flag on the shared
memory segment, then other server processed voluntarily invalidates its
userspace avc deployed on the process local memory prior to looking up.

In addition, characteristic of database workload enables to apply more
optimization in the performance perspective.
The subject security context of the userspace avc deployed on the process
local memory is unchanged (except for trusted procedure execution),
so SE-PostgreSQL categorizes all the avc entries by subject context to
omit comparison of subject context.
(Note that subject context is not database object, so it does not have
object identifer. It requires strcmp() to compare subject context.)

Because the database workload (with row level security) needs to lookup
the avc entry massive times in a short time, this kind of optimization
is very important. At the past, I tried to compare subject context to
lookup avc entry, then it recorded 50% of transaction-per-second toward
the vanilla PostgreSQL. :(

Thus, now, it is not necessary to consider use cases in SE-PostgreSQL
to update userspace avc stuff in libselinux.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libselinux: disable refcounting in the userspace AVC
       [not found]         ` <3660A45FD9BC4D52AA7737D1CF2F1F5E@CIMSLAB.NL>
@ 2009-09-03  7:02           ` KaiGai Kohei
  0 siblings, 0 replies; 10+ messages in thread
From: KaiGai Kohei @ 2009-09-03  7:02 UTC (permalink / raw)
  To: Remmolt G. Zwartsenberg; +Cc: selinux, henk.busscher

Remmolt G. Zwartsenberg wrote:
> Hi KaiGai,
> 
>>From a mediation and billing perspective, i agree with you.
> The Network Management part of the OSS system uses URL's that resolve to
> high-latency authentication, that need no encryption in the columns.
> 
> In VOIP tunnels this can lead to congestion, ie in 2-way Irridium
> conversations. Erroneous frame dragging?

Sorry, I'm not sure what do you want to discuss.

At the previous message, I introduced a workload inside of the PostgreSQL
which tend to lookup userspace avc with same subject context many times.
It is not a workload between the client application and RDBMS.

Thanks,

> Kind regards,
> 
> ~remmolt 
> 
> Philips NEC Business Commumication Systems NL
> 
> remmolt@zwartsenberg.eu 
> 
> -----Original Message-----
> From: owner-selinux@tycho.nsa.gov [mailto:owner-selinux@tycho.nsa.gov] On
> Behalf Of KaiGai Kohei
> Sent: donderdag 3 september 2009 4:07
> To: Stephen Smalley
> Cc: Joshua Brindle; Eamon Walsh; SELinux List; KaiGai Kohei
> Subject: Re: [PATCH] libselinux: disable refcounting in the userspace AVC
> 
> Stephen Smalley wrote:
>>> IIRC sepostgres reimplemented its sidtab and avc because our current one
>>> wasn't very friendly to multithread/multiprocess object managers.
>> Looking back, I think KaiGai's reasons were:
>> - he didn't see benefit to the indirection of the AVC SID table since
>> PostgreSQL has to directly manage security contexts for persistent
>> labeling,
> 
> Yes, it uses object identifier which is associated with a certain
> text representation of security context as a key to lookup its
> userspace avc.
> 
>> - he wanted to enable sharing of the AVC (via shared memory) and the
>> netlink thread among all PostgreSQL instances (processes, not just
>> threads).
> 
> The pgsql-hackers didn't like an idea to deploy userspace avc on the
> shared memory segment, so I had to change its implementation.
> Now, it put only a flag to notice avc invalidation for all the server
> processes. A background netlink process updates the flag on the shared
> memory segment, then other server processed voluntarily invalidates its
> userspace avc deployed on the process local memory prior to looking up.
> 
> In addition, characteristic of database workload enables to apply more
> optimization in the performance perspective.
> The subject security context of the userspace avc deployed on the process
> local memory is unchanged (except for trusted procedure execution),
> so SE-PostgreSQL categorizes all the avc entries by subject context to
> omit comparison of subject context.
> (Note that subject context is not database object, so it does not have
> object identifer. It requires strcmp() to compare subject context.)
> 
> Because the database workload (with row level security) needs to lookup
> the avc entry massive times in a short time, this kind of optimization
> is very important. At the past, I tried to compare subject context to
> lookup avc entry, then it recorded 50% of transaction-per-second toward
> the vanilla PostgreSQL. :(
> 
> Thus, now, it is not necessary to consider use cases in SE-PostgreSQL
> to update userspace avc stuff in libselinux.
> 
> Thanks,


-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: SELinux Packages and Library interactions...
  2009-09-02 19:37         ` SELinux Packages and Library interactions Hasan Rezaul-CHR010
@ 2009-09-03 12:52           ` Stephen Smalley
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Smalley @ 2009-09-03 12:52 UTC (permalink / raw)
  To: Hasan Rezaul-CHR010; +Cc: SELinux List, Joshua Brindle, Chad Sellers

On Wed, 2009-09-02 at 15:37 -0400, Hasan Rezaul-CHR010 wrote:
> Hi All,
> 
> Can someone point me to a nice diagram that describes the detailed
> interaction of all the SELinux packages and libraries (policycoreutils,
> checkpolicy, libselinux, libsepol, libsemanage, etc).
> 
> I am just trying to better understands what piece does what and how the
> pieces interact with each other. 
> 
> Thanks in advance.

Not sure about a diagram, but the basic descriptions and relationships
can be summarized as follows:

1. libsepol is the binary policy manipulation library.  It doesn't
depend upon or use any of the other components.

2. checkpolicy is the policy compiler.  It uses libsepol to generate the
binary policy.  checkpolicy uses the static libsepol since it deals with
low level details of the policy that have not been
encapsulated/abstracted by a proper shared library interface.

3. libselinux is the runtime SELinux library that provides interfaces
(e.g. library functions for the SELinux kernel APIs like getcon(), other
support functions like getseuserbyname()) to SELinux-aware applications.
libselinux may use the shared libsepol to manipulate the binary policy
if necessary (e.g. to downgrade the policy format to an older version
supported by the kernel) when loading policy.

4. libsemanage is the policy management library.  It uses libsepol for
binary policy manipulation and libselinux for interacting with the
SELinux system.  It also exec's helper programs for loading policy and
for checking whether the file_contexts configuration is valid
(load_policy and setfiles from policycoreutils) presently, although this
may change at least for the bootstrapping case (for rpm).

5. sepolgen is a python module/library that forms the core of the modern
audit2allow (a rewrite).

6. policycoreutils is a collection of policy utilities (originally the
"core" set of utilities needed to use SELinux, although it has grown a
bit over time), which have different dependencies. sestatus, secon,
run_init, and newrole only use libselinux.  load_policy and setfiles
only use libselinux and libsepol.   semodule and semanage use
libsemanage (and thus bring in dependencies on libsepol and libselinux
as well).  setsebool uses libselinux to make non-persistent boolean
changes (via the kernel interface) and uses libsemanage to make
persistent boolean changes.  

-- 
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-09-03 12:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-02  3:35 [PATCH] libselinux: disable refcounting in the userspace AVC Eamon Walsh
2009-09-02 12:00 ` Stephen Smalley
2009-09-02 13:35   ` Joshua Brindle
2009-09-02 13:54     ` Stephen Smalley
2009-09-02 16:35       ` Joshua Brindle
2009-09-02 19:37         ` SELinux Packages and Library interactions Hasan Rezaul-CHR010
2009-09-03 12:52           ` Stephen Smalley
2009-09-03  1:19         ` [PATCH] libselinux: disable refcounting in the userspace AVC Eamon Walsh
2009-09-03  2:07       ` KaiGai Kohei
     [not found]         ` <3660A45FD9BC4D52AA7737D1CF2F1F5E@CIMSLAB.NL>
2009-09-03  7:02           ` KaiGai Kohei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.