21 Aug
2012
21 Aug
'12
1:10 a.m.
Hi Holger,
On Mon, Aug 20, 2012 at 03:14:29PM +0200, Holger Hans Peter Freyther wrote:
Release the e1inp_line when the signalling link is
destroyed. The
e1inp_line was leaked on every OML/RSL disconnect.
The leak occured on the following call path.
e1inp_close_socket
ipaccess_close
e1inp_sign_link_destroy
ipaccess_drop_oml
ipaccess_drop
handle_ts1_read
ipaccess_fd_cb
---
src/e1_input.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/e1_input.c b/src/e1_input.c
index 957b74c..a85dd91 100644
--- a/src/e1_input.c
+++ b/src/e1_input.c
@@ -486,6 +486,7 @@ void e1inp_sign_link_destroy(struct e1inp_sign_link *link)
if (link->ts->line->driver->close)
link->ts->line->driver->close(link);
+ e1inp_line_put(link->ts->line);
talloc_free(link);
Good catch. If you want to fix it like that, I think that you also
need to add the following chunk below:
diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c
index 76d1994..3185bc0 100644
--- a/src/input/ipaccess.c
+++ b/src/input/ipaccess.c
@@ -249,9 +249,6 @@ static int ipaccess_drop(struct osmo_fd *bfd)
bfd->fd = -1;
ret = -ENOENT;
}
- /* put the virtual E1 line that we cloned for this socket, if
- * it becomes unused, it gets released. */
- e1inp_line_put(line);
return ret;
}
IIRC, the refcnt becomes 2 if both the OML and RSL links are up.
ipaccess_drop calls line->ops->sign_link_down(line), which usually
call e1inp_sign_link_destroy twice (one for the RSL link, and one for
the OML link).
With your patch, the refcount becomes zero after the two
e1inp_sign_link_destroy calls that happen in ->sign_link_down. Thus,
releasing the line that we were leaking.
Still, if you don't remove that e1inp_line_put in ipaccess_drop, you
hit an access-after-release since e1inp_line_put takes an already
released line.
But let me check this tomorrow again after some sleeping :-).