ipfilter pass NULL mbuf to ether_output()

[Manuel Bouyer]

Hi,
In the update to IPFilter 4.1.29, sys/dist/ipf/netinet/ip_fil_netbsd.c
got this change in ipfr_fastroute6():
@@ -1498,9 +1544,9 @@
if ((error == 0) && (m0->m_pkthdr.len <= mtu)) {
*mpp = NULL;
# if __NetBSD_Version__ >= 499001100
- error = nd6_output(ifp, ifp, m0, satocsin6(dst), rt);
+ error = nd6_output(ifp, ifp, *mpp, satocsin6(dst), rt);
# else
- error = nd6_output(ifp, ifp, m0, dst6, rt);
+ error = nd6_output(ifp, ifp, *mpp, dst6, rt);
# endif

The effect of this change is to call nd6_output() with a NULL mbuf pointer,
and nd6_output() will call ether_output with this NULL pointer, and will
do a NULL pointer dereference. This happens when using 'return-rst' or
'return-icmp' in ipf6.conf. Reverting this changes avoids the panic and
makes ipfilter behaves as intended.

What was the purpose of this change ? Is it OK to revert it ?

[Darren Reed]

Post by Manuel Bouyer
Hi,
In the update to IPFilter 4.1.29, sys/dist/ipf/netinet/ip_fil_netbsd.c
@@ -1498,9 +1544,9 @@
if ((error == 0) && (m0->m_pkthdr.len <= mtu)) {
*mpp = NULL;
# if __NetBSD_Version__ >= 499001100
- error = nd6_output(ifp, ifp, m0, satocsin6(dst), rt);
+ error = nd6_output(ifp, ifp, *mpp, satocsin6(dst), rt);
# else
- error = nd6_output(ifp, ifp, m0, dst6, rt);
+ error = nd6_output(ifp, ifp, *mpp, dst6, rt);
# endif
The effect of this change is to call nd6_output() with a NULL mbuf pointer,
and nd6_output() will call ether_output with this NULL pointer, and will
do a NULL pointer dereference. This happens when using 'return-rst' or
'return-icmp' in ipf6.conf. Reverting this changes avoids the panic and
makes ipfilter behaves as intended.
What was the purpose of this change ? Is it OK to revert it ?

I believe the correct fix is to move the "*mpp = NULL" to be
after the calls to nd6_output()... either should work, but moving
the "*mpp = NULL" is what I'd consider to be more correct,
as although m0 is where the packet starts, if there's an mblk in
the chain that's before it (i.e. *mpp points to it) then it will end
up being leaked.

Darren


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