Skip to content

Commit

Permalink
ipc/msg.c: Fix lost wakeup in msgsnd().
Browse files Browse the repository at this point in the history
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 <[email protected]>
Acked-by: Rik van Riel <[email protected]>
Cc: [email protected]  # for 3.11
Signed-off-by: Manfred Spraul <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
manfred-colorfu authored and torvalds committed Sep 3, 2013
1 parent ec1882a commit bebcb92
Showing 1 changed file with 5 additions and 7 deletions.
12 changes: 5 additions & 7 deletions ipc/msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -680,16 +680,18 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext,
goto out_unlock1;
}

ipc_lock_object(&msq->q_perm);

for (;;) {
struct msg_sender s;

err = -EACCES;
if (ipcperms(ns, &msq->q_perm, S_IWUGO))
goto out_unlock1;
goto out_unlock0;

err = security_msg_queue_msgsnd(msq, msg, msgflg);
if (err)
goto out_unlock1;
goto out_unlock0;

if (msgsz + msq->q_cbytes <= msq->q_qbytes &&
1 + msq->q_qnum <= msq->q_qbytes) {
Expand All @@ -699,10 +701,9 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext,
/* queue full, wait: */
if (msgflg & IPC_NOWAIT) {
err = -EAGAIN;
goto out_unlock1;
goto out_unlock0;
}

ipc_lock_object(&msq->q_perm);
ss_add(msq, &s);

if (!ipc_rcu_getref(msq)) {
Expand Down Expand Up @@ -730,10 +731,7 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext,
goto out_unlock0;
}

ipc_unlock_object(&msq->q_perm);
}

ipc_lock_object(&msq->q_perm);
msq->q_lspid = task_tgid_vnr(current);
msq->q_stime = get_seconds();

Expand Down

0 comments on commit bebcb92

Please sign in to comment.