lkml.org 
[lkml]   [2021]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.12 090/127] ipc/mqueue, msg, sem: avoid relying on a stack reference past its expiry
    Date
    From: Varad Gautam <varad.gautam@suse.com>

    commit a11ddb37bf367e6b5239b95ca759e5389bb46048 upstream.

    do_mq_timedreceive calls wq_sleep with a stack local address. The
    sender (do_mq_timedsend) uses this address to later call pipelined_send.

    This leads to a very hard to trigger race where a do_mq_timedreceive
    call might return and leave do_mq_timedsend to rely on an invalid
    address, causing the following crash:

    RIP: 0010:wake_q_add_safe+0x13/0x60
    Call Trace:
    __x64_sys_mq_timedsend+0x2a9/0x490
    do_syscall_64+0x80/0x680
    entry_SYSCALL_64_after_hwframe+0x44/0xa9
    RIP: 0033:0x7f5928e40343

    The race occurs as:

    1. do_mq_timedreceive calls wq_sleep with the address of `struct
    ext_wait_queue` on function stack (aliased as `ewq_addr` here) - it
    holds a valid `struct ext_wait_queue *` as long as the stack has not
    been overwritten.

    2. `ewq_addr` gets added to info->e_wait_q[RECV].list in wq_add, and
    do_mq_timedsend receives it via wq_get_first_waiter(info, RECV) to call
    __pipelined_op.

    3. Sender calls __pipelined_op::smp_store_release(&this->state,
    STATE_READY). Here is where the race window begins. (`this` is
    `ewq_addr`.)

    4. If the receiver wakes up now in do_mq_timedreceive::wq_sleep, it
    will see `state == STATE_READY` and break.

    5. do_mq_timedreceive returns, and `ewq_addr` is no longer guaranteed
    to be a `struct ext_wait_queue *` since it was on do_mq_timedreceive's
    stack. (Although the address may not get overwritten until another
    function happens to touch it, which means it can persist around for an
    indefinite time.)

    6. do_mq_timedsend::__pipelined_op() still believes `ewq_addr` is a
    `struct ext_wait_queue *`, and uses it to find a task_struct to pass to
    the wake_q_add_safe call. In the lucky case where nothing has
    overwritten `ewq_addr` yet, `ewq_addr->task` is the right task_struct.
    In the unlucky case, __pipelined_op::wake_q_add_safe gets handed a
    bogus address as the receiver's task_struct causing the crash.

    do_mq_timedsend::__pipelined_op() should not dereference `this` after
    setting STATE_READY, as the receiver counterpart is now free to return.
    Change __pipelined_op to call wake_q_add_safe on the receiver's
    task_struct returned by get_task_struct, instead of dereferencing `this`
    which sits on the receiver's stack.

    As Manfred pointed out, the race potentially also exists in
    ipc/msg.c::expunge_all and ipc/sem.c::wake_up_sem_queue_prepare. Fix
    those in the same way.

    Link: https://lkml.kernel.org/r/20210510102950.12551-1-varad.gautam@suse.com
    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")
    Signed-off-by: Varad Gautam <varad.gautam@suse.com>
    Reported-by: Matthias von Faber <matthias.vonfaber@aox-tech.de>
    Acked-by: Davidlohr Bueso <dbueso@suse.de>
    Acked-by: Manfred Spraul <manfred@colorfullife.com>
    Cc: Christian Brauner <christian.brauner@ubuntu.com>
    Cc: Oleg Nesterov <oleg@redhat.com>
    Cc: "Eric W. Biederman" <ebiederm@xmission.com>
    Cc: <stable@vger.kernel.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    ipc/mqueue.c | 6 ++++--
    ipc/msg.c | 6 ++++--
    ipc/sem.c | 6 ++++--
    3 files changed, 12 insertions(+), 6 deletions(-)

    --- a/ipc/mqueue.c
    +++ b/ipc/mqueue.c
    @@ -1004,12 +1004,14 @@ static inline void __pipelined_op(struct
    struct mqueue_inode_info *info,
    struct ext_wait_queue *this)
    {
    + struct task_struct *task;
    +
    list_del(&this->list);
    - get_task_struct(this->task);
    + task = get_task_struct(this->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
    --- a/ipc/msg.c
    +++ b/ipc/msg.c
    @@ -251,11 +251,13 @@ static void expunge_all(struct msg_queue
    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 *r_tsk;
    +
    + r_tsk = get_task_struct(msr->r_tsk);

    /* 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, r_tsk);
    }
    }

    --- a/ipc/sem.c
    +++ b/ipc/sem.c
    @@ -784,12 +784,14 @@ would_block:
    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 *sleeper;
    +
    + sleeper = get_task_struct(q->sleeper);

    /* see SEM_BARRIER_2 for purpuse/pairing */
    smp_store_release(&q->status, error);

    - wake_q_add_safe(wake_q, q->sleeper);
    + wake_q_add_safe(wake_q, sleeper);
    }

    static void unlink_queue(struct sem_array *sma, struct sem_queue *q)

    \
     
     \ /
      Last update: 2021-05-24 18:07    [W:2.409 / U:0.484 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site