On Wed, 2015-11-25 at 18:24 +0000, Rainer Weikusat wrote:
> Eric Dumazet <eric.duma...@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
