laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/36125?usp=email )
(
4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: stream_{cli,srv}: Free received messages when not forwarded ......................................................................
stream_{cli,srv}: Free received messages when not forwarded
If a message is not forwarded (to a read callback function, it must be freed, to prevent memory leaks.
The message musst be freed before calling osmo_stream_srv_destroy() or stream_cli_handle_connecting(), because within the function calls the client/server instance may get destroyed and the message is 'owned' by it. Calling msgb_free(msg) afterwards may result in double free bug.
Related: OS#5753 Change-Id: Ic043f11cdba0df9e0b78cac8db7206800098e0ba --- M src/stream_cli.c M src/stream_srv.c 2 files changed, 26 insertions(+), 0 deletions(-)
Approvals: laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified
diff --git a/src/stream_cli.c b/src/stream_cli.c index eef442c..1197b53 100644 --- a/src/stream_cli.c +++ b/src/stream_cli.c @@ -438,6 +438,7 @@
switch (cli->state) { case STREAM_CLI_STATE_CONNECTING: + msgb_free(msg); stream_cli_handle_connecting(cli, res); break; case STREAM_CLI_STATE_CONNECTED: @@ -445,6 +446,8 @@ osmo_stream_cli_reconnect(cli); else if (cli->iofd_read_cb) cli->iofd_read_cb(cli, msg); + else + msgb_free(msg); break; default: osmo_panic("%s() called with unexpected state %d\n", __func__, cli->state); @@ -486,6 +489,7 @@
switch (cli->state) { case STREAM_CLI_STATE_CONNECTING: + msgb_free(msg); stream_cli_handle_connecting(cli, res); break; case STREAM_CLI_STATE_CONNECTED: @@ -494,6 +498,8 @@ /* Forward message to read callback, also if the connection failed. */ if (cli->iofd_read_cb) cli->iofd_read_cb(cli, msg); + else + msgb_free(msg); break; default: osmo_panic("%s() called with unexpected state %d\n", __func__, cli->state); diff --git a/src/stream_srv.c b/src/stream_srv.c index 33494fa..95a2cbb 100644 --- a/src/stream_srv.c +++ b/src/stream_srv.c @@ -562,6 +562,7 @@
if (OSMO_UNLIKELY(res <= 0)) { /* This connection is dead, destroy it. */ + msgb_free(msg); osmo_stream_srv_destroy(conn); } else { if (conn->flags & OSMO_STREAM_SRV_F_FLUSH_DESTROY) { @@ -609,6 +610,7 @@
if (OSMO_UNLIKELY(res <= 0)) { /* This connection is dead, destroy it. */ + msgb_free(msg); osmo_stream_srv_destroy(conn); } else { if (conn->flags & OSMO_STREAM_SRV_F_FLUSH_DESTROY) {