Re: [PATCH] ipc/mqueue: Avoid relying on a stack reference past its expiry

From: Manfred Spraul
Date: Sat May 08 2021 - 13:21:39 EST


Hi Davidlohr,

On 5/4/21 11:53 PM, Davidlohr Bueso wrote:
@@ -1005,11 +1023,9 @@ static inline void __pipelined_op(struct wake_q_head *wake_q,
                   struct ext_wait_queue *this)
 {
     list_del(&this->list);
-    get_task_struct(this->task);
-
+    wake_q_add(wake_q, this->task);
     /* see MQ_BARRIER for purpose/pairing */
     smp_store_release(&this->state, STATE_READY);
-    wake_q_add_safe(wake_q, this->task);
 }

 /* pipelined_send() - send a message directly to the task waiting in

This can result in lost wakeups:
wake_q_add() wakes up this->task, and the tasks reads this->state before the smp_store_release() writes STATE_READY.

I would really prefer if we make kernel/futex.c and ipc/whatever as similar as possible:
The futex slow path, ipc/sem.c, ipc/msg.c and ipc/mqueue.c are all the same, thus the code should be the same as well.
[a task goes to sleep, another wakes it up, the woken up task doesn't acquire any locks, and the wake_q framework is used]

--

    Manfred