[PATCH] e1: Memory leak/Reference leak on the e1inp_line

[Pablo Neira Ayuso]

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