linux.git/ipc/msg.c, branch v3.10.31

ipc,msg: prevent race with rmid in msgsnd,msgrcv

2013-10-18T14:45:48+00:00

commit 4271b05a227dc6175b66c3d9941aeab09048aeb2 upstream.

This fixes a race in both msgrcv() and msgsnd() between finding the msg
and actually dealing with the queue, as another thread can delete shmid
underneath us if we are preempted before acquiring the
kern_ipc_perm.lock.

Manfred illustrates this nicely:

Assume a preemptible kernel that is preempted just after

    msq = msq_obtain_object_check(ns, msqid)

in do_msgrcv().  The only lock that is held is rcu_read_lock().

Now the other thread processes IPC_RMID.  When the first task is
resumed, then it will happily wait for messages on a deleted queue.

Fix this by checking for if the queue has been deleted after taking the
lock.

Signed-off-by: Davidlohr Bueso 
Reported-by: Manfred Spraul 
Cc: Rik van Riel 
Cc: Mike Galbraith 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Signed-off-by: Greg Kroah-Hartman

ipc: fix race with LSMs

2013-10-18T14:45:48+00:00

commit 53dad6d3a8e5ac1af8bacc6ac2134ae1a8b085f1 upstream.

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 
Signed-off-by: Davidlohr Bueso 
Acked-by: Manfred Spraul 
Signed-off-by: Linus Torvalds 
Cc: Mike Galbraith 
Signed-off-by: Greg Kroah-Hartman

ipc,msg: drop msg_unlock

2013-10-18T14:45:48+00:00

commit 4718787d1f626f45ddb239912bc07266b9880044 upstream.

There is only one user left, drop this function and just call
ipc_unlock_object() and rcu_read_unlock().

Signed-off-by: Davidlohr Bueso 
Tested-by: Sedat Dilek 
Cc: Rik van Riel 
Cc: Manfred Spraul 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Cc: Mike Galbraith 
Signed-off-by: Greg Kroah-Hartman

ipc: rename ids->rw_mutex

2013-10-18T14:45:47+00:00

commit d9a605e40b1376eb02b067d7690580255a0df68f upstream.

Since in some situations the lock can be shared for readers, we shouldn't
be calling it a mutex, rename it to rwsem.

Signed-off-by: Davidlohr Bueso 
Tested-by: Sedat Dilek 
Cc: Rik van Riel 
Cc: Manfred Spraul 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Cc: Mike Galbraith 
Signed-off-by: Greg Kroah-Hartman

ipc/msg.c: Fix lost wakeup in msgsnd().

2013-10-18T14:45:47+00:00

commit bebcb928c820d0ee83aca4b192adc195e43e66a2 upstream.

The check if the queue is full and adding current to the wait queue of
pending msgsnd() operations (ss_add()) must be atomic.

Otherwise:
 - the thread that performs msgsnd() finds a full queue and decides to
   sleep.
 - the thread that performs msgrcv() first reads all messages from the
   queue and then sleeps, because the queue is empty.
 - the msgrcv() calls do not perform any wakeups, because the msgsnd()
   task has not yet called ss_add().
 - then the msgsnd()-thread first calls ss_add() and then sleeps.

Net result: msgsnd() and msgrcv() both sleep forever.

Observed with msgctl08 from ltp with a preemptible kernel.

Fix: Call ipc_lock_object() before performing the check.

The patch also moves security_msg_queue_msgsnd() under ipc_lock_object:
 - msgctl(IPC_SET) explicitely mentions that it tries to expunge any
   pending operations that are not allowed anymore with the new
   permissions.  If security_msg_queue_msgsnd() is called without locks,
   then there might be races.
 - it makes the patch much simpler.

Reported-and-tested-by: Vineet Gupta 
Acked-by: Rik van Riel 
Signed-off-by: Manfred Spraul 
Signed-off-by: Linus Torvalds 
Cc: Mike Galbraith 
Signed-off-by: Greg Kroah-Hartman

ipc: remove unused functions

2013-10-18T14:45:46+00:00

commit 9ad66ae65fc8d3e7e3344310fb0aa835910264fe upstream.

We can now drop the msg_lock and msg_lock_check functions along with a
bogus comment introduced previously in semctl_down.

Signed-off-by: Davidlohr Bueso 
Cc: Andi Kleen 
Cc: Rik van Riel 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Cc: Mike Galbraith 
Signed-off-by: Greg Kroah-Hartman

ipc,msg: shorten critical region in msgrcv

2013-10-18T14:45:46+00:00

commit 41a0d523d0f626e9da0dc01de47f1b89058033cf upstream.

do_msgrcv() is the last msg queue function that abuses the ipc lock Take
it only when needed when actually updating msq.

Signed-off-by: Davidlohr Bueso 
Cc: Andi Kleen 
Cc: Rik van Riel 
Tested-by: Sedat Dilek 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Cc: Mike Galbraith 
Signed-off-by: Greg Kroah-Hartman

ipc,msg: shorten critical region in msgsnd

2013-10-18T14:45:46+00:00

commit 3dd1f784ed6603d7ab1043e51e6371235edf2313 upstream.

do_msgsnd() is another function that does too many things with the ipc
object lock acquired.  Take it only when needed when actually updating
msq.

Signed-off-by: Davidlohr Bueso 
Cc: Andi Kleen 
Cc: Rik van Riel 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Cc: Mike Galbraith 
Signed-off-by: Greg Kroah-Hartman

ipc,msg: make msgctl_nolock lockless

2013-10-18T14:45:46+00:00

commit ac0ba20ea6f2201a1589d6dc26ad1a4f0f967bb8 upstream.

While the INFO cmd doesn't take the ipc lock, the STAT commands do
acquire it unnecessarily.  We can do the permissions and security checks
only holding the rcu lock.

This function now mimics semctl_nolock().

Signed-off-by: Davidlohr Bueso 
Cc: Andi Kleen 
Cc: Rik van Riel 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Cc: Mike Galbraith 
Signed-off-by: Greg Kroah-Hartman

ipc,msg: introduce lockless functions to obtain the ipc object

2013-10-18T14:45:46+00:00

commit a5001a0d9768568de5d613c3b3a5b9c7721299da upstream.

Add msq_obtain_object() and msq_obtain_object_check(), which will allow
us to get the ipc object without acquiring the lock.  Just as with
semaphores, these functions are basically wrappers around
ipc_obtain_object*().

Signed-off-by: Davidlohr Bueso 
Cc: Andi Kleen 
Cc: Rik van Riel 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Cc: Mike Galbraith 
Signed-off-by: Greg Kroah-Hartman