From: alexander.levin@verizon.com
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"stable@vger.kernel.org" <stable@vger.kernel.org>
Cc: Xin Long <lucien.xin@gmail.com>,
"David S . Miller" <davem@davemloft.net>,
alexander.levin@verizon.com
Subject: [PATCH AUTOSEL for 4.4 12/20] sctp: use the right sk after waking up from wait_buf sleep
Date: Sat, 2 Dec 2017 15:52:58 +0000 [thread overview]
Message-ID: <20171202155243.7774-12-alexander.levin@verizon.com> (raw)
In-Reply-To: <20171202155243.7774-1-alexander.levin@verizon.com>
From: Xin Long <lucien.xin@gmail.com>
[ Upstream commit cea0cc80a6777beb6eb643d4ad53690e1ad1d4ff ]
Commit dfcb9f4f99f1 ("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 <nhorman@tuxdriver.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>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
net/sctp/socket.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 0bc9615bf878..61189c576963 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -82,8 +82,8 @@
/* Forward declarations for internal helper functions. */
static int sctp_writeable(struct sock *sk);
static void sctp_wfree(struct sk_buff *skb);
-static int sctp_wait_for_sndbuf(struct sctp_association *, long *timeo_p,
- size_t msg_len);
+static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
+ size_t msg_len, struct sock **orig_sk);
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);
@@ -1953,7 +1953,8 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
if (!sctp_wspace(asoc)) {
- err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len);
+ /* sk can be changed by peel off when waiting for buf. */
+ err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len, &sk);
if (err) {
if (err == -ESRCH) {
/* asoc is already dead. */
@@ -4466,12 +4467,6 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
if (!asoc)
return -EINVAL;
- /* If there is a thread waiting on more sndbuf space for
- * sending on this asoc, it cannot be peeled.
- */
- if (waitqueue_active(&asoc->wait))
- return -EBUSY;
-
/* An association cannot be branched off from an already peeled-off
* socket, nor is this supported for tcp style sockets.
*/
@@ -6981,7 +6976,7 @@ 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,
- size_t msg_len)
+ size_t msg_len, struct sock **orig_sk)
{
struct sock *sk = asoc->base.sk;
int err = 0;
@@ -7015,11 +7010,17 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
release_sock(sk);
current_timeo = schedule_timeout(current_timeo);
lock_sock(sk);
+ if (sk != asoc->base.sk) {
+ release_sock(sk);
+ sk = asoc->base.sk;
+ lock_sock(sk);
+ }
*timeo_p = current_timeo;
}
out:
+ *orig_sk = sk;
finish_wait(&asoc->wait, &wait);
/* Release the association's refcnt. */
--
2.11.0
next prev parent reply other threads:[~2017-12-02 15:54 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-02 15:52 [PATCH AUTOSEL for 4.4 01/20] kbuild: pkg: use --transform option to prefix paths in tar alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 02/20] mac80211_hwsim: Fix memory leak in hwsim_new_radio_nl() alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 03/20] route: also update fnhe_genid when updating a route cache alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 04/20] route: update fnhe_expires for redirect when the fnhe exists alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 05/20] lib/genalloc.c: make the avail variable an atomic_long_t alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 06/20] dynamic-debug-howto: fix optional/omitted ending line number to be LARGE instead of 0 alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 07/20] NFS: Fix a typo in nfs_rename() alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 08/20] sunrpc: Fix rpc_task_begin trace point alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 10/20] sparc64/mm: set fields in deferred pages alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 09/20] block: wake up all tasks blocked in get_request() alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 11/20] sctp: do not free asoc when it is already dead in sctp_sendmsg alexander.levin
2017-12-02 15:52 ` alexander.levin [this message]
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 15/20] xfrm: Copy policy family in clone_policy alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 13/20] atm: horizon: Fix irq release error alexander.levin
2017-12-02 15:52 ` [PATCH AUTOSEL for 4.4 14/20] jump_label: Invoke jump_label_test() via early_initcall() alexander.levin
2017-12-02 15:53 ` [PATCH AUTOSEL for 4.4 16/20] IB/mlx4: Increase maximal message size under UD QP alexander.levin
2017-12-02 15:53 ` [PATCH AUTOSEL for 4.4 19/20] ipvlan: fix ipv6 outbound device alexander.levin
2017-12-02 15:53 ` [PATCH AUTOSEL for 4.4 18/20] afs: Connect up the CB.ProbeUuid alexander.levin
2017-12-02 15:53 ` [PATCH AUTOSEL for 4.4 17/20] IB/mlx5: Assign send CQ and recv CQ of UMR QP alexander.levin
2017-12-02 15:53 ` [PATCH AUTOSEL for 4.4 20/20] audit: ensure that 'audit=1' actually enables audit for PID 1 alexander.levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171202155243.7774-12-alexander.levin@verizon.com \
--to=alexander.levin@verizon.com \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=lucien.xin@gmail.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®