From 7cee4b67e90aeb260bde11ee7f299e507c94bc10 Mon Sep 17 00:00:00 2001
From: Harald Welte <laforge@gnumonks.org>
Date: Sat, 8 Apr 2017 22:17:19 +0200
Subject: stream/datagram: Ensure reliable close/destroy

* when using osmo_*_destroy(), always call *_close() internally to
  make sure we don't free memory holding references to sockets that are
  still open
* when closing the socket, always make sure to set the fd to -1 in all
  cases, to avoid attempts to avoid later close() on a new file using
  the same fd number as the socket closed previously.

Change-Id: I29c37da6e8f5be8ab030e68952a8f92add146821
---
 src/datagram.c | 11 ++++++++++-
 src/stream.c   |  9 ++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/datagram.c b/src/datagram.c
index 13f1b5c..d98221f 100644
--- a/src/datagram.c
+++ b/src/datagram.c
@@ -54,8 +54,11 @@ struct osmo_dgram_tx {
  *  abstraction and close the socket */
 void osmo_dgram_tx_close(struct osmo_dgram_tx *conn)
 {
+	if (conn->ofd.fd == -1)
+		return;
 	osmo_fd_unregister(&conn->ofd);
 	close(conn->ofd.fd);
+	conn->ofd.fd = -1;
 }
 
 static int osmo_dgram_tx_write(struct osmo_dgram_tx *conn)
@@ -173,6 +176,7 @@ osmo_dgram_tx_set_data(struct osmo_dgram_tx *conn, void *data)
  *  \param[in] conn Datagram Transmitter to destroy */
 void osmo_dgram_tx_destroy(struct osmo_dgram_tx *conn)
 {
+	osmo_dgram_tx_close(conn);
 	talloc_free(conn);
 }
 
@@ -198,6 +202,7 @@ int osmo_dgram_tx_open(struct osmo_dgram_tx *conn)
 	conn->ofd.fd = ret;
 	if (osmo_fd_register(&conn->ofd) < 0) {
 		close(ret);
+		conn->ofd.fd = -1;
 		return -EIO;
 	}
 	return 0;
@@ -317,10 +322,10 @@ void osmo_dgram_rx_set_read_cb(struct osmo_dgram_rx *conn,
 }
 
 /*! \brief Destroy the datagram receiver. Releases Memory.
- *  Caller must make sure to osmo_dgram_rx_close() before calling
  *  \param[in] conn Datagram Receiver */
 void osmo_dgram_rx_destroy(struct osmo_dgram_rx *conn)
 {
+	osmo_dgram_rx_close(conn);
 	talloc_free(conn);
 }
 
@@ -345,6 +350,7 @@ int osmo_dgram_rx_open(struct osmo_dgram_rx *conn)
 	conn->ofd.fd = ret;
 	if (osmo_fd_register(&conn->ofd) < 0) {
 		close(ret);
+		conn->ofd.fd = -1;
 		return -EIO;
 	}
 	return 0;
@@ -356,8 +362,11 @@ int osmo_dgram_rx_open(struct osmo_dgram_rx *conn)
  *  \param[in] conn Stream Server Link to close */
 void osmo_dgram_rx_close(struct osmo_dgram_rx *conn)
 {
+	if (conn->ofd.fd == -1)
+		return;
 	osmo_fd_unregister(&conn->ofd);
 	close(conn->ofd.fd);
+	conn->ofd.fd = -1;
 }
 
 /*
diff --git a/src/stream.c b/src/stream.c
index 7bac1cc..f899a41 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -367,10 +367,11 @@ osmo_stream_cli_set_read_cb(struct osmo_stream_cli *cli,
 	cli->read_cb = read_cb;
 }
 
-/*! \brief Destroy a Osmocom stream client
+/*! \brief Destroy a Osmocom stream client (includes close)
  *  \param[in] cli Stream Client to destroy */
 void osmo_stream_cli_destroy(struct osmo_stream_cli *cli)
 {
+	osmo_stream_cli_close(cli);
 	osmo_timer_del(&cli->timer);
 	talloc_free(cli);
 }
@@ -402,6 +403,7 @@ int osmo_stream_cli_open2(struct osmo_stream_cli *cli, int reconnect)
 	cli->ofd.fd = ret;
 	if (osmo_fd_register(&cli->ofd) < 0) {
 		close(ret);
+		cli->ofd.fd = -1;
 		return -EIO;
 	}
 	return 0;
@@ -601,11 +603,11 @@ void osmo_stream_srv_link_set_accept_cb(struct osmo_stream_srv_link *link,
 	link->accept_cb = accept_cb;
 }
 
-/*! \brief Destroy the stream server link. Releases Memory.
- *  Caller must make sure to osmo_stream_srv_link_close() before calling
+/*! \brief Destroy the stream server link. Closes + Releases Memory.
  *  \param[in] link Stream Server Link */
 void osmo_stream_srv_link_destroy(struct osmo_stream_srv_link *link)
 {
+	osmo_stream_srv_link_close(link);
 	talloc_free(link);
 }
 
@@ -630,6 +632,7 @@ int osmo_stream_srv_link_open(struct osmo_stream_srv_link *link)
 	link->ofd.fd = ret;
 	if (osmo_fd_register(&link->ofd) < 0) {
 		close(ret);
+		link->ofd.fd = -1;
 		return -EIO;
 	}
 	return 0;
-- 
cgit v1.2.3