On Wed, Aug 29, 2012 at 09:30:58PM +0200, Holger Hans Peter Freyther wrote:
> On Fri, Aug 24, 2012 at 12:09:08PM +0200, Pablo Neira Ayuso wrote:
>
> > @@ -416,20 +447,21 @@ static int handle_ts1_read(struct osmo_fd *bfd)
> > LOGP(DLINP, LOGL_ERROR, "Fix your application, "
> > "no action set for signalling messages.\n");
> > ret = -EINVAL;
> > - goto err;
> > + goto err_msg;
> > }
>
> ah nice, potential memleak fix. :)
>
> > if (e1i_ts->line->ops->sign_link(msg) < 0) {
> > LOGP(DLINP, LOGL_ERROR, "Bad signalling message,"
> > "sign_link returned error: %s\n",
> > osmo_hexdump(msgb_l2(msg), msgb_l2len(msg)));
> > ret = -EINVAL;
> > + goto err;
>
> bad. but not from your code. sign_link does delete the msgb.. dumping it
> will work most of the time but is a "read after free".
Indeed. abis_[rsl|oml]_rcvmsg are usually called in the path of
->sign_link, and they are in charge of releasing the msgb.
I'm going to remove that dump and send a patch to add it to
openBSC's abis_[rsl|oml]_rcvmsg (before msgb is released).
> Acked-by: Holger Freyther <[email protected]>
>
>
> as far as I can see.. the error paths have no leaks
Thanks for your review Holger. Will fix the thing above and push the
patchset.
