sctp: return error if the asoc has been peeled off in sctp_wait_for_sndbuf

commit a0ff660058b88d12625a783ce9e5c1371c87951f upstream.

After commit cea0cc80a677 ("sctp: use the right sk after waking up from
wait_buf sleep"), it may change to lock another sk if the asoc has been
peeled off in sctp_wait_for_sndbuf.

However, the asoc's new sk could be already closed elsewhere, as it's in
the sendmsg context of the old sk that can't avoid the new sk's closing.
If the sk's last one refcnt is held by this asoc, later on after putting
this asoc, the new sk will be freed, while under it's own lock.

This patch is to revert that commit, but fix the old issue by returning
error under the old sk's lock.

Fixes: cea0cc80a677 ("sctp: use the right sk after waking up from wait_buf sleep")
Reported-by: syzbot+ac6ea7baa4432811eb50@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index f4477d7..8a6aea5 100644 (file)
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -94,7 +94,7 @@
 static int sctp_writeable(struct sock *sk);
 static void sctp_wfree(struct sk_buff *skb);
 static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
 static int sctp_writeable(struct sock *sk);
 static void sctp_wfree(struct sk_buff *skb);
 static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,

-                               size_t msg_len, struct sock **orig_sk);
+                               size_t msg_len);

 static int sctp_wait_for_packet(struct sock * sk, int *err, long *timeo_p);
 static int sctp_wait_for_connect(struct sctp_association *, long *timeo_p);
 static int sctp_wait_for_accept(struct sock *sk, long timeo);
 static int sctp_wait_for_packet(struct sock * sk, int *err, long *timeo_p);
 static int sctp_wait_for_connect(struct sctp_association *, long *timeo_p);
 static int sctp_wait_for_accept(struct sock *sk, long timeo);


@@ -1912,7 +1912,7 @@ SCTP_STATIC int sctp_sendmsg(struct kiocb *iocb, struct sock *sk,
        timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
        if (!sctp_wspace(asoc)) {
                /* sk can be changed by peel off when waiting for buf. */
        timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
        if (!sctp_wspace(asoc)) {
                /* sk can be changed by peel off when waiting for buf. */

-               err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len, &sk);
+               err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len);

                if (err)
                        goto out_free;
        }
                if (err)
                        goto out_free;
        }


@@ -6465,12 +6465,12 @@ void sctp_sock_rfree(struct sk_buff *skb)
 
 /* Helper function to wait for space in the sndbuf.  */
 static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
 
 /* Helper function to wait for space in the sndbuf.  */
 static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,

-                               size_t msg_len, struct sock **orig_sk)
+                               size_t msg_len)

 {
        struct sock *sk = asoc->base.sk;
 {
        struct sock *sk = asoc->base.sk;

-       int err = 0;

        long current_timeo = *timeo_p;
        DEFINE_WAIT(wait);
        long current_timeo = *timeo_p;
        DEFINE_WAIT(wait);

+       int err = 0;

 
        SCTP_DEBUG_PRINTK("wait_for_sndbuf: asoc=%p, timeo=%ld, msg_len=%zu\n",
                          asoc, (long)(*timeo_p), msg_len);
 
        SCTP_DEBUG_PRINTK("wait_for_sndbuf: asoc=%p, timeo=%ld, msg_len=%zu\n",
                          asoc, (long)(*timeo_p), msg_len);


@@ -6498,17 +6498,13 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
                sctp_release_sock(sk);
                current_timeo = schedule_timeout(current_timeo);
                sctp_lock_sock(sk);
                sctp_release_sock(sk);
                current_timeo = schedule_timeout(current_timeo);
                sctp_lock_sock(sk);

-               if (sk != asoc->base.sk) {
-                       release_sock(sk);
-                       sk = asoc->base.sk;
-                       lock_sock(sk);
-               }
+               if (sk != asoc->base.sk)
+                       goto do_error;

 
                *timeo_p = current_timeo;
        }
 
 out:
 
                *timeo_p = current_timeo;
        }
 
 out:

-       *orig_sk = sk;

        finish_wait(&asoc->wait, &wait);
 
        /* Release the association's refcnt.  */
        finish_wait(&asoc->wait, &wait);
 
        /* Release the association's refcnt.  */