panic with ng_ipfw+ng_car and net.inet.ip.fw.one_pass=0

Mon May 25 19:29:30 UTC 2009

Hi,

Some times ago it has been posted to fido7.ru.unix.bsd about panics when using
ipfw + ng_ipfw + ng_car.

http://groups.google.com/group/fido7.ru.unix.bsd/browse_thread/thread/5907d1ba4e76675d

For those who haven't learnt Russian yet ;-) here are some details. Max
Irgiznov reported that when ng_ipf+ng_car construction was used and
net.inet.ip.fw.one_pass=0 was set, the system reliably panicked on ipfw rules
reload if there was some traffic through ng_car.

The problem here is in the following. When the packet is returning back from
ng_car queue to ipfw_chk and one_pass is turned off the next rule is being
tried. But if the rules were reloaded while the packet was sitting in ng_car,
the next rule pointer might be dangling and the kernel will panic.

(kgdb) bt
#0  doadump () at pcpu.h:196
#1  0xc07e1f7e in boot (howto=260) at /usr/src/sys/kern/kern_shutdown.c:418
#2  0xc07e2252 in panic (fmt=Variable "fmt" is not available.
) at /usr/src/sys/kern/kern_shutdown.c:574
#3  0xc0495eb7 in db_panic (addr=Could not find the frame base for "db_panic".
) at /usr/src/sys/ddb/db_command.c:446
#4  0xc04968bc in db_command (last_cmdp=0xc0c97514, cmd_table=0x0, dopager=1)
    at /usr/src/sys/ddb/db_command.c:413
#5  0xc04969ca in db_command_loop () at /usr/src/sys/ddb/db_command.c:466
#6  0xc04981bd in db_trap (type=12, code=0) at /usr/src/sys/ddb/db_main.c:228
#7  0xc080ec76 in kdb_trap (type=12, code=0, tf=0xe6945774) at /usr/src/sys/kern/subr_kdb.c:524
#8  0xc0ad9e4f in trap_fatal (frame=0xe6945774, eva=3735929068) at /usr/src/sys/i386/i386/trap.c:930
#9  0xc0ada790 in trap (frame=0xe6945774) at /usr/src/sys/i386/i386/trap.c:320
#10 0xc0abeaab in calltrap () at /usr/src/sys/i386/i386/exception.s:159
#11 0xc903328c in ipfw_chk (args=0xe6945acc) at /usr/src/sys/modules/ipfw/../../netinet/ip_fw2.c:2516
#12 0xc90373f7 in ipfw_check_in (arg=0x0, m0=0xe6945bd0, ifp=0xc41f9000, dir=1, inp=0x0)
    at /usr/src/sys/modules/ipfw/../../netinet/ip_fw_pfil.c:125
#13 0xc088d6e8 in pfil_run_hooks (ph=0xc0d1f620, mp=0xe6945c24, ifp=0xc41f9000, dir=1, inp=0x0)
    at /usr/src/sys/net/pfil.c:78
#14 0xc08c766d in ip_input (m=0xc409ad00) at /usr/src/sys/netinet/ip_input.c:416
#15 0xc9011c39 in ng_ipfw_rcvdata (hook=0xc61a1780, item=0xc8fe5090)
    at /usr/src/sys/modules/netgraph/ipfw/../../../netgraph/ng_ipfw.c:250
#16 0xc68b80af in ng_apply_item (node=0xc7054c00, item=0xc8fe5090, rw=0)
    at /usr/src/sys/modules/netgraph/netgraph/../../../netgraph/ng_base.c:2336
#17 0xc68b939f in ngthread (arg=0x0) at /usr/src/sys/modules/netgraph/netgraph/../../../netgraph/ng_base.c:3304
#18 0xc07be4c8 in fork_exit (callout=0xc68b91f0 <ngthread>, arg=0x0, frame=0xe6945d38)
    at /usr/src/sys/kern/kern_fork.c:810
#19 0xc0abeb20 in fork_trampoline () at /usr/src/sys/i386/i386/exception.s:264
(kgdb) frame 11
#11 0xc903328c in ipfw_chk (args=0xe6945acc) at /usr/src/sys/modules/ipfw/../../netinet/ip_fw2.c:2516
warning: Source file is more recent than executable.

2516                    if (set_disable & (1 << f->set) )
(kgdb) list
2511                    ipfw_insn *cmd;
2512                    uint32_t tablearg = 0;
2513                    int l, cmdlen, skip_or; /* skip rest of OR block */
2514
2515    again:
2516                    if (set_disable & (1 << f->set) )
2517                            continue;
2518
2519                    skip_or = 0;
2520                    for (l = f->cmd_len, cmd = f->cmd ; l > 0 ;
(kgdb) p f
$1 = (struct ip_fw *) 0xdeadc0de
(kgdb) 

DUMMYNET does not have such problems as ip_dn_ruledel_ptr(rule) is called when
the rule is removed in reap_rules(). The first thought was to do the same here
i.e. to broadcast "remove the rule" message to netgraph nodes, but glancing
through the netgraph man I haven't figured out how it could be done if it is
possible at all.

So the other solution is to have some counter that increases every time when
any rules are removed. When the packet is directed by ipfw to netgraph
subsystem, the current value of the counter is stored in mtag. When the packet
is coming back the current value of the counter is compared with one from the
mtag and if they differ the packet is dropped.

Just to prove the concept I have modified ip_fw2.c for 7.2-STABLE accordingly
and it works for me. The patch is attached.

I would like to hear other people opinion, first of all if the proposed idea
is good enough or there might be other better solutions for the problem
(e.g. "remove the rule" broadcasting is possible). But also if somebody have
any remarks about the patch itself I would happy to see them. E.g. I have
added the counter just as static variable but as for me struct ip_fw_chain
could be better place for this. Also is there any need to mark the tag with
MTAG_PERSISTENT bit?

-- 
Mikolaj Golub

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ip_fw2.c.patch
Type: text/x-diff
Size: 1952 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-net/attachments/20090525/46293d3b/ip_fw2.c.bin