Change in libosmo-netif[master]: detect freed connections in osmo_stream_srv_read()
Stefan Sperling has submitted this change and it was merged. (
https://gerrit.osmocom.org/11705 )
Change subject: detect freed connections in osmo_stream_srv_read()
..
detect freed connections in osmo_stream_srv_read()
While we are processing a read event, the connection's
callback might free the connection. Check for this and don't
attempt to process further events on an already freed connection.
Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0
Related: OS#3685
Depends: g#11704 (for libosmo-sccp)
---
M src/stream.c
1 file changed, 10 insertions(+), 7 deletions(-)
Approvals:
Jenkins Builder: Verified
Pau Espin Pedrol: Looks good to me, approved
diff --git a/src/stream.c b/src/stream.c
index 6eb2313..4548414 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -784,19 +784,21 @@
int flags;
};
-static void osmo_stream_srv_read(struct osmo_stream_srv *conn)
+static int osmo_stream_srv_read(struct osmo_stream_srv *conn)
{
+ int rc = 0;
+
LOGP(DLINP, LOGL_DEBUG, "message received\n");
if (conn->flags & OSMO_STREAM_SRV_F_FLUSH_DESTROY) {
LOGP(DLINP, LOGL_DEBUG, "Connection is being flushed and
closed; ignoring received message\n");
- return;
+ return 0;
}
if (conn->cb)
- conn->cb(conn);
+ rc = conn->cb(conn);
- return;
+ return rc;
}
static void osmo_stream_srv_write(struct osmo_stream_srv *conn)
@@ -845,14 +847,15 @@
static int osmo_stream_srv_cb(struct osmo_fd *ofd, unsigned int what)
{
struct osmo_stream_srv *conn = ofd->data;
+ int rc = 0;
LOGP(DLINP, LOGL_DEBUG, "connected read/write\n");
if (what & BSC_FD_READ)
- osmo_stream_srv_read(conn);
- if (what & BSC_FD_WRITE)
+ rc = osmo_stream_srv_read(conn);
+ if (rc != -EBADF && (what & BSC_FD_WRITE))
osmo_stream_srv_write(conn);
- return 0;
+ return rc;
}
/*! \brief Create a Stream Server inside the specified link
--
To view, visit https://gerrit.osmocom.org/11705
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0
Gerrit-Change-Number: 11705
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol
Gerrit-Reviewer: Stefan Sperling
Change in libosmo-netif[master]: detect freed connections in osmo_stream_srv_read()
Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/11705 ) Change subject: detect freed connections in osmo_stream_srv_read() .. Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/11705 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0 Gerrit-Change-Number: 11705 Gerrit-PatchSet: 2 Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: Stefan Sperling Gerrit-Comment-Date: Fri, 09 Nov 2018 15:18:07 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in libosmo-netif[master]: detect freed connections in osmo_stream_srv_read()
Stefan Sperling has posted comments on this change. ( https://gerrit.osmocom.org/11705 ) Change subject: detect freed connections in osmo_stream_srv_read() .. Patch Set 2: (1 comment) https://gerrit.osmocom.org/#/c/11705/1/src/stream.c File src/stream.c: https://gerrit.osmocom.org/#/c/11705/1/src/stream.c@855 PS1, Line 855: if (rc != -EBADF && (what & BSC_FD_WRITE)) > In all places I know of, we use -EBADF as an indicator that the osmo_fd > struct was freed and should […] Indeed, using EBADF is more consistent with other code. Thanks for hint. -- To view, visit https://gerrit.osmocom.org/11705 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0 Gerrit-Change-Number: 11705 Gerrit-PatchSet: 2 Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: Stefan Sperling Gerrit-Comment-Date: Fri, 09 Nov 2018 14:44:43 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in libosmo-netif[master]: detect freed connections in osmo_stream_srv_read()
Hello Pau Espin Pedrol, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11705 to look at the new patch set (#2). Change subject: detect freed connections in osmo_stream_srv_read() .. detect freed connections in osmo_stream_srv_read() While we are processing a read event, the connection's callback might free the connection. Check for this and don't attempt to process further events on an already freed connection. Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0 Related: OS#3685 Depends: g#11704 (for libosmo-sccp) --- M src/stream.c 1 file changed, 10 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/05/11705/2 -- To view, visit https://gerrit.osmocom.org/11705 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0 Gerrit-Change-Number: 11705 Gerrit-PatchSet: 2 Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Pau Espin Pedrol
Change in libosmo-netif[master]: detect freed connections in osmo_stream_srv_read()
Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/11705 ) Change subject: detect freed connections in osmo_stream_srv_read() .. Patch Set 1: Code-Review-1 (1 comment) https://gerrit.osmocom.org/#/c/11705/1/src/stream.c File src/stream.c: https://gerrit.osmocom.org/#/c/11705/1/src/stream.c@855 PS1, Line 855: if (rc < 0) In all places I know of, we use -EBADF as an indicator that the osmo_fd struct was freed and should not keep being used, since usually having any error parsing some message doesn't mean you want to stop from writing on the socket. See for instance: libosmo-abis/src/input/ipaccess.c:401 libosmo-abis/src/input/ipa.c:138 libosmo-abis/src/input/ipa.c:384 libosmocore/src/write_queue.c And grep for "-EBADF" in general. -- To view, visit https://gerrit.osmocom.org/11705 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0 Gerrit-Change-Number: 11705 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Comment-Date: Fri, 09 Nov 2018 14:16:19 + Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
Change in libosmo-netif[master]: detect freed connections in osmo_stream_srv_read()
Stefan Sperling has uploaded this change for review. (
https://gerrit.osmocom.org/11705
Change subject: detect freed connections in osmo_stream_srv_read()
..
detect freed connections in osmo_stream_srv_read()
While we are processing a read event, the connection's
callback might free the connection. Check for this and don't
attempt to process further events on an already freed connection.
Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0
Related: OS#3685
Depends: g#11704 (for libosmo-sccp)
---
M src/stream.c
1 file changed, 12 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/05/11705/1
diff --git a/src/stream.c b/src/stream.c
index 6eb2313..7fa69ca 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -784,19 +784,21 @@
int flags;
};
-static void osmo_stream_srv_read(struct osmo_stream_srv *conn)
+static int osmo_stream_srv_read(struct osmo_stream_srv *conn)
{
+ int rc = 0;
+
LOGP(DLINP, LOGL_DEBUG, "message received\n");
if (conn->flags & OSMO_STREAM_SRV_F_FLUSH_DESTROY) {
LOGP(DLINP, LOGL_DEBUG, "Connection is being flushed and
closed; ignoring received message\n");
- return;
+ return 0;
}
if (conn->cb)
- conn->cb(conn);
+ rc = conn->cb(conn);
- return;
+ return rc;
}
static void osmo_stream_srv_write(struct osmo_stream_srv *conn)
@@ -845,10 +847,14 @@
static int osmo_stream_srv_cb(struct osmo_fd *ofd, unsigned int what)
{
struct osmo_stream_srv *conn = ofd->data;
+ int rc;
LOGP(DLINP, LOGL_DEBUG, "connected read/write\n");
- if (what & BSC_FD_READ)
- osmo_stream_srv_read(conn);
+ if (what & BSC_FD_READ) {
+ rc = osmo_stream_srv_read(conn);
+ if (rc < 0)
+ return rc;
+ }
if (what & BSC_FD_WRITE)
osmo_stream_srv_write(conn);
--
To view, visit https://gerrit.osmocom.org/11705
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0
Gerrit-Change-Number: 11705
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling
