Netdev List
 help / color / mirror / Atom feed
From: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
To: <netdev@vger.kernel.org>
Cc: jon.maloy@ericsson.com, tipc-discussion@lists.sourceforge.net
Subject: [PATCH net v1 3/6] tipc: fix connection refcount error
Date: Tue, 24 Jan 2017 13:00:45 +0100	[thread overview]
Message-ID: <1485259248-21812-4-git-send-email-parthasarathy.bhuvaragan@ericsson.com> (raw)
In-Reply-To: <1485259248-21812-1-git-send-email-parthasarathy.bhuvaragan@ericsson.com>

Until now, the generic server framework maintains the connection
id's per subscriber in server's conn_idr. At tipc_close_conn, we
remove the connection id from the server list, but the connection is
valid until we call the refcount cleanup. Hence we have a window
where the server allocates the same connection to an new subscriber
leading to inconsistent reference count. We have another refcount
warning we grab the refcount in tipc_conn_lookup() for connections
with flag with CF_CONNECTED not set. This usually occurs at shutdown
when the we stop the topology server and withdraw TIPC_CFG_SRV
publication thereby triggering a withdraw message to subscribers.

In this commit, we:
1. remove the connection from the server list at recount cleanup.
2. grab the refcount for a connection only if CF_CONNECTED is set.

Tested-by: John Thompson <thompa.atl@gmail.com>
Acked-by: Ying Xue <ying.xue@windriver.com>
Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
---
 net/tipc/server.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/net/tipc/server.c b/net/tipc/server.c
index 215849ce453d..2e803601aa99 100644
--- a/net/tipc/server.c
+++ b/net/tipc/server.c
@@ -91,7 +91,8 @@ static void tipc_sock_release(struct tipc_conn *con);
 static void tipc_conn_kref_release(struct kref *kref)
 {
 	struct tipc_conn *con = container_of(kref, struct tipc_conn, kref);
-	struct sockaddr_tipc *saddr = con->server->saddr;
+	struct tipc_server *s = con->server;
+	struct sockaddr_tipc *saddr = s->saddr;
 	struct socket *sock = con->sock;
 	struct sock *sk;
 
@@ -106,6 +107,11 @@ static void tipc_conn_kref_release(struct kref *kref)
 		tipc_sock_release(con);
 		sock_release(sock);
 		con->sock = NULL;
+
+		spin_lock_bh(&s->idr_lock);
+		idr_remove(&s->conn_idr, con->conid);
+		s->idr_in_use--;
+		spin_unlock_bh(&s->idr_lock);
 	}
 
 	tipc_clean_outqueues(con);
@@ -128,8 +134,10 @@ static struct tipc_conn *tipc_conn_lookup(struct tipc_server *s, int conid)
 
 	spin_lock_bh(&s->idr_lock);
 	con = idr_find(&s->conn_idr, conid);
-	if (con)
+	if (con && test_bit(CF_CONNECTED, &con->flags))
 		conn_get(con);
+	else
+		con = NULL;
 	spin_unlock_bh(&s->idr_lock);
 	return con;
 }
@@ -198,15 +206,8 @@ static void tipc_sock_release(struct tipc_conn *con)
 
 static void tipc_close_conn(struct tipc_conn *con)
 {
-	struct tipc_server *s = con->server;
-
 	if (test_and_clear_bit(CF_CONNECTED, &con->flags)) {
 
-		spin_lock_bh(&s->idr_lock);
-		idr_remove(&s->conn_idr, con->conid);
-		s->idr_in_use--;
-		spin_unlock_bh(&s->idr_lock);
-
 		/* We shouldn't flush pending works as we may be in the
 		 * thread. In fact the races with pending rx/tx work structs
 		 * are harmless for us here as we have already deleted this
-- 
2.1.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

  parent reply	other threads:[~2017-01-24 12:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-24 12:00 [PATCH net v1 0/6] topology server fixes for nametable soft lockup Parthasarathy Bhuvaragan
2017-01-24 12:00 ` [PATCH net v1 1/6] tipc: fix nametbl_lock soft lockup at node/link events Parthasarathy Bhuvaragan
2017-01-24 12:00 ` [PATCH net v1 2/6] tipc: add subscription refcount to avoid invalid delete Parthasarathy Bhuvaragan
2017-01-24 12:00 ` Parthasarathy Bhuvaragan [this message]
2017-01-24 12:00 ` [PATCH net v1 4/6] tipc: fix nametbl_lock soft lockup at module exit Parthasarathy Bhuvaragan
2017-01-24 12:00 ` [PATCH net v1 5/6] tipc: ignore requests when the connection state is not CONNECTED Parthasarathy Bhuvaragan
2017-01-24 12:00 ` [PATCH net v1 6/6] tipc: fix cleanup at module unload Parthasarathy Bhuvaragan
2017-01-24 21:16 ` [PATCH net v1 0/6] topology server fixes for nametable soft lockup David Miller

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=1485259248-21812-4-git-send-email-parthasarathy.bhuvaragan@ericsson.com \
    --to=parthasarathy.bhuvaragan@ericsson.com \
    --cc=jon.maloy@ericsson.com \
    --cc=netdev@vger.kernel.org \
    --cc=tipc-discussion@lists.sourceforge.net \
    /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