Commit e154659b authored by Florian Westphal's avatar Florian Westphal Committed by David S. Miller

mptcp: fix double-unlock in mptcp_poll

mptcp_connect/28740 is trying to release lock (sk_lock-AF_INET) at:
[<ffffffff82c15869>] mptcp_poll+0xb9/0x550
but there are no more locks to release!
Call Trace:
 lock_release+0x50f/0x750
 release_sock+0x171/0x1b0
 mptcp_poll+0xb9/0x550
 sock_poll+0x157/0x470
 ? get_net_ns+0xb0/0xb0
 do_sys_poll+0x63c/0xdd0

Problem is that __mptcp_tcp_fallback() releases the mptcp socket lock,
but after recent change it doesn't do this in all of its return paths.

To fix this, remove the unlock from __mptcp_tcp_fallback() and
always do the unlock in the caller.

Also add a small comment as to why we have this
__mptcp_needs_tcp_fallback().

Fixes: 0b4f33de ("mptcp: fix tcp fallback crash")
Reported-by: syzbot+e56606435b7bfeea8cf5@syzkaller.appspotmail.com
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 3fe260e0
...@@ -97,12 +97,7 @@ static struct socket *__mptcp_tcp_fallback(struct mptcp_sock *msk) ...@@ -97,12 +97,7 @@ static struct socket *__mptcp_tcp_fallback(struct mptcp_sock *msk)
if (likely(!__mptcp_needs_tcp_fallback(msk))) if (likely(!__mptcp_needs_tcp_fallback(msk)))
return NULL; return NULL;
if (msk->subflow) {
release_sock((struct sock *)msk);
return msk->subflow; return msk->subflow;
}
return NULL;
} }
static bool __mptcp_can_create_subflow(const struct mptcp_sock *msk) static bool __mptcp_can_create_subflow(const struct mptcp_sock *msk)
...@@ -734,9 +729,10 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) ...@@ -734,9 +729,10 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
goto out; goto out;
} }
fallback:
ssock = __mptcp_tcp_fallback(msk); ssock = __mptcp_tcp_fallback(msk);
if (unlikely(ssock)) { if (unlikely(ssock)) {
fallback: release_sock(sk);
pr_debug("fallback passthrough"); pr_debug("fallback passthrough");
ret = sock_sendmsg(ssock, msg); ret = sock_sendmsg(ssock, msg);
return ret >= 0 ? ret + copied : (copied ? copied : ret); return ret >= 0 ? ret + copied : (copied ? copied : ret);
...@@ -769,8 +765,14 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) ...@@ -769,8 +765,14 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
if (ret < 0) if (ret < 0)
break; break;
if (ret == 0 && unlikely(__mptcp_needs_tcp_fallback(msk))) { if (ret == 0 && unlikely(__mptcp_needs_tcp_fallback(msk))) {
/* Can happen for passive sockets:
* 3WHS negotiated MPTCP, but first packet after is
* plain TCP (e.g. due to middlebox filtering unknown
* options).
*
* Fall back to TCP.
*/
release_sock(ssk); release_sock(ssk);
ssock = __mptcp_tcp_fallback(msk);
goto fallback; goto fallback;
} }
...@@ -883,6 +885,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, ...@@ -883,6 +885,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
ssock = __mptcp_tcp_fallback(msk); ssock = __mptcp_tcp_fallback(msk);
if (unlikely(ssock)) { if (unlikely(ssock)) {
fallback: fallback:
release_sock(sk);
pr_debug("fallback-read subflow=%p", pr_debug("fallback-read subflow=%p",
mptcp_subflow_ctx(ssock->sk)); mptcp_subflow_ctx(ssock->sk));
copied = sock_recvmsg(ssock, msg, flags); copied = sock_recvmsg(ssock, msg, flags);
...@@ -1467,12 +1470,11 @@ static int mptcp_setsockopt(struct sock *sk, int level, int optname, ...@@ -1467,12 +1470,11 @@ static int mptcp_setsockopt(struct sock *sk, int level, int optname,
*/ */
lock_sock(sk); lock_sock(sk);
ssock = __mptcp_tcp_fallback(msk); ssock = __mptcp_tcp_fallback(msk);
release_sock(sk);
if (ssock) if (ssock)
return tcp_setsockopt(ssock->sk, level, optname, optval, return tcp_setsockopt(ssock->sk, level, optname, optval,
optlen); optlen);
release_sock(sk);
return -EOPNOTSUPP; return -EOPNOTSUPP;
} }
...@@ -1492,12 +1494,11 @@ static int mptcp_getsockopt(struct sock *sk, int level, int optname, ...@@ -1492,12 +1494,11 @@ static int mptcp_getsockopt(struct sock *sk, int level, int optname,
*/ */
lock_sock(sk); lock_sock(sk);
ssock = __mptcp_tcp_fallback(msk); ssock = __mptcp_tcp_fallback(msk);
release_sock(sk);
if (ssock) if (ssock)
return tcp_getsockopt(ssock->sk, level, optname, optval, return tcp_getsockopt(ssock->sk, level, optname, optval,
option); option);
release_sock(sk);
return -EOPNOTSUPP; return -EOPNOTSUPP;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment