Commit 43d1aa34 by Serge Hallyn Committed by Stéphane Graber

Fix up struct lxc_container locking

1. in container_free, set c->privlock to NULL before calling sem_destroy, to prevent a window where another thread could call sem_wait(c->privlock) while c->privlock is not NULL but is already destroyed. 2. in container_get, check for numthreads < 0 before calling lxclock. Once numthreads is 0, it never goes back up. Following is a comment added to lxccontainer.c: /* * Consider the following case: freer | racing get()er ================================================================== lxc_container_put() | lxc_container_get() \ lxclock(c->privlock) | c->numthreads < 1? (no) \ c->numthreads = 0 | \ lxclock(c->privlock) -> waits \ lxcunlock() | \ \ lxc_container_free() | \ lxclock() returns | \ c->numthreads < 1 -> return 0 \ \ (free stuff) | \ \ sem_destroy(privlock) | * When the get()er checks numthreads the first time, one of the following * is true: * 1. freer has set numthreads = 0. get() returns 0 * 2. freer is between lxclock and setting numthreads to 0. get()er will * sem_wait on privlock, get lxclock after freer() drops it, then see * numthreads is 0 and exit without touching lxclock again.. * 3. freer has not yet locked privlock. If get()er runs first, then put()er * will see --numthreads = 1 and not call lxc_container_free(). */ Signed-off-by: 's avatarSerge Hallyn <serge.hallyn@ubuntu.com> Acked-by: 's avatarSeth Arnold <seth.arnold@canonical.com> Acked-by: 's avatarStéphane Graber <stgraber@ubuntu.com>
parent e649c803
......@@ -72,9 +72,10 @@ static void lxc_container_free(struct lxc_container *c)
c->slock = NULL;
}
if (c->privlock) {
sem_destroy(c->privlock);
free(c->privlock);
sem_t *l = c->privlock;
c->privlock = NULL;
sem_destroy(l);
free(l);
}
if (c->name) {
free(c->name);
......@@ -91,11 +92,39 @@ static void lxc_container_free(struct lxc_container *c)
free(c);
}
/*
* Consider the following case:
freer | racing get()er
==================================================================
lxc_container_put() | lxc_container_get()
\ lxclock(c->privlock) | c->numthreads < 1? (no)
\ c->numthreads = 0 | \ lxclock(c->privlock) -> waits
\ lxcunlock() | \
\ lxc_container_free() | \ lxclock() returns
| \ c->numthreads < 1 -> return 0
\ \ (free stuff) |
\ \ sem_destroy(privlock) |
* When the get()er checks numthreads the first time, one of the following
* is true:
* 1. freer has set numthreads = 0. get() returns 0
* 2. freer is between lxclock and setting numthreads to 0. get()er will
* sem_wait on privlock, get lxclock after freer() drops it, then see
* numthreads is 0 and exit without touching lxclock again..
* 3. freer has not yet locked privlock. If get()er runs first, then put()er
* will see --numthreads = 1 and not call lxc_container_free().
*/
int lxc_container_get(struct lxc_container *c)
{
if (!c)
return 0;
// if someone else has already started freeing the container, don't
// try to take the lock, which may be invalid
if (c->numthreads < 1)
return 0;
if (lxclock(c->privlock, 0))
return 0;
if (c->numthreads < 1) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment