Skip to content

Commit

Permalink
ipc: fix race with LSMs
Browse files Browse the repository at this point in the history 
Currently, IPC mechanisms do security and auditing related checks under
RCU.  However, since security modules can free the security structure,
for example, through selinux_[sem,msg_queue,shm]_free_security(), we can
race if the structure is freed before other tasks are done with it,
creating a use-after-free condition.  Manfred illustrates this nicely,
for instance with shared mem and selinux:

 -> do_shmat calls rcu_read_lock()
 -> do_shmat calls shm_object_check().
     Checks that the object is still valid - but doesn't acquire any locks.
     Then it returns.
 -> do_shmat calls security_shm_shmat (e.g. selinux_shm_shmat)
 -> selinux_shm_shmat calls ipc_has_perm()
 -> ipc_has_perm accesses ipc_perms->security

shm_close()
 -> shm_close acquires rw_mutex & shm_lock
 -> shm_close calls shm_destroy
 -> shm_destroy calls security_shm_free (e.g. selinux_shm_free_security)
 -> selinux_shm_free_security calls ipc_free_security(&shp->shm_perm)
 -> ipc_free_security calls kfree(ipc_perms->security)

This patch delays the freeing of the security structures after all RCU
readers are done.  Furthermore it aligns the security life cycle with
that of the rest of IPC - freeing them based on the reference counter.
For situations where we need not free security, the current behavior is
kept.  Linus states:

 "... the old behavior was suspect for another reason too: having the
  security blob go away from under a user sounds like it could cause
  various other problems anyway, so I think the old code was at least
  _prone_ to bugs even if it didn't have catastrophic behavior."

I have tested this patch with IPC testcases from LTP on both my
quad-core laptop and on a 64 core NUMA server.  In both cases selinux is
enabled, and tests pass for both voluntary and forced preemption models.
While the mentioned races are theoretical (at least no one as reported
them), I wanted to make sure that this new logic doesn't break anything
we weren't aware of.

Suggested-by: Linus Torvalds <[email protected]>
Signed-off-by: Davidlohr Bueso <[email protected]>
Acked-by: Manfred Spraul <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
@torvalds
Davidlohr Bueso authored and torvalds committed Sep 24, 2013
1 parent 4a10c2a commit 53dad6d
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 48 deletions.
19 changes: 13 additions & 6 deletions ipc/msg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,15 @@ static inline void msg_rmid(struct ipc_namespace *ns, struct msg_queue *s)
ipc_rmid(&msg_ids(ns), &s->q_perm);
}

static void msg_rcu_free(struct rcu_head *head)
{
struct ipc_rcu *p = container_of(head, struct ipc_rcu, rcu);
struct msg_queue *msq = ipc_rcu_to_struct(p);

security_msg_queue_free(msq);
ipc_rcu_free(head);
}

/**
* newque - Create a new msg queue
* @ns: namespace
Expand All @@ -189,15 +198,14 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
msq->q_perm.security = NULL;
retval = security_msg_queue_alloc(msq);
if (retval) {
ipc_rcu_putref(msq);
ipc_rcu_putref(msq, ipc_rcu_free);
return retval;
}

/* ipc_addid() locks msq upon success. */
id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni);
if (id < 0) {
security_msg_queue_free(msq);
ipc_rcu_putref(msq);
ipc_rcu_putref(msq, msg_rcu_free);
return id;
}

Expand Down Expand Up @@ -276,8 +284,7 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
free_msg(msg);
}
atomic_sub(msq->q_cbytes, &ns->msg_bytes);
security_msg_queue_free(msq);
ipc_rcu_putref(msq);
ipc_rcu_putref(msq, msg_rcu_free);
}

