Brainy: Memory Leak in if_ieee1394

[Maxime Villard]

Hi,
there is a memory leak here:

----------------------- sys/net/if_ieee1394subr.c ----------------------

MGETHDR(m, M_DONTWAIT, MT_HEADER);
if (m == NULL)
goto bad;
m->m_flags |= m0->m_flags & (M_BCAST|M_MCAST); /* copy bcast */
MH_ALIGN(m, sizeof(struct ieee1394_fraghdr));
m->m_len = sizeof(struct ieee1394_fraghdr);
ifh = mtod(m, struct ieee1394_fraghdr *);
ifh->ifh_ft_size =
htons(IEEE1394_FT_SUBSEQ | IEEE1394_FT_MORE | (totlen - 1));
ifh->ifh_etype_off = htons(off);
ifh->ifh_dgl = htons(ic->ic_dgl);
ifh->ifh_reserved = 0;
m->m_next = m_copy(m0, sizeof(*ifh) + off, fraglen);
if (m->m_next == NULL)
XX goto bad;
m->m_pkthdr.len = sizeof(*ifh) + fraglen;
off += fraglen;
*mp = m;
mp = &m->m_nextpkt;
}
ifh->ifh_ft_size &= ~htons(IEEE1394_FT_MORE); /* last fragment */
m_adj(m0, -(m0->m_pkthdr.len - maxsize));

ic->ic_dgl++;
return m0;

bad:
while ((m = m0) != NULL) {
m0 = m->m_nextpkt;
m->m_nextpkt = NULL;
m_freem(m);
}
return NULL;

------------------------------------------------------------------------

You can see that 'm' is allocated, and if m_copy() fails, the function
jumps to 'bad' and the pointer is overwritten; so the memory is lost.

I think a correct fix would be:

if (m->m_next == NULL) {
m_freem(m);
goto bad;
}

But I also think it really needs to be tested...

Found by Brainy.

Thanks,
Maxime

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-...@muc.de

[Ignatios Souvatzis]

Hi,

On Sat, Jul 25, 2015 at 10:28:21AM +0200, Maxime Villard wrote:

> there is a memory leak here:
>
> ----------------------- sys/net/if_ieee1394subr.c ----------------------

...

> I think a correct fix would be:
>
> if (m->m_next == NULL) {
> m_freem(m);
> goto bad;
> }

Looks good to me.

-is
--
A medium apple... weighs 182 grams, yields 95 kcal, and contains no
caffeine, thus making it unsuitable for sysadmins. - Brian Kantor