Re: [PATCH net-next] net: bail on sock_wfree, sock_rfree when we have a TCP_TIMEWAIT sk

On Thu, Jul 2, 2015 at 1:18 AM, Alex Gartrell [off-list ref] wrote:

On Wednesday, July 1, 2015, Eric Dumazet [off-list ref] wrote:

quoted

On Wed, Jul 1, 2015 at 11:14 PM, David Miller [off-list ref] wrote:

quoted

From: Alex Gartrell <redacted>
Date: Wed, 1 Jul 2015 13:13:09 -0700

quoted

If we early-demux bind a TCP_TIMEWAIT socket to an skb and then orphan
it
(as we need to do in the ipvs forwarding case), sock_wfree and
sock_rfree
are going to reach into the inet_timewait_sock and mess with fields
that
don't exist.

Signed-off-by: Alex Gartrell <redacted>

If we're forwarding, we should not find a local socket, period.

A socket cannot change state to TCP_TIMEWAIT.

A new object is allocated and old one is removed from ehash, then
freed (rcu rules being applied)

Also sock_wfree() has nothing to do with early demux. It is for output
path skbs only.

Alright I kind of cheated and didn't include full context here. The problem
is that within ipvs we are getting  packets that have been early demuxed and
associated with time wait sockets which we then wish to forward immediately
(ip_vs_xmit.c).  Under normal circumstances it would never be associated
with any sk at all, but it is because of early demux, so we want to drop the
relationship by calling skb_orphan.  This invokes the destructor which lands
us there.

So that is how we reach this illegal "treating a twsk like an sk" state.

If there is a better way to drop the association than skb_orphan I will use
it.

I think you are mistaken Alex.

socket early demux cannot possibly set skb->destructor to sock_rfree()

If skb->destructor is set by early demux, it correctly points to sock_edemux()

And this one correctly handles all socket variants.

/* All sockets share common refcount, but have different destructors */
void sock_gen_put(struct sock *sk)
{
        if (!atomic_dec_and_test(&sk->sk_refcnt))
                return;

        if (sk->sk_state == TCP_TIME_WAIT)
                inet_twsk_free(inet_twsk(sk));
        else if (sk->sk_state == TCP_NEW_SYN_RECV)
                reqsk_free(inet_reqsk(sk));
        else
                sk_free(sk);
}
EXPORT_SYMBOL_GPL(sock_gen_put);

void sock_edemux(struct sk_buff *skb)
{
        sock_gen_put(skb->sk);
}
EXPORT_SYMBOL(sock_edemux);