/*
Expand Down Expand Up @@ -717,7 +724,7 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext,
rcu_read_lock();
ipc_lock_object(&msq->q_perm);

ipc_rcu_putref(msq);
ipc_rcu_putref(msq, ipc_rcu_free);
if (msq->q_perm.deleted) {
err = -EIDRM;
goto out_unlock0;
Expand Down
34 changes: 18 additions & 16 deletions ipc/sem.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,15 @@ static void merge_queues(struct sem_array *sma)
}
}

static void sem_rcu_free(struct rcu_head *head)
{
struct ipc_rcu *p = container_of(head, struct ipc_rcu, rcu);
struct sem_array *sma = ipc_rcu_to_struct(p);

security_sem_free(sma);
ipc_rcu_free(head);
}

/*
* If the request contains only one semaphore operation, and there are
* no complex transactions pending, lock only the semaphore involved.
Expand Down Expand Up @@ -374,12 +383,7 @@ static inline struct sem_array *sem_obtain_object_check(struct ipc_namespace *ns
static inline void sem_lock_and_putref(struct sem_array *sma)
{
sem_lock(sma, NULL, -1);
ipc_rcu_putref(sma);
}

static inline void sem_putref(struct sem_array *sma)
{
ipc_rcu_putref(sma);
ipc_rcu_putref(sma, ipc_rcu_free);
}

static inline void sem_rmid(struct ipc_namespace *ns, struct sem_array *s)
Expand Down Expand Up @@ -458,14 +462,13 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
sma->sem_perm.security = NULL;
retval = security_sem_alloc(sma);
if (retval) {
ipc_rcu_putref(sma);
ipc_rcu_putref(sma, ipc_rcu_free);
return retval;
}

id = ipc_addid(&sem_ids(ns), &sma->sem_perm, ns->sc_semmni);
if (id < 0) {
security_sem_free(sma);
ipc_rcu_putref(sma);
ipc_rcu_putref(sma, sem_rcu_free);
return id;
}
ns->used_sems += nsems;
Expand Down Expand Up @@ -1047,8 +1050,7 @@ static void freeary(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)

wake_up_sem_queue_do(&tasks);
ns->used_sems -= sma->sem_nsems;
security_sem_free(sma);
ipc_rcu_putref(sma);
ipc_rcu_putref(sma, sem_rcu_free);
}

static unsigned long copy_semid_to_user(void __user *buf, struct semid64_ds *in, int version)
Expand Down Expand Up @@ -1292,7 +1294,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
rcu_read_unlock();
sem_io = ipc_alloc(sizeof(ushort)*nsems);
if(sem_io == NULL) {
sem_putref(sma);
ipc_rcu_putref(sma, ipc_rcu_free);
return -ENOMEM;
}

Expand Down Expand Up @@ -1328,20 +1330,20 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
if(nsems > SEMMSL_FAST) {
sem_io = ipc_alloc(sizeof(ushort)*nsems);
if(sem_io == NULL) {
sem_putref(sma);
ipc_rcu_putref(sma, ipc_rcu_free);
return -ENOMEM;
}
}

if (copy_from_user (sem_io, p, nsems*sizeof(ushort))) {
sem_putref(sma);
ipc_rcu_putref(sma, ipc_rcu_free);
err = -EFAULT;
goto out_free;
}

for (i = 0; i < nsems; i++) {
if (sem_io[i] > SEMVMX) {
sem_putref(sma);
ipc_rcu_putref(sma, ipc_rcu_free);
err = -ERANGE;
goto out_free;
}
Expand Down Expand Up @@ -1629,7 +1631,7 @@ static struct sem_undo *find_alloc_undo(struct ipc_namespace *ns, int semid)
/* step 2: allocate new undo structure */
new = kzalloc(sizeof(struct sem_undo) + sizeof(short)*nsems, GFP_KERNEL);
if (!new) {
sem_putref(sma);
ipc_rcu_putref(sma, ipc_rcu_free);
return ERR_PTR(-ENOMEM);
}

Expand Down
17 changes: 12 additions & 5 deletions ipc/shm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,15 @@ static inline void shm_lock_by_ptr(struct shmid_kernel *ipcp)
ipc_lock_object(&ipcp->shm_perm);
}

