Change in libosmo-sccp[master]: WIP: Fix race during fast re-establishment of inbound M3UA connections

[laforge]

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/19001 )


Change subject: WIP: Fix race during fast re-establishment of inbound M3UA connections
......................................................................

WIP: Fix race during fast re-establishment of inbound M3UA connections

When a client closes and instantaneously re-opens a SCTP socket for an
M3UA connection, there is a chance that both the "shutdwon event" (old
connection socket becomes readable for sctp event) and the "init event"
(listen-fd becomes readable) happen during the same scheduler interval /
select() cycle.  As there is no guaranteed order by which we call our
file descriptor callbacks, it means that we may end up processing
then new connection (accept) before we get the notification that the
old one is dead.

The fact that the fd number of the accept-fd is mostly lower than the fd
number of the individual per-client connection actually makes it likely
that the order is exactly the opposite of what would feel "logical".

As the ASP is identified by the tuple of (src-port, src-ip, dst-port, dst-ip),
both the old connection and the new connection map to the same ASP
object.  So we need to handle this situation gracefully:  If we get a
new connection for a tuple that we already [think we still] have one,
close the old one and use the new.

Change-Id: I9b3ae6dfcf6efeabb7fb6c33503d1d7924fec2fa
Closes: OS#4625
---
M src/osmo_ss7.c
1 file changed, 12 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/01/19001/1

diff --git a/src/osmo_ss7.c b/src/osmo_ss7.c
index b7d69cb..9aeed9b 100644
--- a/src/osmo_ss7.c
+++ b/src/osmo_ss7.c
@@ -1832,6 +1832,17 @@
 	if (asp) {
 		LOGP(DLSS7, LOGL_INFO, "%s: matched connection to ASP %s\n",
 			sock_name, asp->cfg.name);
+		/* we need to check if we already have a socket associated, and close it.  Otherwise it might
+		 * happen that both the listen-fd for this accept() and the old socket are marked 'readable'
+		 * during the same scheduling interval, and we're processing them in the "wrong" order, i.e.
+		 * we first see the accept of the new fd before we see the close on the old fd */
+		if (asp->server) {
+			LOGPASP(asp, DLSS7, LOGL_FATAL, "accept of new connection from %s before old was closed "
+				"-> close old one\n", sock_name);
+			osmo_stream_srv_set_data(asp->server, NULL);
+			osmo_stream_srv_destroy(asp->server);
+			asp->server = NULL;
+		}
 	} else {
 		if (!oxs->cfg.accept_dyn_reg) {
 			LOGP(DLSS7, LOGL_NOTICE, "%s: %s connection without matching "
@@ -1870,13 +1881,13 @@
 			talloc_free(sock_name);
 			return -1;
 		}
+		llist_add_tail(&asp->siblings, &oxs->asp_list);
 	}
 
 	/* update the ASP reference back to the server over which the
 	 * connection came in */
 	asp->server = srv;
 	asp->xua_server = oxs;
-	llist_add_tail(&asp->siblings, &oxs->asp_list);
 	/* update the ASP socket name */
 	if (asp->sock_name)
 		talloc_free(asp->sock_name);

-- 
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/19001
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: I9b3ae6dfcf6efeabb7fb6c33503d1d7924fec2fa
Gerrit-Change-Number: 19001
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200624/117a1f6d/attachment.htm>