panic on invalid ifp pointer in iflib drivers

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

panic on invalid ifp pointer in iflib drivers

Keller, Jacob E
Hi,

I'm investigating an issue on the iflib ixl driver in 11.3-RELEASE as well as 12-RELEASE. We found a panic in that occurs if SCTP/IPv6 traffic is being transmitted while the device is detached:

Fatal trap 12: page fault while in kernel mode
cpuid = 0; apic id = 00
fault virtual address   = 0xfffffe0000411e38
fault code              = supervisor read data, page not present
instruction pointer     = 0x20:0xffffffff80c84700
stack pointer           = 0x28:0xfffffe2f4351b600
frame pointer           = 0x28:0xfffffe2f4351b650
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
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe2f4351b2c0
vpanic() at vpanic+0x17e/frame 0xfffffe2f4351b320
panic() at panic+0x43/frame 0xfffffe2f4351b380
trap_fatal() at trap_fatal+0x369/frame 0xfffffe2f4351b3d0
trap_pfault() at trap_pfault+0x62/frame 0xfffffe2f4351b420
trap() at trap+0x2b3/frame 0xfffffe2f4351b530
calltrap() at calltrap+0x8/frame 0xfffffe2f4351b530
--- trap 0xc, rip = 0xffffffff80c84700, rsp = 0xfffffe2f4351b600, rbp = 0xfffffe2f4351b650 ---
in6_selecthlim() at in6_selecthlim+0x20/frame 0xfffffe2f4351b650
sctp_lowlevel_chunk_output() at sctp_lowlevel_chunk_output+0xeb2/frame 0xfffffe2f4351b790
sctp_chunk_output() at sctp_chunk_output+0x68c/frame 0xfffffe2f4351c110
sctp_timeout_handler() at sctp_timeout_handler+0x2d8/frame 0xfffffe2f4351c180
softclock_call_cc() at softclock_call_cc+0x15b/frame 0xfffffe2f4351c230
softclock() at softclock+0x7c/frame 0xfffffe2f4351c260
intr_event_execute_handlers() at intr_event_execute_handlers+0x9a/frame 0xfffffe2f4351c2a0
ithread_loop() at ithread_loop+0xb7/frame 0xfffffe2f4351c2f0
fork_exit() at fork_exit+0x84/frame 0xfffffe2f4351c330
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe2f4351c330
--- trap 0, rip = 0, rsp = 0, rbp = 0 ---
KDB: enter: panic


From what I've gathered so far, it appears that the issue is a use-after-free where the SCTP stack gets an ifp pointer that's no longer valid. We've reproduced this issue on multiple iflib-based drivers, including ixl and the recently published ice driver code (available on phabricator).

Additionally, we cannot reproduce it on legacy-stack drivers for ixl, or a mellanox 100G board we have. This leads me to believe that it's an issue in iflib rather than in the specific device drivers.

I am not sure exactly what's going wrong here... anyone have suggestions? I thought it might be an issue of when ether_ifdetach is called. That function is supposed to clear all of the pre-existing routes from the route entry list. I'm thinking maybe somehow a route gets added after ether_ifdetach is called.

