IPFW2 layer2 filtering broken - PATCH

Gary Allan dragonfly at gallan.plus.com
Mon Jan 24 11:12:13 PST 2005


> Matthew Dillon wrote:
    Well, our m_freem() allows m to be NULL so the NULL check is not
    necessary.
    From my read of the code, the 'eh = mtod(...)' is necessary, but 
    most of the time the returned 'm' will be the same as the passed 'm'
    so I am not surprised that you did not seeh = mtod(m, struct ether_header *);e any difference.

    Your email wasn't quite clear on the point... what change did you make
    which seemed to fix the problem for you?  It couldn't be the m_freem()
    change and you seem to indicate that it wasn't the 'eh = mtod...'
    change either.
					-Matt
					Matthew Dillon 
					<dillon at xxxxxxxxxxxxx>
Apologies,

The change was the removal of the "eh = mtod(m, struct ether_header *);" 
line. When present all incoming packets are dropped with sysctl 
net.link.ether.ipfw=1 and ipfw2 loaded.

I don't know if removing the line is the correct solution as I don't 
understand the code well enough but everything appears to be working 
after the change.

Amended diff:

diff -ruN if_ethersubr.c if_ethersubr.c.new
--- if_ethersubr.c      2005-01-24 18:24:07.000000000 +0000
+++ if_ethersubr.c.new  2005-01-24 18:48:56.000000000 +0000
@@ -680,7 +680,6 @@
                        m_freem(m);
                        return;
                }
-               eh = mtod(m, struct ether_header *);
        }
        ether_type = ntohs(eh->ether_type);



I narrowed the symptoms down to the second location where 
ether_ipfw_chk() is invoked and compared the source to FreeBSD RELENG_4. 
Also ether_ipfw_chk is called from another location within 
if_ethersubr.c. Here there is a second mtod() call not present in the 
FreeBSD code. (I don't know if thats a problem or not.)

* $DragonFly: src/sys/net/if_ethersubr.c,v 1.25 2005/01/06 09:14:13 hsu 
Exp $
 */

. ..

if (IPFW_LOADED && ether_ipfw != 0) {
                struct ether_header save_eh, *eh;
                eh = mtod(m, struct ether_header *);
                save_eh = *eh;
                m_adj(m, ETHER_HDR_LEN);
                if (!ether_ipfw_chk(&m, ifp, &rule, eh, FALSE)) {
                        if (m != NULL) {
                                m_freem(m);
                                return ENOBUFS; /* pkt dropped */
                        } else
                                return 0;       /* consumed e.g. in a 
pipe */
                }
                eh = mtod(m, struct ether_header *);
                /* packet was ok, restore the ethernet header */
                if ((void *)(eh + 1) == (void *)m->m_data) {
                        m->m_data -= ETHER_HDR_LEN ;
                        m->m_len += ETHER_HDR_LEN ;
                        m->m_pkthdr.len += ETHER_HDR_LEN ;
                } else {

Regards

G.Allan





More information about the Bugs mailing list