static void shm_rcu_free(struct rcu_head *head)
{
struct ipc_rcu *p = container_of(head, struct ipc_rcu, rcu);
struct shmid_kernel *shp = ipc_rcu_to_struct(p);

security_shm_free(shp);
ipc_rcu_free(head);
}

static inline void shm_rmid(struct ipc_namespace *ns, struct shmid_kernel *s)
{
ipc_rmid(&shm_ids(ns), &s->shm_perm);
Expand Down Expand Up @@ -208,8 +217,7 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp)
user_shm_unlock(file_inode(shp->shm_file)->i_size,
shp->mlock_user);
fput (shp->shm_file);
security_shm_free(shp);
ipc_rcu_putref(shp);
ipc_rcu_putref(shp, shm_rcu_free);
}

/*
Expand Down Expand Up @@ -497,7 +505,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
shp->shm_perm.security = NULL;
error = security_shm_alloc(shp);
if (error) {
ipc_rcu_putref(shp);
ipc_rcu_putref(shp, ipc_rcu_free);
return error;
}

Expand Down Expand Up @@ -566,8 +574,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
user_shm_unlock(size, shp->mlock_user);
fput(file);
no_file:
security_shm_free(shp);
ipc_rcu_putref(shp);
ipc_rcu_putref(shp, shm_rcu_free);
return error;
}

Expand Down
32 changes: 12 additions & 20 deletions ipc/util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,6 @@ void ipc_free(void* ptr, int size)
kfree(ptr);
}

struct ipc_rcu {
struct rcu_head rcu;
atomic_t refcount;
} ____cacheline_aligned_in_smp;

/**
* ipc_rcu_alloc - allocate ipc and rcu space
* @size: size desired
Expand All @@ -505,27 +500,24 @@ int ipc_rcu_getref(void *ptr)
return atomic_inc_not_zero(&p->refcount);
}

/**
* ipc_schedule_free - free ipc + rcu space
* @head: RCU callback structure for queued work
*/
static void ipc_schedule_free(struct rcu_head *head)
{
vfree(container_of(head, struct ipc_rcu, rcu));
}

void ipc_rcu_putref(void *ptr)
void ipc_rcu_putref(void *ptr, void (*func)(struct rcu_head *head))
{
struct ipc_rcu *p = ((struct ipc_rcu *)ptr) - 1;

if (!atomic_dec_and_test(&p->refcount))
return;

if (is_vmalloc_addr(ptr)) {
call_rcu(&p->rcu, ipc_schedule_free);
} else {
kfree_rcu(p, rcu);
}
call_rcu(&p->rcu, func);
}

void ipc_rcu_free(struct rcu_head *head)
{
struct ipc_rcu *p = container_of(head, struct ipc_rcu, rcu);

if (is_vmalloc_addr(p))
vfree(p);
else
kfree(p);
}

/**
Expand Down
10 changes: 9 additions & 1 deletion ipc/util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ static inline void msg_exit_ns(struct ipc_namespace *ns) { }
static inline void shm_exit_ns(struct ipc_namespace *ns) { }
#endif

struct ipc_rcu {
struct rcu_head rcu;
atomic_t refcount;
} ____cacheline_aligned_in_smp;

#define ipc_rcu_to_struct(p) ((void *)(p+1))

/*
* Structure that holds the parameters needed by the ipc operations
* (see after)
Expand Down Expand Up @@ -120,7 +127,8 @@ void ipc_free(void* ptr, int size);
*/
void* ipc_rcu_alloc(int size);
int ipc_rcu_getref(void *ptr);
void ipc_rcu_putref(void *ptr);
void ipc_rcu_putref(void *ptr, void (*func)(struct rcu_head *head));
void ipc_rcu_free(struct rcu_head *head);

struct kern_ipc_perm *ipc_lock(struct ipc_ids *, int);
struct kern_ipc_perm *ipc_obtain_object(struct ipc_ids *ids, int id);
Expand Down

0 comments on commit 53dad6d

Please sign in to comment.