All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Ying Xu <yinxu@redhat.com>,
	Xin Long <lucien.xin@gmail.com>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 5.8 11/16] sctp: not disable bh in the whole sctp_get_port_local()
Date: Fri, 11 Sep 2020 14:47:28 +0200	[thread overview]
Message-ID: <20200911122500.139722863@linuxfoundation.org> (raw)
In-Reply-To: <20200911122459.585735377@linuxfoundation.org>

From: Xin Long <lucien.xin@gmail.com>

[ Upstream commit 3106ecb43a05dc3e009779764b9da245a5d082de ]

With disabling bh in the whole sctp_get_port_local(), when
snum == 0 and too many ports have been used, the do-while
loop will take the cpu for a long time and cause cpu stuck:

  [ ] watchdog: BUG: soft lockup - CPU#11 stuck for 22s!
  [ ] RIP: 0010:native_queued_spin_lock_slowpath+0x4de/0x940
  [ ] Call Trace:
  [ ]  _raw_spin_lock+0xc1/0xd0
  [ ]  sctp_get_port_local+0x527/0x650 [sctp]
  [ ]  sctp_do_bind+0x208/0x5e0 [sctp]
  [ ]  sctp_autobind+0x165/0x1e0 [sctp]
  [ ]  sctp_connect_new_asoc+0x355/0x480 [sctp]
  [ ]  __sctp_connect+0x360/0xb10 [sctp]

There's no need to disable bh in the whole function of
sctp_get_port_local. So fix this cpu stuck by removing
local_bh_disable() called at the beginning, and using
spin_lock_bh() instead.

The same thing was actually done for inet_csk_get_port() in
Commit ea8add2b1903 ("tcp/dccp: better use of ephemeral
ports in bind()").

Thanks to Marcelo for pointing the buggy code out.

v1->v2:
  - use cond_resched() to yield cpu to other tasks if needed,
    as Eric noticed.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Ying Xu <yinxu@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/sctp/socket.c |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -8297,8 +8297,6 @@ static int sctp_get_port_local(struct so
 
 	pr_debug("%s: begins, snum:%d\n", __func__, snum);
 
-	local_bh_disable();
-
 	if (snum == 0) {
 		/* Search for an available port. */
 		int low, high, remaining, index;
@@ -8316,20 +8314,21 @@ static int sctp_get_port_local(struct so
 				continue;
 			index = sctp_phashfn(net, rover);
 			head = &sctp_port_hashtable[index];
-			spin_lock(&head->lock);
+			spin_lock_bh(&head->lock);
 			sctp_for_each_hentry(pp, &head->chain)
 				if ((pp->port == rover) &&
 				    net_eq(net, pp->net))
 					goto next;
 			break;
 		next:
-			spin_unlock(&head->lock);
+			spin_unlock_bh(&head->lock);
+			cond_resched();
 		} while (--remaining > 0);
 
 		/* Exhausted local port range during search? */
 		ret = 1;
 		if (remaining <= 0)
-			goto fail;
+			return ret;
 
 		/* OK, here is the one we will use.  HEAD (the port
 		 * hash table list entry) is non-NULL and we hold it's
@@ -8344,7 +8343,7 @@ static int sctp_get_port_local(struct so
 		 * port iterator, pp being NULL.
 		 */
 		head = &sctp_port_hashtable[sctp_phashfn(net, snum)];
-		spin_lock(&head->lock);
+		spin_lock_bh(&head->lock);
 		sctp_for_each_hentry(pp, &head->chain) {
 			if ((pp->port == snum) && net_eq(pp->net, net))
 				goto pp_found;
@@ -8444,10 +8443,7 @@ success:
 	ret = 0;
 
 fail_unlock:
-	spin_unlock(&head->lock);
-
-fail:
-	local_bh_enable();
+	spin_unlock_bh(&head->lock);
 	return ret;
 }
 



  parent reply	other threads:[~2020-09-11 15:20 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-11 12:47 [PATCH 5.8 00/16] 5.8.9-rc1 review Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 01/16] io_uring: fix cancel of deferred reqs with ->files Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 02/16] io_uring: fix linked deferred ->files cancellation Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 03/16] RDMA/cma: Simplify DEVICE_REMOVAL for internal_id Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 04/16] RDMA/cma: Using the standard locking pattern when delivering the removal event Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 05/16] RDMA/cma: Remove unneeded locking for req paths Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 06/16] RDMA/cma: Execute rdma_cm destruction from a handler properly Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 07/16] ipv4: Silence suspicious RCU usage warning Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 08/16] ipv6: Fix sysctl max for fib_multipath_hash_policy Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 09/16] netlabel: fix problems with mapping removal Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 10/16] net: usb: dm9601: Add USB ID of Keenetic Plus DSL Greg Kroah-Hartman
2020-09-11 12:47 ` Greg Kroah-Hartman [this message]
2020-09-11 12:47 ` [PATCH 5.8 12/16] taprio: Fix using wrong queues in gate mask Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 13/16] tipc: fix shutdown() of connectionless socket Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 14/16] tipc: fix using smp_processor_id() in preemptible Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 15/16] net: disable netpoll on fresh napis Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 16/16] mptcp: free acked data before waiting for more memory Greg Kroah-Hartman
2020-09-11 17:10 ` [PATCH 5.8 00/16] 5.8.9-rc1 review Jon Hunter
2020-09-12 12:44   ` Greg Kroah-Hartman
2020-09-11 22:19 ` Shuah Khan
2020-09-12 12:44   ` Greg Kroah-Hartman
2020-09-12  2:19 ` Guenter Roeck
2020-09-12 12:44   ` Greg Kroah-Hartman
2020-09-12  7:27 ` Naresh Kamboju
2020-09-12 12:44   ` Greg Kroah-Hartman

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=20200911122500.139722863@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=marcelo.leitner@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=yinxu@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.