Re: IPFW2 layer2 filtering broken - PATCH

[Joerg Sonnenberger <joerg@xxxxxxxxxxxxxxxxx>]

On Mon, Jan 24, 2005 at 11:12:11AM -0800, Jeffrey Hsu wrote:
> This is an interface problem.  When ether_ipfw_chk() does not modify the
> mbuf, the recomputed eh pointer is incorrect because the mbuf has already
> been adjusted.  An ugly workaround is something like
> 
>        if (IPFW_LOADED && ether_ipfw != 0) {
> +               struct mbuf *n = m;
> +
>                if (!ether_ipfw_chk(&m, NULL, &rule, eh, FALSE)) {
>                        m_freem(m);
>                        return;
>                }
> -               eh = mtod(m, struct ether_header *);
> +               if (m != n)
> +                       eh = mtod(m, struct ether_header *);
>        }
> 
> Alternatively, we could change the 4th parameter to ether_ipfw_chk()
> to &eh and update it inside ether_ipfw_chk().

I'm not even sure if that is enough. The problem is that
ehter_ipfw_chk does an m_pullup. That can return a new mbuf.
So far so bad. The first problem is that the ether header might
be part of the old mbuf, but is not copied because of the m_adj
done earlier. This is a race condition. The second problem is that
ether header might really be outside the mbuf, in which case the
modifiction is just lost.

As a solution, ether_ipfw_chk has to update the eh pointer itself
and deal with the case of eh being part of the header. Do we care
enough about a few cycles? If not, we could just copy the ether header
into a temporary buffer, do the m_pullup and copy it into the new
mbuf if necessary.

Joerg