From 06f91b4bbc440e9509c85acb7be1b15388b7bc0f Mon Sep 17 00:00:00 2001 From: Manfred Spraul Date: Sat, 8 May 2021 18:41:30 +0200 Subject: [PATCH] ipc/sem.c, mqueue.c, msg.c: Fix incorrect wake_q_add_safe() The wakeup code used by ipc contains a potential use-after-free: When modifying the code to use wake_q_add_safe(), it was forgotten to transfer the task struct pointer into a local variable before the smp_store_release(). Solution: Add local variables to the affected functions. Result: ipc is now using the same approach as kernel/futex.c and kernel/locking/rwsem.c. Note: No need to use READ_ONCE(), as smp_store_release() contains a barrier(), thus the compiler cannot reread ptr->task. Signed-off-by: Manfred Spraul Fixes: c5b2cbdbdac563 ("ipc/mqueue.c: update/document memory barriers") Fixes: 8116b54e7e23ef ("ipc/sem.c: document and update memory barriers") Fixes: 0d97a82ba830d8 ("ipc/msg.c: update and document memory barriers") Reported-by: Matthias von Faber Cc: Varad Gautam Cc: Christian Brauner Cc: Oleg Nesterov Cc: "Eric W. Biederman" Cc: Manfred Spraul Cc: Andrew Morton Cc: Davidlohr Bueso --- ipc/mqueue.c | 12 +++++++++--- ipc/msg.c | 6 ++++-- ipc/sem.c | 6 ++++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 8031464ed4ae..838c4f24a337 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -78,7 +78,11 @@ struct posix_msg_tree_node { * MQ_BARRIER: * To achieve proper release/acquire memory barrier pairing, the state is set to * STATE_READY with smp_store_release(), and it is read with READ_ONCE followed - * by smp_acquire__after_ctrl_dep(). In addition, wake_q_add_safe() is used. + * by smp_acquire__after_ctrl_dep(). Immediately after the smp_store_release(), + * the struct ext_wait_queue can go out of scope. Thus the task struct pointer + * is copied into a local variable. The wakeup is performed using + * wake_q_add_safe(). + * * * This prevents the following races: * @@ -1004,12 +1008,14 @@ static inline void __pipelined_op(struct wake_q_head *wake_q, struct mqueue_inode_info *info, struct ext_wait_queue *this) { + struct task_struct *task = this->task; + list_del(&this->list); - get_task_struct(this->task); + get_task_struct(task); /* see MQ_BARRIER for purpose/pairing */ smp_store_release(&this->state, STATE_READY); - wake_q_add_safe(wake_q, this->task); + wake_q_add_safe(wake_q, task); } /* pipelined_send() - send a message directly to the task waiting in diff --git a/ipc/msg.c b/ipc/msg.c index acd1bc7af55a..d273482b71ea 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -251,11 +251,13 @@ static void expunge_all(struct msg_queue *msq, int res, struct msg_receiver *msr, *t; list_for_each_entry_safe(msr, t, &msq->q_receivers, r_list) { - get_task_struct(msr->r_tsk); + struct task_struct *task = msr->r_tsk; + + get_task_struct(task); /* see MSG_BARRIER for purpose/pairing */ smp_store_release(&msr->r_msg, ERR_PTR(res)); - wake_q_add_safe(wake_q, msr->r_tsk); + wake_q_add_safe(wake_q, task); } } diff --git a/ipc/sem.c b/ipc/sem.c index e0ec239680cb..04700a823e79 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -784,12 +784,14 @@ static int perform_atomic_semop(struct sem_array *sma, struct sem_queue *q) static inline void wake_up_sem_queue_prepare(struct sem_queue *q, int error, struct wake_q_head *wake_q) { - get_task_struct(q->sleeper); + struct task_struct *task = q->sleeper; + + get_task_struct(task); /* see SEM_BARRIER_2 for purpose/pairing */ smp_store_release(&q->status, error); - wake_q_add_safe(wake_q, q->sleeper); + wake_q_add_safe(wake_q, task); } static void unlink_queue(struct sem_array *sma, struct sem_queue *q) -- 2.30.2