Re: ipfilter pass NULL mbuf to ether_output()

[Darren Reed <darrenr%netbsd.org@localhost>]

Manuel Bouyer wrote:
> 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 ?
>   

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