What type of locking is needed for ss/sidtab.c?

[Hiten Pandya]

On Tue, Jul 23, 2002 at 04:56:16PM -0700, Chris Wright wrote the words in effect of:
> The Linux case currently does:
> 
> #define SIDTAB_LOCK(s, flags) spin_lock_irqsave(&s->lock,flags)
> #define SIDTAB_UNLOCK(s,flags) spin_unlock_irqrestore(&s->lock,flags)

OK.  This means that the use of Spin locks should suffice in FreeBSD.  I
guess the safe_up() and safe_down() stuff in services_private.h is
deprecated, because I did a google search and it came up with diffs
which removed it.

> This assumes non-sleeping kmalloc in sidtab_get_sids().

OK.  I have given a _try_ to add/port the locking stuff in the FreeBSD
case for the security server.  I am not sure how correct it is, but I
done many checks of what I was up to; patch is attached with this mail.

I apologise in advance for any silly mistakes. :-)

I have not been able to test the patch properly, because I am getting a
panic in sebsd_ops() because I modified this file by mistake, so I am
currently updating my source.

The (review) patch is also available at:
	http://www.unixdaemons.com/~hiten/work/diffs/sidtab.sebsd.patch
Thanks.

-- 
Hiten Pandya
http://www.unixdaemons.com/~hiten
hiten at unixdaemons.com, hiten at uk.FreeBSD.org, hiten at xMach.org
PGP: http://pgp.mit.edu:11371/pks/lookup?search=Hiten+Pandya&op=index
-------------- next part --------------
Only in sys/security/sebsd/ss_0/: old
Only in sys/security/sebsd/ss_0/: policy_locks.txt
diff -u sys/security/sebsd/ss_0/old/services.c sys/security/sebsd/ss_0/services.c
--- sys/security/sebsd/ss_0/old/services.c	Tue Jul 23 20:52:56 2002
+++ sys/security/sebsd/ss_0/services.c	Wed Jul 24 12:32:06 2002
@@ -20,6 +20,9 @@
 #include <sys/kernel.h>
 #include <sys/systm.h>
 #include <sys/malloc.h>
+#include <sys/lock.h>
+#include <sys/mutex.h>
+#include <sys/sx.h>
 #else /* _KERNEL */
 #include <errno.h>
 #endif /* _KERNEL */
@@ -57,6 +60,10 @@
  */
 static __u32 latest_granting = 0;
 
+#ifdef _KERNEL
+POLICY_INIT;
+LOAD_INIT;
+#endif
 
 /*
  * Return the boolean value of a constraint expression 
@@ -1346,7 +1353,6 @@
 	POLICY_RDUNLOCK;
 	return rc;	
 }
-
 
 /* FLASK */
 
diff -u sys/security/sebsd/ss_0/old/services_private.h sys/security/sebsd/ss_0/services_private.h
--- sys/security/sebsd/ss_0/old/services_private.h	Tue Jul 23 20:52:56 2002
+++ sys/security/sebsd/ss_0/services_private.h	Wed Jul 24 12:32:00 2002
@@ -61,10 +61,14 @@
 #define POLICY_RDUNLOCK safe_up(&policy_sem)
 #define POLICY_WRUNLOCK safe_up(&policy_sem)
 #else
-#define POLICY_RDLOCK
-#define POLICY_WRLOCK
-#define POLICY_RDUNLOCK
-#define POLICY_WRUNLOCK
+struct sx;
+struct sx policy_lock;
+#define	POLICY_INIT		\
+	SX_SYSINIT(policy_lock, &policy_lock, "SEBSD Policy Lock")
+#define POLICY_RDLOCK 		sx_slock(&policy_lock)
+#define POLICY_WRLOCK 		sx_xlock(&policy_lock)
+#define POLICY_RDUNLOCK 	sx_sunlock(&policy_lock)
+#define POLICY_WRUNLOCK 	sx_xunlock(&policy_lock)
 #endif
 
 #ifdef __KERNEL__
@@ -74,8 +78,12 @@
 #define INTERRUPTS_OFF local_irq_disable()
 #define INTERRUPTS_ON local_irq_enable()
 #else
-#define LOAD_LOCK 
-#define LOAD_UNLOCK
+struct mtx;
+struct mtx load_lock;
+#define LOAD_INIT	\
+	MTX_SYSINIT(load_lock, &load_lock, "SEBSD Load Lock", MTX_SPIN)
+#define LOAD_LOCK 	mtx_lock_spin(&load_lock)
+#define LOAD_UNLOCK	mtx_unlock_spin(&load_lock)
 #define INTERRUPTS_OFF 
 #define INTERRUPTS_ON 
 #endif
diff -u sys/security/sebsd/ss_0/old/sidtab.c sys/security/sebsd/ss_0/sidtab.c
--- sys/security/sebsd/ss_0/old/sidtab.c	Tue Jul 23 20:52:56 2002
+++ sys/security/sebsd/ss_0/sidtab.c	Wed Jul 24 13:51:02 2002
@@ -29,9 +29,11 @@
 #define SIDTAB_LOCK(s) safe_down(&s->sem)
 #define SIDTAB_UNLOCK(s) safe_up(&s->sem)
 #else
-#define INIT_SIDTAB_LOCK(s) 
-#define SIDTAB_LOCK(s) 0
-#define SIDTAB_UNLOCK(s)
+#define INIT_SIDTAB_LOCK(s) \
+	mtx_init(&(s)->sidtab_mtx, "SID Table lock", NULL, MTX_DEF)
+#define SIDTAB_TRYLOCK(s) mtx_trylock(&(s)->sidtab_mtx)
+#define SIDTAB_LOCK(s)    mtx_lock(&(s)->sidtab_mtx)
+#define SIDTAB_UNLOCK(s)  mtx_unlock(&(s)->sidtab_mtx)
 #endif
 
 #ifndef __TBD_CDV__
@@ -258,7 +260,8 @@
 
 	sid = sidtab_search_context(s, context);
 	if (!sid) {
-		if (SIDTAB_LOCK(s))
+		ret = SIDTAB_TRYLOCK(s);
+		if (ret == 0)
 			return -EAGAIN;
 		/* Rescan now that we hold the semaphore. */
 		sid = sidtab_search_context(s, context);
diff -u sys/security/sebsd/ss_0/old/sidtab.h sys/security/sebsd/ss_0/sidtab.h
--- sys/security/sebsd/ss_0/old/sidtab.h	Tue Jul 23 20:52:56 2002
+++ sys/security/sebsd/ss_0/sidtab.h	Wed Jul 24 13:49:53 2002
@@ -27,12 +27,15 @@
 
 #define SIDTAB_SIZE SIDTAB_HASH_BUCKETS
 
+struct mtx;
 typedef struct {
 	sidtab_ptr_t *htable;
 	unsigned int nel;	/* number of elements */
 	unsigned int next_sid;	/* next SID to allocate */
 #ifdef __KERNEL__
 	struct semaphore sem;
+#else
+	struct mtx sidtab_mtx;	/* serialisation */
 #endif
 } sidtab_t;