In the iflib_device_deregister function, ether_ifdetach is called just after iflib_stop, (which would call a device's if_stop routine), and then the task queues are shutdown, a driver's ifdi_detach handler is called, and the ifp is free'd at the end. In the ixl legacy driver, ether_ifdetach is called prior to the stop routine. However, in the mlx5 driver, it's called after a call to close_locked()...

So I'm really not sure exactly what could cause a stale ifp pointer to get into the route entry list.

Thanks,
Jake
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: panic on invalid ifp pointer in iflib drivers

Keller, Jacob E
On Wed, 2019-10-16 at 21:16 +0000, Keller, Jacob E wrote:
> Hi,
>  
> I’m investigating an issue on the iflib ixl driver in 11.3-RELEASE as
> well as 12-RELEASE. We found a panic in that occurs if SCTP/IPv6
> traffic is being transmitted while the device is detached:
>  

I've just been told it has reproduced this on the latest 12-stable as
well.

> Fatal trap 12: page fault while in kernel mode
> cpuid = 0; apic id = 00
> fault virtual address   = 0xfffffe0000411e38
> fault code              = supervisor read data, page not present
> instruction pointer     = 0x20:0xffffffff80c84700
> stack pointer           = 0x28:0xfffffe2f4351b600
> frame pointer           = 0x28:0xfffffe2f4351b650
> 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
> KDB: stack backtrace:
> db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame
> 0xfffffe2f4351b2c0
> vpanic() at vpanic+0x17e/frame 0xfffffe2f4351b320
> panic() at panic+0x43/frame 0xfffffe2f4351b380
> trap_fatal() at trap_fatal+0x369/frame 0xfffffe2f4351b3d0
> trap_pfault() at trap_pfault+0x62/frame 0xfffffe2f4351b420
> trap() at trap+0x2b3/frame 0xfffffe2f4351b530
> calltrap() at calltrap+0x8/frame 0xfffffe2f4351b530
> --- trap 0xc, rip = 0xffffffff80c84700, rsp = 0xfffffe2f4351b600, rbp
> = 0xfffffe2f4351b650 ---
> in6_selecthlim() at in6_selecthlim+0x20/frame 0xfffffe2f4351b650
> sctp_lowlevel_chunk_output() at
> sctp_lowlevel_chunk_output+0xeb2/frame 0xfffffe2f4351b790
> sctp_chunk_output() at sctp_chunk_output+0x68c/frame
> 0xfffffe2f4351c110
> sctp_timeout_handler() at sctp_timeout_handler+0x2d8/frame
> 0xfffffe2f4351c180
> softclock_call_cc() at softclock_call_cc+0x15b/frame
> 0xfffffe2f4351c230
> softclock() at softclock+0x7c/frame 0xfffffe2f4351c260
> intr_event_execute_handlers() at
> intr_event_execute_handlers+0x9a/frame 0xfffffe2f4351c2a0
> ithread_loop() at ithread_loop+0xb7/frame 0xfffffe2f4351c2f0
> fork_exit() at fork_exit+0x84/frame 0xfffffe2f4351c330
> fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe2f4351c330
> --- trap 0, rip = 0, rsp = 0, rbp = 0 ---
> KDB: enter: panic
>  
>  
> From what I’ve gathered so far, it appears that the issue is a use-
> after-free where the SCTP stack gets an ifp pointer that’s no longer
> valid. We’ve reproduced this issue on multiple iflib-based drivers,
> including ixl and the recently published ice driver code (available
> on phabricator).
>
> Additionally, we cannot reproduce it on legacy-stack drivers for ixl,
> or a mellanox 100G board we have. This leads me to believe that it’s
> an issue in iflib rather than in the specific device drivers.
>  
> I am not sure exactly what’s going wrong here... anyone have
> suggestions? I thought it might be an issue of when ether_ifdetach is
> called. That function is supposed to clear all of the pre-existing
> routes from the route entry list. I’m thinking maybe somehow a route
> gets added after ether_ifdetach is called.
>  
> In the iflib_device_deregister function, ether_ifdetach is called
> just after iflib_stop, (which would call a device’s if_stop routine),
> and then the task queues are shutdown, a driver’s ifdi_detach handler
> is called, and the ifp is free’d at the end. In the ixl legacy
> driver, ether_ifdetach is called prior to the stop routine. However,
> in the mlx5 driver, it’s called after a call to close_locked()...
>  
> So I’m really not sure exactly what could cause a stale ifp pointer
> to get into the route entry list.
>  
> Thanks,
> Jake

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: panic on invalid ifp pointer in iflib drivers

John Baldwin
In reply to this post by Keller, Jacob E
On 10/16/19 2:16 PM, Keller, Jacob E wrote:

> Hi,
>
> I'm investigating an issue on the iflib ixl driver in 11.3-RELEASE as well as 12-RELEASE. We found a panic in that occurs if SCTP/IPv6 traffic is being transmitted while the device is detached:
>
> Fatal trap 12: page fault while in kernel mode
> cpuid = 0; apic id = 00
> fault virtual address   = 0xfffffe0000411e38
> fault code              = supervisor read data, page not present
> instruction pointer     = 0x20:0xffffffff80c84700
> stack pointer           = 0x28:0xfffffe2f4351b600
> frame pointer           = 0x28:0xfffffe2f4351b650
> 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
> KDB: stack backtrace:
> db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe2f4351b2c0
> vpanic() at vpanic+0x17e/frame 0xfffffe2f4351b320
> panic() at panic+0x43/frame 0xfffffe2f4351b380
> trap_fatal() at trap_fatal+0x369/frame 0xfffffe2f4351b3d0
> trap_pfault() at trap_pfault+0x62/frame 0xfffffe2f4351b420
> trap() at trap+0x2b3/frame 0xfffffe2f4351b530
> calltrap() at calltrap+0x8/frame 0xfffffe2f4351b530
> --- trap 0xc, rip = 0xffffffff80c84700, rsp = 0xfffffe2f4351b600, rbp = 0xfffffe2f4351b650 ---
> in6_selecthlim() at in6_selecthlim+0x20/frame 0xfffffe2f4351b650
> sctp_lowlevel_chunk_output() at sctp_lowlevel_chunk_output+0xeb2/frame 0xfffffe2f4351b790
> sctp_chunk_output() at sctp_chunk_output+0x68c/frame 0xfffffe2f4351c110
> sctp_timeout_handler() at sctp_timeout_handler+0x2d8/frame 0xfffffe2f4351c180
> softclock_call_cc() at softclock_call_cc+0x15b/frame 0xfffffe2f4351c230
> softclock() at softclock+0x7c/frame 0xfffffe2f4351c260
> intr_event_execute_handlers() at intr_event_execute_handlers+0x9a/frame 0xfffffe2f4351c2a0
> ithread_loop() at ithread_loop+0xb7/frame 0xfffffe2f4351c2f0
> fork_exit() at fork_exit+0x84/frame 0xfffffe2f4351c330
> fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe2f4351c330
> --- trap 0, rip = 0, rsp = 0, rbp = 0 ---
> KDB: enter: panic
>
>
> From what I've gathered so far, it appears that the issue is a use-after-free where the SCTP stack gets an ifp pointer that's no longer valid. We've reproduced this issue on multiple iflib-based drivers, including ixl and the recently published ice driver code (available on phabricator).
>
> Additionally, we cannot reproduce it on legacy-stack drivers for ixl, or a mellanox 100G board we have. This leads me to believe that it's an issue in iflib rather than in the specific device drivers.
>
> I am not sure exactly what's going wrong here... anyone have suggestions? I thought it might be an issue of when ether_ifdetach is called. That function is supposed to clear all of the pre-existing routes from the route entry list. I'm thinking maybe somehow a route gets added after ether_ifdetach is called.
>
> In the iflib_device_deregister function, ether_ifdetach is called just after iflib_stop, (which would call a device's if_stop routine), and then the task queues are shutdown, a driver's ifdi_detach handler is called, and the ifp is free'd at the end. In the ixl legacy driver, ether_ifdetach is called prior to the stop routine. However, in the mlx5 driver, it's called after a call to close_locked()...
>
> So I'm really not sure exactly what could cause a stale ifp pointer to get into the route entry list.

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.  

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.

--
John Baldwin
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

RE: panic on invalid ifp pointer in iflib drivers

Keller, Jacob E
> -----Original Message-----
> From: John Baldwin <[hidden email]>
> Sent: Thursday, October 17, 2019 9:31 AM
> To: Keller, Jacob E <[hidden email]>; [hidden email]
> Cc: [hidden email]; Joyner, Eric <[hidden email]>
> Subject: Re: panic on invalid ifp pointer in iflib drivers
>
> On 10/16/19 2:16 PM, Keller, Jacob E wrote:
> > Hi,
> >
> > I'm investigating an issue on the iflib ixl driver in 11.3-RELEASE as well as 12-
> RELEASE. We found a panic in that occurs if SCTP/IPv6 traffic is being transmitted
> while the device is detached:
> >
> > Fatal trap 12: page fault while in kernel mode
> > cpuid = 0; apic id = 00
> > fault virtual address   = 0xfffffe0000411e38
> > fault code              = supervisor read data, page not present
> > instruction pointer     = 0x20:0xffffffff80c84700
> > stack pointer           = 0x28:0xfffffe2f4351b600
> > frame pointer           = 0x28:0xfffffe2f4351b650
> > 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
> > KDB: stack backtrace:
> > db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame
> 0xfffffe2f4351b2c0
> > vpanic() at vpanic+0x17e/frame 0xfffffe2f4351b320
> > panic() at panic+0x43/frame 0xfffffe2f4351b380
> > trap_fatal() at trap_fatal+0x369/frame 0xfffffe2f4351b3d0
> > trap_pfault() at trap_pfault+0x62/frame 0xfffffe2f4351b420
> > trap() at trap+0x2b3/frame 0xfffffe2f4351b530
> > calltrap() at calltrap+0x8/frame 0xfffffe2f4351b530
> > --- trap 0xc, rip = 0xffffffff80c84700, rsp = 0xfffffe2f4351b600, rbp =
> 0xfffffe2f4351b650 ---
> > in6_selecthlim() at in6_selecthlim+0x20/frame 0xfffffe2f4351b650
> > sctp_lowlevel_chunk_output() at sctp_lowlevel_chunk_output+0xeb2/frame
> 0xfffffe2f4351b790
> > sctp_chunk_output() at sctp_chunk_output+0x68c/frame 0xfffffe2f4351c110
> > sctp_timeout_handler() at sctp_timeout_handler+0x2d8/frame
> 0xfffffe2f4351c180
> > softclock_call_cc() at softclock_call_cc+0x15b/frame 0xfffffe2f4351c230
> > softclock() at softclock+0x7c/frame 0xfffffe2f4351c260
> > intr_event_execute_handlers() at intr_event_execute_handlers+0x9a/frame
> 0xfffffe2f4351c2a0
> > ithread_loop() at ithread_loop+0xb7/frame 0xfffffe2f4351c2f0
> > fork_exit() at fork_exit+0x84/frame 0xfffffe2f4351c330
> > fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe2f4351c330
> > --- trap 0, rip = 0, rsp = 0, rbp = 0 ---
> > KDB: enter: panic
> >
> >
> > From what I've gathered so far, it appears that the issue is a use-after-free
> where the SCTP stack gets an ifp pointer that's no longer valid. We've reproduced
> this issue on multiple iflib-based drivers, including ixl and the recently published
> ice driver code (available on phabricator).
> >
> > Additionally, we cannot reproduce it on legacy-stack drivers for ixl, or a
> mellanox 100G board we have. This leads me to believe that it's an issue in iflib
> rather than in the specific device drivers.
> >
> > I am not sure exactly what's going wrong here... anyone have suggestions? I
> thought it might be an issue of when ether_ifdetach is called. That function is
> supposed to clear all of the pre-existing routes from the route entry list. I'm
> thinking maybe somehow a route gets added after ether_ifdetach is called.
> >
> > In the iflib_device_deregister function, ether_ifdetach is called just after
> iflib_stop, (which would call a device's if_stop routine), and then the task queues
> are shutdown, a driver's ifdi_detach handler is called, and the ifp is free'd at the
> end. In the ixl legacy driver, ether_ifdetach is called prior to the stop routine.
> However, in the mlx5 driver, it's called after a call to close_locked()...
> >
> > So I'm really not sure exactly what could cause a stale ifp pointer to get into the
> route entry list.
>
> 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.
>

So we should (a) move ether_ifdetach before the stop, and...

> Any other references to the ifnet via eventhandlers, etc. should also be
> deregistered before calling the stop routine.
>

(b) make sure all of the event handlers are moved before the stop too.

> 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.
>

Right. I think also the VLAN event handlers need to be moved too.

Let me give that a try out to see if it fixes things.

Thanks,
Jake

> --
> John Baldwin
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

RE: panic on invalid ifp pointer in iflib drivers

Keller, Jacob E
In reply to this post by John Baldwin
> -----Original Message-----
> From: John Baldwin <[hidden email]>
> Sent: Thursday, October 17, 2019 9:31 AM
> To: Keller, Jacob E <[hidden email]>; [hidden email]
> Cc: [hidden email]; Joyner, Eric <[hidden email]>
> 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.

> --
> John Baldwin
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: panic on invalid ifp pointer in iflib drivers

John Baldwin
On 10/17/19 4:22 PM, Keller, Jacob E wrote:
> Hmm.. now that I look at that more closely I think it's a separate issue.

This may just be the rcvif issue where we don't reference count the ifp we
store in rcvif in mbufs?  That was my reaction to your first e-mail except
that you said it wasn't reproducible on some other drivers.  I wonder if
other drivers would also provoke this if you just ran them in a detach/attach
loop long enough.

--
John Baldwin
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

RE: panic on invalid ifp pointer in iflib drivers

Keller, Jacob E
In reply to this post by Keller, Jacob E
> -----Original Message-----
> From: Keller, Jacob E
> Sent: Thursday, October 17, 2019 4:22 PM
> To: John Baldwin <[hidden email]>; [hidden email]
> Cc: [hidden email]; Joyner, Eric <[hidden email]>
> Subject: RE: panic on invalid ifp pointer in iflib drivers
>
> > -----Original Message-----
> > From: John Baldwin <[hidden email]>
> > Sent: Thursday, October 17, 2019 9:31 AM
> > To: Keller, Jacob E <[hidden email]>; [hidden email]
> > Cc: [hidden email]; Joyner, Eric <[hidden email]>
> > 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
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

RE: panic on invalid ifp pointer in iflib drivers

Keller, Jacob E
In reply to this post by John Baldwin
> -----Original Message-----
> From: John Baldwin <[hidden email]>
> Sent: Thursday, October 17, 2019 4:34 PM
> To: Keller, Jacob E <[hidden email]>; [hidden email]
> Cc: [hidden email]; Joyner, Eric <[hidden email]>
> Subject: Re: panic on invalid ifp pointer in iflib drivers
>
> On 10/17/19 4:22 PM, Keller, Jacob E wrote:
> > Hmm.. now that I look at that more closely I think it's a separate issue.
>
> This may just be the rcvif issue where we don't reference count the ifp we
> store in rcvif in mbufs?  That was my reaction to your first e-mail except
> that you said it wasn't reproducible on some other drivers.  I wonder if
> other drivers would also provoke this if you just ran them in a detach/attach
> loop long enough.
>

This is almost certainly the rcvif issue based on where it panics.

I never saw this panic before. I 've also only reproduced it on a kernel with INVARIANTS and memguard set to ifnet.

Thanks,
Jake

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[hidden email]"