What type of locking is needed for ss/sidtab.c?
Hiten Pandya
hiten at angelica.unixdaemons.com
Wed Jul 24 16:03:22 GMT 2002
On Wed, Jul 24, 2002 at 10:35:45AM -0400, John Baldwin wrote the words in effect of:
> On 24-Jul-2002 Hiten Pandya wrote:
> > 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.
>
> No, in FreeBSD you should use default mutexes. spin mutexes should almost
> never be used. AFAIK, Linux doesn't have mutexes that block when they are
> contested and doing priority propagation, etc. just spin locks.
Oh OK. The reason I used a spin mutex is because I thought this would
be in the startup, but I was wrong. Nevertheless, I have changed to
MTX_DEF type mutexes.
> >> 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.
>
> Well, you would need M_NOWAIT to malloc() to get it to be nonblocking,
> or rework the algo to malloc() before you acquire the locks.
Assuming you refered to sidtab_get_sids(), I have done the neccessary
changes to it to make use of nonblocking malloc().
Also, I used a Shared/Exclusive (sx) lock in the code, because according
to Linux terminology and implementation(?), read/write refers to
shared/exclusive. Is there any problems if I were to use SX_SYSINIT()
or sx locks?
I am attaching a newer version of the patch with this mail.
Thanking in advance.
--
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 16:53:43 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,10 @@
#define INTERRUPTS_OFF local_irq_disable()
#define INTERRUPTS_ON local_irq_enable()
#else
-#define LOAD_LOCK
-#define LOAD_UNLOCK
-#define INTERRUPTS_OFF
-#define INTERRUPTS_ON
+struct mtx;
+struct mtx load_lock;
+#define LOAD_INIT \
+ MTX_SYSINIT(load_lock, &load_lock, "SEBSD Load Lock", MTX_DEF)
+#define LOAD_LOCK mtx_lock(&load_lock)
+#define LOAD_UNLOCK mtx_unlock(&load_lock)
#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 16:50:09 2002
@@ -12,6 +12,8 @@
#include <sys/conf.h>
#include <sys/kernel.h>
#include <sys/systm.h>
+#include <sys/lock.h>
+#include <sys/mutex.h>
#endif /* FreeBSD _KERNEL */
#include <machine/limits.h>
@@ -29,9 +31,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 +262,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);
@@ -358,8 +363,8 @@
SIDTAB_LOCK(s);
mynel = s->nel;
mysids = (security_id_t *)sebsd_malloc(mynel*sizeof(security_id_t),
- M_SEBSD_SS, M_WAITOK);
- if (!mysids) {
+ M_SEBSD_SS, M_NOWAIT);
+ if (mysids == NULL) {
rc = -ENOMEM;
goto out;
}
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;
More information about the trustedbsd-discuss
mailing list