Re: cxgb: several issues

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 09/10/2016 à 01:10, Christos Zoulas a écrit :
> In article <86942d41-b91b-5ad5-4edf-fa02c5c1e6a8%m00nbsd.net@localhost>,
> Maxime Villard  <max%m00nbsd.net@localhost> wrote:
> > Mootja tells me that there is a memory leak here [1]. 'm' is allocated and
> > overwritten at l.130. The problem is, it seems to me 'm' is actually useless,
> > but I'm not sure if it somehow gets referenced earlier. Note, by the way, that
> > there is a similar bug later: at l.162 'm0' is allocated, and then overwritten
> > at l.185. And again, I'm not sure whether the leak can be triggered or not.
> >
> > Does someone have an idea? If not, I guess I'll just add some XXXs and file a
> > PR.
> >
> > [1] https://nxr.netbsd.org/xref/src/sys/dev/pci/cxgb/cxgb_l2t.c#106
>
> m is actually useless. the newer code in FreeBSD does not pass m in.
> m is placed in the arp queue earlier. I would put a comment that this
> is useless, remove the allocation and put a KASSERT there to make sure
> that m is not NULL.

It won't work, since in some calls m is explicitly NULL. In fact, it seems that
in the original freebsd version [1], there was an additional cxgb_ofld_send()
that actually made m useful. Since then they have made a lot of changes, and we
should probably sync our driver with them instead of just fixing 6+ year old
bugs.

I'll add some XXXs and leave it as is for now.

[1] https://svnweb.freebsd.org/base/head/sys/dev/cxgb/cxgb_l2t.c?annotate=169978&pathrev=178301#l122