lkml.org 
[lkml]   [2015]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: use-after-free in sock_wake_async
    From
    Date
    On Wed, 2015-11-25 at 18:24 +0000, Rainer Weikusat wrote:
    > Eric Dumazet <eric.dumazet@gmail.com> writes:
    > > On Wed, 2015-11-25 at 17:30 +0000, Rainer Weikusat wrote:
    > >
    > >> In case this is wrong, it obviously implies that sk_sleep(sk) must not
    > >> be used anywhere as it accesses the same struck sock, hence, when that
    > >> can "suddenly" disappear despite locks are used in the way indicated
    > >> above, there is now safe way to invoke that, either, as it just does a
    > >> rcu_dereference_raw based on the assumption that the caller knows that
    > >> the i-node (and the corresponding wait queue) still exist.
    > >>
    > >
    > > Oh well.
    > >
    > > sk_sleep() is not used if the return is NULL
    >
    > static long unix_stream_data_wait(struct sock *sk, long timeo,
    > struct sk_buff *last, unsigned int last_len)
    > {
    > struct sk_buff *tail;
    > DEFINE_WAIT(wait);
    >
    > unix_state_lock(sk);
    >
    > for (;;) {
    > prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
    >
    > tail = skb_peek_tail(&sk->sk_receive_queue);
    > if (tail != last ||
    > (tail && tail->len != last_len) ||
    > sk->sk_err ||
    > (sk->sk_shutdown & RCV_SHUTDOWN) ||
    > signal_pending(current) ||
    > !timeo)
    > break;
    >
    > set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
    > unix_state_unlock(sk);
    > timeo = freezable_schedule_timeout(timeo);
    > unix_state_lock(sk);
    >
    > if (sock_flag(sk, SOCK_DEAD))
    > break;
    >
    > clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
    > }
    >
    > finish_wait(sk_sleep(sk), &wait);
    > unix_state_unlock(sk);
    > return timeo;
    > }
    >
    > Neither prepare_to_wait nor finish_wait check if the pointer is
    > null. For the finish_wait case, it shouldn't be null because if
    > SOCK_DEAD is not found to be set after the unix_state_lock was acquired,
    > unix_release_sock didn't execute the corresponding code yet, hence,
    > inode etc will remain available until after the corresponding unlock.


    >
    > But this isn't true anymore if the inode can go away despite
    > sock_release couldn't complete yet.


    You are looking at the wrong side.

    Of course, the thread 'owning' a socket has a reference on it, so it
    knows sk->sk_socket and sk->sk_ww is not NULL.

    The problem is that at the time a wakeup is done, it can be done by a
    process or softirq having no ref on the 'struct socket', as
    sk->sk_socket can become NULL at anytime.

    This is why we have sk_wq , and RCU protection, so that we do not have
    to use expensive atomic operations in this fast path.






    \
     
     \ /
      Last update: 2015-11-25 20:01    [W:5.547 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site