Skip to contentSkip to conversation

sctp: handle association restarts when the socket is closed.

PATCH Applied Vladislav Yasevich 3 participants 5 messages Oct 3, 2014 – Oct 6, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sctp: handle association restarts when the socket is closed.

From: Vlad Yasevich <[email protected]>

Currently association restarts do not take into consideration the state of the socket. When a restart happens, the current assocation simply transitions into established state. This creates a condition where a remote system, through a the restart procedure, may create a local association that is no way reachable by user. The conditions to trigger this are as follows:
1) Remote does not acknoledge some data causing data to remain outstanding.
2) Local application calls close() on the socket. Since data is still outstanding, the association is placed in SHUTDOWN_PENDING state. However, the socket is closed.
3) The remote tries to create a new association, triggering a restart on the local system. The association moves from SHUTDOWN_PENDING to ESTABLISHED. At this point, it is no longer reachable by any socket on the local system.

This patch addresses the above situation by moving the newly ESTABLISHED association into SHUTDOWN-SENT state and bundling a SHUTDOWN after the COOKIE-ACK chunk. This way, the restarted associate immidiately enters the shutdown procedure and forces the termination of the unreachable association.

---
 include/net/sctp/command.h |  2 +-
 net/sctp/sm_statefuns.c    | 19 ++++++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)
include/net/sctp/command.h +1 -1
index f22538e..2487676 100644
@@ -115,7 +115,7 @@ typedef enum {
  * analysis of the state functions, but in reality just taken from
  * thin air in the hopes othat we don't trigger a kernel panic.
  */
-#define SCTP_MAX_NUM_COMMANDS 14
+#define SCTP_MAX_NUM_COMMANDS 20 
 
 typedef union {
 	void *zero_all;	/* Set to NULL to clear the entire union */
net/sctp/sm_statefuns.c +16 -3
index d3f1ea4..c8f6063 100644
@@ -1775,9 +1775,22 @@ static sctp_disposition_t sctp_sf_do_dupcook_a(struct net *net,
 	/* Update the content of current association. */
 	sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
 	sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP, SCTP_ULPEVENT(ev));
-	sctp_add_cmd_sf(commands, SCTP_CMD_NEW_STATE,
-			SCTP_STATE(SCTP_STATE_ESTABLISHED));
-	sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
+	if (sctp_state(asoc, SHUTDOWN_PENDING) &&
+	    (sctp_sstate(asoc->base.sk, CLOSING) ||
+	     sock_flag(asoc->base.sk, SOCK_DEAD))) {
+		/* if were currently in SHUTDOWN_PENDING, but the socket
+		 * has been closed by user, don't transition to ESTABLISHED.
+		 * Instead trigger SHUTDOWN bundled with COOKIE_ACK.
+		 */
+		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
+		return sctp_sf_do_9_2_start_shutdown(net, ep, asoc,
+						     SCTP_ST_CHUNK(0), NULL,
+						     commands);
+	} else {
+		sctp_add_cmd_sf(commands, SCTP_CMD_NEW_STATE,
+				SCTP_STATE(SCTP_STATE_ESTABLISHED));
+		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
+	}
 	return SCTP_DISPOSITION_CONSUME;
 
 nomem_ev:
Signature · 1 line
1.9.3
maintainer Applied
#1 lore raw

From: Vladislav Yasevich <[email protected]>
Date: Fri, 3 Oct 2014 18:16:20 -0400

Quoted 18 lines

From: Vlad Yasevich <[email protected]>

Currently association restarts do not take into consideration the state of the socket. When a restart happens, the current assocation simply transitions into established state. This creates a condition where a remote system, through a the restart procedure, may create a local association that is no way reachable by user. The conditions to trigger this are as follows:
1) Remote does not acknoledge some data causing data to remain outstanding.
2) Local application calls close() on the socket. Since data is still outstanding, the association is placed in SHUTDOWN_PENDING state. However, the socket is closed.
3) The remote tries to create a new association, triggering a restart on the local system. The association moves from SHUTDOWN_PENDING to ESTABLISHED. At this point, it is no longer reachable by any socket on the local system.

Quoted 8 lines

This patch addresses the above situation by moving the newly ESTABLISHED association into SHUTDOWN-SENT state and bundling a SHUTDOWN after the COOKIE-ACK chunk. This way, the restarted associate immidiately enters the shutdown procedure and forces the termination of the unreachable association.

Applied, thanks.

Candidate for -stable?

#2 lore raw

From: David Miller

Quoted 27 lines

From: Vladislav Yasevich <[email protected]>
Date: Fri, 3 Oct 2014 18:16:20 -0400

Quoted 24 lines

From: Vlad Yasevich <[email protected]>

Currently association restarts do not take into consideration the state of the socket. When a restart happens, the current assocation simply transitions into established state. This creates a condition where a remote system, through a the restart procedure, may create a local association that is no way reachable by user. The conditions to trigger this are as follows:
1) Remote does not acknoledge some data causing data to remain outstanding.
2) Local application calls close() on the socket. Since data is still outstanding, the association is placed in SHUTDOWN_PENDING state. However, the socket is closed.
3) The remote tries to create a new association, triggering a restart on the local system. The association moves from SHUTDOWN_PENDING to ESTABLISHED. At this point, it is no longer reachable by any socket on the local system.

This patch addresses the above situation by moving the newly ESTABLISHED association into SHUTDOWN-SENT state and bundling a SHUTDOWN after the COOKIE-ACK chunk. This way, the restarted associate immidiately enters the shutdown procedure and forces the termination of the unreachable association.

Quoted 6 lines
Quoted 2 lines

Applied, thanks.

Candidate for -stable?

Maybe, but while I raised the problem and tested the fix, I added a workaround to our M3UA code to ABORT sctp connections. I'll predicate it on a kernel version when I know which one it is in, but I'm not going to check backports to old distributions. (I'm tempted to disable sctp on 2.6.18 kernels...)

So I'm not worried -stable.

Other sctp users may be more worried.

David

#3 lore raw
In reply to David Miller
Quoted 27 lines · David Miller

From: Vladislav Yasevich <[email protected]>
Date: Fri, 3 Oct 2014 18:16:20 -0400

Quoted 24 lines

From: Vlad Yasevich <[email protected]>

Currently association restarts do not take into consideration the state of the socket. When a restart happens, the current assocation simply transitions into established state. This creates a condition where a remote system, through a the restart procedure, may create a local association that is no way reachable by user. The conditions to trigger this are as follows:
1) Remote does not acknoledge some data causing data to remain outstanding.
2) Local application calls close() on the socket. Since data is still outstanding, the association is placed in SHUTDOWN_PENDING state. However, the socket is closed.
3) The remote tries to create a new association, triggering a restart on the local system. The association moves from SHUTDOWN_PENDING to ESTABLISHED. At this point, it is no longer reachable by any socket on the local system.

This patch addresses the above situation by moving the newly ESTABLISHED association into SHUTDOWN-SENT state and bundling a SHUTDOWN after the COOKIE-ACK chunk. This way, the restarted associate immidiately enters the shutdown procedure and forces the termination of the unreachable association.

Quoted 7 lines
Quoted 2 lines

Applied, thanks.

Candidate for -stable?

I say yes. If this problem is triggered it is total pain to get the memory clean-up.

-vlad

maintainer
#4 lore raw

From: Vlad Yasevich <[email protected]>
Date: Mon, 06 Oct 2014 09:08:14 -0400

Quoted 5 lines · David Miller
Quoted 1 line

Candidate for -stable?

I say yes. If this problem is triggered it is total pain to get the memory clean-up.

Ok, queued up, thanks.