Ordering problem in if_detach_internal regarding if_bridge

[Alexander V. Chernikov]

On 23.06.2014 20:39, Alexander V. Chernikov wrote:
> On 23.06.2014 19:32, John Baldwin wrote:
>> On Friday, June 20, 2014 11:25:51 am Roger Pau Monné wrote:
>>> Hello,
>>>
>>> I've stumbled across the following panic when testing Xen netback with 
>>> if_bridge:
>>>
>>> Kernel page fault with the following non-sleepable locks held:
>>> exclusive sleep mutex if_bridge (if_bridge) r = 0 (0xfffff80006306c18) 
>> locked @ /usr/src/sys/m
>>> KDB: stack backtrace:
>>> X_db_symbol_values() at X_db_symbol_values+0x10b/frame 0xfffffe0000213490
>>> kdb_backtrace() at kdb_backtrace+0x39/frame 0xfffffe0000213540
>>> witness_warn() at witness_warn+0x4a8/frame 0xfffffe0000213600
>>> trap() at trap+0xc9d/frame 0xfffffe00002136a0
>>> trap() at trap+0x669/frame 0xfffffe00002138b0
>>> calltrap() at calltrap+0x8/frame 0xfffffe00002138b0
>>> --- trap 0xc, rip = 0xffffffff8221a0ef, rsp = 0xfffffe0000213970, rbp = 
>> 0xfffffe00002139e0 ---
>>> bridge_input() at bridge_input+0x5ff/frame 0xfffffe00002139e0
>>> ether_vlanencap() at ether_vlanencap+0x4a3/frame 0xfffffe0000213a10
>>> netisr_dispatch_src() at netisr_dispatch_src+0x90/frame 0xfffffe0000213a80
>>> ether_ifattach() at ether_ifattach+0x19f/frame 0xfffffe0000213ab0
>>> ath_dfs_get_thresholds() at ath_dfs_get_thresholds+0x81ce/frame 
>> 0xfffffe0000213b30
>>> intr_event_execute_handlers() at intr_event_execute_handlers+0x93/frame 
>> 0xfffffe0000213b70
>>> db_dump_intr_event() at db_dump_intr_event+0x796/frame 0xfffffe0000213bb0
>>> fork_exit() at fork_exit+0x84/frame 0xfffffe0000213bf0
>>> fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe0000213bf0
>>> --- trap 0, rip = 0, rsp = 0xfffffe0000213cb0, rbp = 0 ---
>>>
>>> I've tracked this down to if_detach_internal setting ifp->if_addr to 
>>> NULL before calling EVENTHANDLER_INVOKE(ifnet_departure_event..., which 
>>> causes a panic in GRAB_OUR_PACKETS in the if_bridge code when it tries 
>>> to perform IF_LLADDR on an interface that's in the process of being 
>>> destroyed (ifp->if_addr set to NULL, but the ifnet_departure_event event 
>>> has not fired yet).
>>>
>>> I have the following naive patch that moves the firing of the event 
>>> before if_addr is set to NULL, but I'm not familiar with the ordering 
>>> in if_detach_internal, so I'm not sure if this might cause problems in 
>>> other parts of the code, could someone familiar with the net stuff 
>>> comment on the best way to deal with it?
> 
> We should notify kernel customers only when we are really taking this
> interface down and every other subsystem cannot add any new state to the
> interface.
> 
> In this patch you're sending notification before taking ifnet down,
> removing its L3 addresses, routes, and so on.
> 
> This can easily lead to panic in, for example, BPF subsystem (since BPF
> state is freed in bpf_ifdetach() handler).
> 
> Addintionally, this will introduce ifaddr / iface messages reversal for
> rtsock.
Whoops. I misread the patch.
It should be OK.

> 
> It looks like we'd better fix if_bridge (and it is still using mutexes,
> what a shame!).
> 
> Can you send me trace with line numbers?
However, these two still stands.
(And I'm wondering how you're getting any traffic on down/dying interface).
> 
>>
>> Hmmm, I have no idea if this is ok or not.  I do think the route message 
>> should go out at the same time as the devctl_notify() call however.  My guess 
>> is it is actually better to do this earlier so that we allow outside consumers
>> to detach from an interface before it is destroyed.  I'm not sure if it would
>> break things, but I would be tempted to move this even earlier right after it
>> is removed from the global ifnet list but before the taskqueue_drain, etc.
>>
> 
> _______________________________________________
> freebsd-net at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
>