[MERGED] libosmo-netif[master]: osmo_stream_srv_fd_cb(): don't leak socket FDs on errors

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Fri Apr 14 09:12:00 UTC 2017


Harald Welte has submitted this change and it was merged.

Change subject: osmo_stream_srv_fd_cb(): don't leak socket FDs on errors
......................................................................


osmo_stream_srv_fd_cb(): don't leak socket FDs on errors

So far we seem to assume that the accept_cb does all handling of socket
fd cleanup. However, there are cases where there is no accept_cb set,
the accept_cb returns error, or an earlier sctp_sock_activate_events()
or the activation of non-blocking mode fails.

For those cases, close the socket and return an error code.

Fixes: CID#57915
Change-Id: I3a3ce9194ab7ca5c1921fc79c2a1c9e10a552cf0
---
M src/stream.c
1 file changed, 23 insertions(+), 6 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/stream.c b/src/stream.c
index 591cd06..653f26b 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -529,6 +529,7 @@
 static int osmo_stream_srv_fd_cb(struct osmo_fd *ofd, unsigned int what)
 {
 	int ret;
+	int sock_fd;
 	struct sockaddr_in sa;
 	socklen_t sa_len = sizeof(sa);
 	struct osmo_stream_srv_link *link = ofd->data;
@@ -541,17 +542,33 @@
 	}
 	LOGP(DLINP, LOGL_DEBUG, "accept()ed new link from %s to port %u\n",
 		inet_ntoa(sa.sin_addr), link->port);
+	sock_fd = ret;
 
-	if (link->proto == IPPROTO_SCTP)
-		sctp_sock_activate_events(ret);
+	if (link->proto == IPPROTO_SCTP) {
+		ret = sctp_sock_activate_events(sock_fd);
+		if (ret < 0)
+			goto error_close_socket;
+	}
 
-	if (link->flags & OSMO_STREAM_SRV_F_NODELAY)
-		setsockopt_nodelay(ret, link->proto, 1);
+	if (link->flags & OSMO_STREAM_SRV_F_NODELAY) {
+		ret = setsockopt_nodelay(ret, link->proto, 1);
+		if (ret < 0)
+			goto error_close_socket;
+	}
 
-	if (link->accept_cb)
-		link->accept_cb(link, ret);
+	if (!link->accept_cb) {
+		ret = -ENOTSUP;
+		goto error_close_socket;
+	}
 
+	ret = link->accept_cb(link, sock_fd);
+	if (ret)
+		goto error_close_socket;
 	return 0;
+
+error_close_socket:
+	close(sock_fd);
+	return ret;
 }
 
 /*! \brief Create an Osmocom Stream Server Link

-- 
To view, visit https://gerrit.osmocom.org/1320
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I3a3ce9194ab7ca5c1921fc79c2a1c9e10a552cf0
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list