Skip to content

Commit

Permalink
mptcp: fix splat when incoming connection is never accepted before ex…
Browse files Browse the repository at this point in the history
…it/close

Following tool (replicated from syzkaller generated code) generates
warning: "IPv4: Attempt to release TCP socket in state 1".

int main(void) {
        struct sockaddr_in sin1 = { .sin_family = 2, .sin_port = 0x4e20, .sin_addr.s_addr = 0x010000e0, };
        struct sockaddr_in sin2 = { .sin_family = 2, .sin_port = 0x0, .sin_addr.s_addr = 0x0100007f, };
        struct sockaddr_in sin3 = { .sin_family = 2, .sin_port = 0x4e20, .sin_addr.s_addr = 0x0100007f, };
        int r0 = socket(0x2, 0x1, 0x106);
        int r1 = socket(0x2, 0x1, 0x106);

        bind(r1, (void *)&sin1, sizeof(sin1));
        connect(r1, (void *)&sin2, sizeof(sin2));
        listen(r1, 3);
        return connect(r0, (void *)&sin3, 0x4d);
}

Reason is that the newly generated mptcp socket is closed via
the ulp release of the tcp socket, via the listeners accept backlog
purge.
To fix this, delay setting the ESTABLISHED state until after
userspace calls accept and via mptcp specific destructor.

Fixes: 58b0991 ("mptcp: create msk early")
Closes: multipath-tcp/mptcp_net-next#9
Signed-off-by: Florian Westphal <fw@strlen.de>
  • Loading branch information
Florian Westphal authored and Paolo Abeni committed Apr 17, 2020
1 parent 33abf4b commit 1392703
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
1 change: 1 addition & 0 deletions net/mptcp/protocol.c
Original file line number Diff line number Diff line change
Expand Up @@ -1444,6 +1444,7 @@ static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
newsk = new_mptcp_sock;
mptcp_copy_inaddrs(newsk, ssk);
list_add(&subflow->node, &msk->conn_list);
inet_sk_state_store(newsk, TCP_ESTABLISHED);

bh_unlock_sock(new_mptcp_sock);

Expand Down
25 changes: 24 additions & 1 deletion net/mptcp/subflow.c
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,29 @@ static bool subflow_hmac_valid(const struct request_sock *req,
return ret;
}

static void mptcp_sock_destruct(struct sock *sk)
{
/* if new mptcp socket isn't accepted, it is free'd
* from the tcp listener sockets request queue, linked
* from req->sk. The tcp socket is released.
* This calls the ULP release function which will
* also remove the mptcp socket, via
* sock_put(ctx->conn).
*
* Problem is that the mptcp socket will not be in
* SYN_RECV state and doesn't have SOCK_DEAD flag.
* Both result in warnings from inet_sock_destruct.
*/

if (sk->sk_state == TCP_SYN_RECV) {
sk->sk_state = TCP_CLOSE;
WARN_ON_ONCE(sk->sk_socket);
sock_orphan(sk);
}

inet_sock_destruct(sk);
}

static struct sock *subflow_syn_recv_sock(const struct sock *sk,
struct sk_buff *skb,
struct request_sock *req,
Expand Down Expand Up @@ -422,7 +445,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
/* new mpc subflow takes ownership of the newly
* created mptcp socket
*/
inet_sk_state_store(new_msk, TCP_ESTABLISHED);
new_msk->sk_destruct = mptcp_sock_destruct;
mptcp_pm_new_connection(mptcp_sk(new_msk), 1);
ctx->conn = new_msk;
new_msk = NULL;
Expand Down

0 comments on commit 1392703

Please sign in to comment.