tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: arpresolve: inverted logic



Le 29/01/2017 à 22:58, Christos Zoulas a écrit :
In article <005b4467-9b1b-d95c-1714-60a2c22d9777%m00nbsd.net@localhost>,
Maxime Villard  <max%m00nbsd.net@localhost> wrote:
Hi,
there appears to be a wrong logic in eco_output and token_output. arpresolve
returns a non-zero value on failure, but these functions think it returns zero.
So when arpresolve succeeds, these functions return 0 and leak the mbuf; when
arpresolve fails, these functions believe it's ok and use 'm' while it has been
freed. In short, either a memory leak or a use-after-free.

I have written a quick patch [1], which I cannot test. Could someone please
review it?

Thanks,
Maxime

[1] http://m00nbsd.net/garbage/arp/arpresolve.diff

LGTM

christos

Alright, thanks, I've committed the patch.

Now; do we also agree on the fact that there is a m_put_rcvif(_rcvif, &s)
missing before this goto [1]? Without talking about m_get_rcvif that could
return NULL.

It seems to me we could also optimize a bit by not unnecessarily holding
the lock if ifp->if_type != IFT_CARP.

[1] https://nxr.netbsd.org/xref/src/sys/netinet/if_arp.c#1317


Home | Main Index | Thread Index | Old Index