Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

Commit

Permalink
mptcp: Don't sock_put too early if we still access the meta
Browse files Browse the repository at this point in the history
In the error code-paths the meta is supposed to get free'd. Problem is:
we do a bh_unlock_sock() after the last sock_put().

(that last sock-put happens in the error code-path when going into
inet_csk_destroy_sock() coming from inet_child_forget())

The reason this is the last sock_put() is because we sock_put already in
__mptcp_check_req_master. Thus, this is error-prone.

Instread of having __mptcp_check_req_master do the last sock_put, let's
let the callers do it. That avoid a bh_unlock_sock() on a free'd socket.

Fixes: Zero-day bug
Signed-off-by: Christoph Paasch <cpaasch@apple.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
(cherry picked from commit 7b0343d)
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
(cherry picked from commit 99ec1d7)
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
  • Loading branch information
cpaasch authored and matttbe committed Aug 31, 2022
1 parent 10572d4 commit d616ea5
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 10 deletions.
6 changes: 5 additions & 1 deletion net/ipv4/tcp_input.c
Original file line number Diff line number Diff line change
Expand Up @@ -6771,14 +6771,18 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
* by the failed call to inet_csk_reqsk_queue_add
*/
bh_unlock_sock(meta_sk);
if (meta_sk != fastopen_sk)
sock_put(meta_sk);
sock_put(fastopen_sk);
reqsk_put(req);
goto drop;
}
sk->sk_data_ready(sk);
bh_unlock_sock(fastopen_sk);
if (meta_sk != fastopen_sk)
if (meta_sk != fastopen_sk) {
bh_unlock_sock(meta_sk);
sock_put(meta_sk);
}
sock_put(fastopen_sk);
} else {
tcp_rsk(req)->tfo_listener = false;
Expand Down
13 changes: 4 additions & 9 deletions net/mptcp/mptcp_ctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2159,9 +2159,6 @@ static int __mptcp_check_req_master(struct sock *child,
*/
mptcp_reqsk_remove_tk(req);

/* Hold when creating the meta-sk in tcp_vX_syn_recv_sock. */
sock_put(meta_sk);

return 0;
}

Expand Down Expand Up @@ -2237,9 +2234,7 @@ int mptcp_check_req_master(struct sock *sk, struct sock *child,
reqsk_queue_removed(&inet_csk(sk)->icsk_accept_queue, req);
if (!inet_csk_reqsk_queue_add(sk, req, meta_sk)) {
bh_unlock_sock(meta_sk);
/* No sock_put() of the meta needed. The reference has
* already been dropped in __mptcp_check_req_master().
*/
sock_put(meta_sk);
sock_put(child);
return -1;
}
Expand All @@ -2249,15 +2244,15 @@ int mptcp_check_req_master(struct sock *sk, struct sock *child,
tcp_sk(meta_sk)->tsoffset = tsoff;
if (!inet_csk_reqsk_queue_add(sk, req, meta_sk)) {
bh_unlock_sock(meta_sk);
/* No sock_put() of the meta needed. The reference has
* already been dropped in __mptcp_check_req_master().
*/
sock_put(meta_sk);
sock_put(child);
reqsk_put(req);
return -1;
}
}

sock_put(meta_sk);

return 0;
}

Expand Down

0 comments on commit d616ea5

Please sign in to comment.