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

Mikolaj Golub at
Mon Jun 1 08:12:51 UTC 2009

On Mon, 25 May 2009 22:29:25 +0300 Mikolaj Golub wrote:

> Hi,
> Some times ago it has been posted to about panics when using
> ipfw + ng_ipfw + ng_car.
> 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.

It looks the problem has not drawn much attention :-).

Anyway, another version of the patch is attached. This time almost all of the
necessary modifications are done in ng_ipfw module. Only the small changes
have been made in ip_fw module and I tried to make them in the same manner as
it is done for dummynet.

The main logic is the same as in the previous patch: have internal counter
ng_ipfw_rdcnt that is increased every time when some rule is deleted from the
chain and compare it with one stored in ng_ipfw_tag when a packet passes

The patch is against 8-CURRENT but it is applied (and has been tested) to
7-STABLE too.

Actually with this version of patch it looks like there is still small chance
of race when ng_ipfw_rdcnt is going to be increased and in the same time the
current value is stored in packet arrived to ng_ipfw. But running attached
test script in loop I was not able to crash patched system while without the
patch the system reliably crashes on the second run of the script.

It would be nice to have this patch at least in CURRENT. Although I think that
some generic mechanism should be developed in ipfw to validate rule pointers
of second pass packets to have net.inet.ip.fw.one_pass=0 feature safe. AFAIK
ipfw improvements is this year Summer of Code project so this problem could be
addressed there. At least it should be documented in ipfw in BUGS section that
the currrent implementation of net.inet.ip.fw.one_pass=0 could panic the
system when is used with netgraph.

Mikolaj Golub

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ng_ipfw.patch
Type: text/x-diff
Size: 3268 bytes
Desc: not available
Url :

More information about the freebsd-net mailing list