Skip to content

Commit cea0cc8

Browse files
lxindavem330
authored andcommitted
sctp: use the right sk after waking up from wait_buf sleep
Commit dfcb9f4 ("sctp: deny peeloff operation on asocs with threads sleeping on it") fixed the race between peeloff and wait sndbuf by checking waitqueue_active(&asoc->wait) in sctp_do_peeloff(). But it actually doesn't work, as even if waitqueue_active returns false the waiting sndbuf thread may still not yet hold sk lock. After asoc is peeled off, sk is not asoc->base.sk any more, then to hold the old sk lock couldn't make assoc safe to access. This patch is to fix this by changing to hold the new sk lock if sk is not asoc->base.sk, meanwhile, also set the sk in sctp_sendmsg with the new sk. With this fix, there is no more race between peeloff and waitbuf, the check 'waitqueue_active' in sctp_do_peeloff can be removed. Thanks Marcelo and Neil for making this clear. v1->v2: fix it by changing to lock the new sock instead of adding a flag in asoc. Suggested-by: Neil Horman <[email protected]> Signed-off-by: Xin Long <[email protected]> Acked-by: Neil Horman <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent ca3af4d commit cea0cc8

File tree

1 file changed

+11
-10
lines changed

1 file changed

+11
-10
lines changed

net/sctp/socket.c

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@
8484
/* Forward declarations for internal helper functions. */
8585
static int sctp_writeable(struct sock *sk);
8686
static void sctp_wfree(struct sk_buff *skb);
87-
static int sctp_wait_for_sndbuf(struct sctp_association *, long *timeo_p,
88-
size_t msg_len);
87+
static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
88+
size_t msg_len, struct sock **orig_sk);
8989
static int sctp_wait_for_packet(struct sock *sk, int *err, long *timeo_p);
9090
static int sctp_wait_for_connect(struct sctp_association *, long *timeo_p);
9191
static int sctp_wait_for_accept(struct sock *sk, long timeo);
@@ -1970,7 +1970,8 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
19701970

19711971
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
19721972
if (!sctp_wspace(asoc)) {
1973-
err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len);
1973+
/* sk can be changed by peel off when waiting for buf. */
1974+
err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len, &sk);
19741975
if (err) {
19751976
if (err == -ESRCH) {
19761977
/* asoc is already dead. */
@@ -5021,12 +5022,6 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
50215022
if (!asoc)
50225023
return -EINVAL;
50235024

5024-
/* If there is a thread waiting on more sndbuf space for
5025-
* sending on this asoc, it cannot be peeled.
5026-
*/
5027-
if (waitqueue_active(&asoc->wait))
5028-
return -EBUSY;
5029-
50305025
/* An association cannot be branched off from an already peeled-off
50315026
* socket, nor is this supported for tcp style sockets.
50325027
*/
@@ -7995,7 +7990,7 @@ void sctp_sock_rfree(struct sk_buff *skb)
79957990

79967991
/* Helper function to wait for space in the sndbuf. */
79977992
static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
7998-
size_t msg_len)
7993+
size_t msg_len, struct sock **orig_sk)
79997994
{
80007995
struct sock *sk = asoc->base.sk;
80017996
int err = 0;
@@ -8029,11 +8024,17 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
80298024
release_sock(sk);
80308025
current_timeo = schedule_timeout(current_timeo);
80318026
lock_sock(sk);
8027+
if (sk != asoc->base.sk) {
8028+
release_sock(sk);
8029+
sk = asoc->base.sk;
8030+
lock_sock(sk);
8031+
}
80328032

80338033
*timeo_p = current_timeo;
80348034
}
80358035

80368036
out:
8037+
*orig_sk = sk;
80378038
finish_wait(&asoc->wait, &wait);
80388039

80398040
/* Release the association's refcnt. */

0 commit comments

Comments
 (0)