panic on invalid ifp pointer in iflib drivers

[Keller, Jacob E]

> -----Original Message-----
> From: Keller, Jacob E
> Sent: Thursday, October 17, 2019 4:22 PM
> To: John Baldwin <jhb at FreeBSD.org>; freebsd-net at freebsd.org
> Cc: shurd at llnw.com; Joyner, Eric <eric.joyner at intel.com>
> Subject: RE: panic on invalid ifp pointer in iflib drivers
> 
> > -----Original Message-----
> > From: John Baldwin <jhb at FreeBSD.org>
> > Sent: Thursday, October 17, 2019 9:31 AM
> > To: Keller, Jacob E <jacob.e.keller at intel.com>; freebsd-net at freebsd.org
> > Cc: shurd at llnw.com; Joyner, Eric <eric.joyner at intel.com>
> > Subject: Re: panic on invalid ifp pointer in iflib drivers
> >
> > Nominally, ifnet drivers should call ether_ifdetach first to remove public
> > references to the ifnet and only call their stop routine after that has returned.
> > This ensures any open if_ioctl invocations have completed, etc. before the
> > stop routine is invoked.  Otherwise you are open to a race where the inteface
> > can be upped via an ioctl after you have stopped the hardware.
> >
> > Any other references to the ifnet via eventhandlers, etc. should also be
> > deregistered before calling the stop routine.
> 
> Looks like iflib moved this much later when we refactored to add a shared
> function to deregister VLAN handlers...
> 
> >
> > After the hardware is stopped, interrupt handlers should be torn down and
> > callouts
> > and tasks drained to ensure there are no other references to the ifp outside of
> > the thread running detach.
> >
> > After that you can release device resources, destroy mutexes, free the ifp, etc.
> > Note that drivers have to be prepared for ether_ifdetach to invoke if_ioctl (e.g.
> > when detaching bpf), but of the drivers I've looked at this has generally been a
> > non-issue.
> >
> > It sounds like iflib should be doing the detach before calling iflib_stop.
> >
> 
> I tested a patch that moved ether_ifdetach above the call to iflib_stop.
> 
> This seems to have made the issue significantly harder to reproduce, but after
> multiple attach/detach cycles with IPv6 traffic: (INVARIANTS and WITNESS are
> enabled, as well as meguard protecting ifnet)
> 
> Unread portion of the kernel message buffer:
> Kernel page fault with the following non-sleepable locks held:
> exclusive sleep mutex ip6qlock (ip6qlock) r = 0 (0xfffffe00007aa848) locked @
> /usr/src/sys/netinet6/frag6.c:849
> shared rw vnet_rwlock (vnet_rwlock) r = 0 (0xffffffff820be700) locked @
> /usr/src/sys/netinet6/frag6.c:845
> stack backtrace:
> #0 0xffffffff80bb6f83 at witness_debugger+0x73
> #1 0xffffffff80bb7fa2 at witness_warn+0x442
> #2 0xffffffff8108a0f3 at trap_pfault+0x53
> #3 0xffffffff810896e4 at trap+0x2b4
> #4 0xffffffff8106201c at calltrap+0x8
> #5 0xffffffff80d8c07a at icmp6_error+0x4aa
> #6 0xffffffff80d8b30e at frag6_freef+0x10e
> #7 0xffffffff80d8b551 at frag6_slowtimo+0x111
> #8 0xffffffff80bdcda4 at pfslowtimo+0x54
> #9 0xffffffff80b65bdf at softclock_call_cc+0x13f
> #10 0xffffffff80b65f9c at softclock+0x7c
> #11 0xffffffff80b0f857 at ithread_loop+0x187
> #12 0xffffffff80b0c4a4 at fork_exit+0x84
> #13 0xffffffff8106305e at fork_trampoline+0xe
> 
> Fatal trap 12: page fault while in kernel mode
> cpuid = 0; apic id = 00
> fault virtual address   = 0xfffffe0000825dd8
> fault code              = supervisor read data, page not present
> instruction pointer     = 0x20:0xffffffff80d8c5b2
> stack pointer           = 0x28:0xfffffe1fc28c6ff0
> frame pointer           = 0x28:0xfffffe1fc28c7090
> code segment            = base 0x0, limit 0xfffff, type 0x1b
>                         = DPL 0, pres 1, long 1, def32 0, gran 1
> processor eflags        = interrupt enabled, resume, IOPL = 0
> current process         = 12 (swi4: clock (0))
> trap number             = 12
> panic: page fault
> cpuid = 0
> time = 1571354026
> KDB: stack backtrace:
> db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame
> 0xfffffe1fc28c6cb0
> vpanic() at vpanic+0x19d/frame 0xfffffe1fc28c6d00
> panic() at panic+0x43/frame 0xfffffe1fc28c6d60
> trap_fatal() at trap_fatal+0x39c/frame 0xfffffe1fc28c6dc0
> trap_pfault() at trap_pfault+0x62/frame 0xfffffe1fc28c6e10
> trap() at trap+0x2b4/frame 0xfffffe1fc28c6f20
> calltrap() at calltrap+0x8/frame 0xfffffe1fc28c6f20
> --- trap 0xc, rip = 0xffffffff80d8c5b2, rsp = 0xfffffe1fc28c6ff0, rbp =
> 0xfffffe1fc28c7090 ---
> icmp6_reflect() at icmp6_reflect+0x242/frame 0xfffffe1fc28c7090
> icmp6_error() at icmp6_error+0x4aa/frame 0xfffffe1fc28c70e0
> frag6_freef() at frag6_freef+0x10e/frame 0xfffffe1fc28c7130
> frag6_slowtimo() at frag6_slowtimo+0x111/frame 0xfffffe1fc28c7180
> pfslowtimo() at pfslowtimo+0x54/frame 0xfffffe1fc28c71b0
> softclock_call_cc() at softclock_call_cc+0x13f/frame 0xfffffe1fc28c7260
> softclock() at softclock+0x7c/frame 0xfffffe1fc28c7290
> ithread_loop() at ithread_loop+0x187/frame 0xfffffe1fc28c72f0
> fork_exit() at fork_exit+0x84/frame 0xfffffe1fc28c7330
> fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe1fc28c7330
> --- trap 0, rip = 0, rsp = 0, rbp = 0 ---
> KDB: enter: panic
> 
> 
> Hmm.. now that I look at that more closely I think it's a separate issue.
> 

KGDB shows this as the spot where it panics:

(kgdb) list /usr/src/sys/netinet6/icmp6.c:2129
2124                            src6 = ia->ia_addr.sin6_addr;
2125                            srcp = &src6;
2126
2127                            if (m->m_pkthdr.rcvif != NULL) {
2128                                    /* XXX: This may not be the outgoing interface */
2129                                    hlim = ND_IFINFO(m->m_pkthdr.rcvif)->chlim;
2130                            } else
2131                                    hlim = V_ip6_defhlim;
2132                    }
2133                    if (ia != NULL)

It looks like a received packet ends up with the stale IFP pointer...

Thanks,
Jake

> > --
> > John Baldwin