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 :-).