Skip to content

Commit 43ae218

Browse files
committed
Merge branch 'mptcp-locking-fixes'
Mat Martineau says: ==================== mptcp: Locking fixes Two separate locking fixes for the networking tree: Patch 1 addresses a MPTCP fastopen error-path deadlock that was found with syzkaller. Patch 2 works around a lockdep false-positive between MPTCP listening and non-listening sockets at socket destruct time. ==================== Link: https://lore.kernel.org/r/20221220195215.238353-1-mathew.j.martineau@linux.intel.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents e20aa07 + fec3adf commit 43ae218

File tree

3 files changed

+35
-8
lines changed

3 files changed

+35
-8
lines changed

net/mptcp/protocol.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,6 +1662,8 @@ static void mptcp_set_nospace(struct sock *sk)
16621662
set_bit(MPTCP_NOSPACE, &mptcp_sk(sk)->flags);
16631663
}
16641664

1665+
static int mptcp_disconnect(struct sock *sk, int flags);
1666+
16651667
static int mptcp_sendmsg_fastopen(struct sock *sk, struct sock *ssk, struct msghdr *msg,
16661668
size_t len, int *copied_syn)
16671669
{
@@ -1672,9 +1674,9 @@ static int mptcp_sendmsg_fastopen(struct sock *sk, struct sock *ssk, struct msgh
16721674
lock_sock(ssk);
16731675
msg->msg_flags |= MSG_DONTWAIT;
16741676
msk->connect_flags = O_NONBLOCK;
1675-
msk->is_sendmsg = 1;
1677+
msk->fastopening = 1;
16761678
ret = tcp_sendmsg_fastopen(ssk, msg, copied_syn, len, NULL);
1677-
msk->is_sendmsg = 0;
1679+
msk->fastopening = 0;
16781680
msg->msg_flags = saved_flags;
16791681
release_sock(ssk);
16801682

@@ -1688,6 +1690,8 @@ static int mptcp_sendmsg_fastopen(struct sock *sk, struct sock *ssk, struct msgh
16881690
*/
16891691
if (ret && ret != -EINPROGRESS && ret != -ERESTARTSYS && ret != -EINTR)
16901692
*copied_syn = 0;
1693+
} else if (ret && ret != -EINPROGRESS) {
1694+
mptcp_disconnect(sk, 0);
16911695
}
16921696

16931697
return ret;
@@ -2353,7 +2357,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
23532357
/* otherwise tcp will dispose of the ssk and subflow ctx */
23542358
if (ssk->sk_state == TCP_LISTEN) {
23552359
tcp_set_state(ssk, TCP_CLOSE);
2356-
mptcp_subflow_queue_clean(ssk);
2360+
mptcp_subflow_queue_clean(sk, ssk);
23572361
inet_csk_listen_stop(ssk);
23582362
mptcp_event_pm_listener(ssk, MPTCP_EVENT_LISTENER_CLOSED);
23592363
}
@@ -2989,6 +2993,14 @@ static int mptcp_disconnect(struct sock *sk, int flags)
29892993
{
29902994
struct mptcp_sock *msk = mptcp_sk(sk);
29912995

2996+
/* We are on the fastopen error path. We can't call straight into the
2997+
* subflows cleanup code due to lock nesting (we are already under
2998+
* msk->firstsocket lock). Do nothing and leave the cleanup to the
2999+
* caller.
3000+
*/
3001+
if (msk->fastopening)
3002+
return 0;
3003+
29923004
inet_sk_state_store(sk, TCP_CLOSE);
29933005

29943006
mptcp_stop_timer(sk);
@@ -3532,7 +3544,7 @@ static int mptcp_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
35323544
/* if reaching here via the fastopen/sendmsg path, the caller already
35333545
* acquired the subflow socket lock, too.
35343546
*/
3535-
if (msk->is_sendmsg)
3547+
if (msk->fastopening)
35363548
err = __inet_stream_connect(ssock, uaddr, addr_len, msk->connect_flags, 1);
35373549
else
35383550
err = inet_stream_connect(ssock, uaddr, addr_len, msk->connect_flags);

net/mptcp/protocol.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ struct mptcp_sock {
295295
u8 recvmsg_inq:1,
296296
cork:1,
297297
nodelay:1,
298-
is_sendmsg:1;
298+
fastopening:1;
299299
int connect_flags;
300300
struct work_struct work;
301301
struct sk_buff *ooo_last_skb;
@@ -628,7 +628,7 @@ void mptcp_close_ssk(struct sock *sk, struct sock *ssk,
628628
struct mptcp_subflow_context *subflow);
629629
void __mptcp_subflow_send_ack(struct sock *ssk);
630630
void mptcp_subflow_reset(struct sock *ssk);
631-
void mptcp_subflow_queue_clean(struct sock *ssk);
631+
void mptcp_subflow_queue_clean(struct sock *sk, struct sock *ssk);
632632
void mptcp_sock_graft(struct sock *sk, struct socket *parent);
633633
struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk);
634634
bool __mptcp_close(struct sock *sk, long timeout);

net/mptcp/subflow.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,7 +1791,7 @@ static void subflow_state_change(struct sock *sk)
17911791
}
17921792
}
17931793

1794-
void mptcp_subflow_queue_clean(struct sock *listener_ssk)
1794+
void mptcp_subflow_queue_clean(struct sock *listener_sk, struct sock *listener_ssk)
17951795
{
17961796
struct request_sock_queue *queue = &inet_csk(listener_ssk)->icsk_accept_queue;
17971797
struct mptcp_sock *msk, *next, *head = NULL;
@@ -1840,8 +1840,23 @@ void mptcp_subflow_queue_clean(struct sock *listener_ssk)
18401840

18411841
do_cancel_work = __mptcp_close(sk, 0);
18421842
release_sock(sk);
1843-
if (do_cancel_work)
1843+
if (do_cancel_work) {
1844+
/* lockdep will report a false positive ABBA deadlock
1845+
* between cancel_work_sync and the listener socket.
1846+
* The involved locks belong to different sockets WRT
1847+
* the existing AB chain.
1848+
* Using a per socket key is problematic as key
1849+
* deregistration requires process context and must be
1850+
* performed at socket disposal time, in atomic
1851+
* context.
1852+
* Just tell lockdep to consider the listener socket
1853+
* released here.
1854+
*/
1855+
mutex_release(&listener_sk->sk_lock.dep_map, _RET_IP_);
18441856
mptcp_cancel_work(sk);
1857+
mutex_acquire(&listener_sk->sk_lock.dep_map,
1858+
SINGLE_DEPTH_NESTING, 0, _RET_IP_);
1859+
}
18451860
sock_put(sk);
18461861
}
18471862

0 commit comments

Comments
 (0)