[PATCH] SELinux: fix sidtab locking bug

[James Morris]

This patch by Kaigai Kohei fixes a bug in the SELinux sidtab code, where 
we do a spin_unlock_irq() while nested under another irq lock, which 
enables interrupts and allows a deadlock to happen:

  sidtab_set() is called between POLICY_WRLOCK and POLICY_WRUNLOCK in 
  services.c:1092. sidtab_set() uses SIDTAB_LOCK()/SIDTAB_UNLOCK(), but 
  SIDTAB_UNLOCK() enables any interruptions because it's defined as 
  spin_unlock_irq(). If an interruption occurs between SIDTAB_UNLOCK() and 
  POLICY_WRUNLOCK, and interruption context try to hold the POLICY_RDLOCK, 
  then a deadlock happen in the result.

The solution is to save & restore flags on the inner lock, per the patch 
below.

Please apply.

Signed-off-by: James Morris <jmorris@xxxxxxxxxx>
Signed-off-by: Stephen Smalley <sds@xxxxxxxxxxxxxx>
Signed-off-by: Kaigai Kohei <kaigai@xxxxxxxxxxxxx>

---

 security/selinux/ss/sidtab.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff -purN -X dontdiff linux-2.6.9-mm1.p/security/selinux/ss/sidtab.c linux-2.6.9-mm1.w/security/selinux/ss/sidtab.c
--- linux-2.6.9-mm1.p/security/selinux/ss/sidtab.c	2004-08-14 01:37:25.000000000 -0400
+++ linux-2.6.9-mm1.w/security/selinux/ss/sidtab.c	2004-10-22 11:53:03.152654328 -0400
@@ -16,8 +16,8 @@
 (sid & SIDTAB_HASH_MASK)
 
 #define INIT_SIDTAB_LOCK(s) spin_lock_init(&s->lock)
-#define SIDTAB_LOCK(s) spin_lock_irq(&s->lock)
-#define SIDTAB_UNLOCK(s) spin_unlock_irq(&s->lock)
+#define SIDTAB_LOCK(s, x) spin_lock_irqsave(&s->lock, x)
+#define SIDTAB_UNLOCK(s, x) spin_unlock_irqrestore(&s->lock, x)
 
 int sidtab_init(struct sidtab *s)
 {
@@ -237,12 +237,13 @@ int sidtab_context_to_sid(struct sidtab 
 {
 	u32 sid;
 	int ret = 0;
+	unsigned long flags;
 
 	*out_sid = SECSID_NULL;
 
 	sid = sidtab_search_context(s, context);
 	if (!sid) {
-		SIDTAB_LOCK(s);
+		SIDTAB_LOCK(s, flags);
 		/* Rescan now that we hold the lock. */
 		sid = sidtab_search_context(s, context);
 		if (sid)
@@ -257,7 +258,7 @@ int sidtab_context_to_sid(struct sidtab 
 		if (ret)
 			s->next_sid--;
 unlock_out:
-		SIDTAB_UNLOCK(s);
+		SIDTAB_UNLOCK(s, flags);
 	}
 
 	if (ret)
@@ -320,17 +321,21 @@ void sidtab_destroy(struct sidtab *s)
 
 void sidtab_set(struct sidtab *dst, struct sidtab *src)
 {
-	SIDTAB_LOCK(src);
+	unsigned long flags;
+
+	SIDTAB_LOCK(src, flags);
 	dst->htable = src->htable;
 	dst->nel = src->nel;
 	dst->next_sid = src->next_sid;
 	dst->shutdown = 0;
-	SIDTAB_UNLOCK(src);
+	SIDTAB_UNLOCK(src, flags);
 }
 
 void sidtab_shutdown(struct sidtab *s)
 {
-	SIDTAB_LOCK(s);
+	unsigned long flags;
+
+	SIDTAB_LOCK(s, flags);
 	s->shutdown = 1;
-	SIDTAB_UNLOCK(s);
+	SIDTAB_UNLOCK(s, flags);
 }


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/