netinet6: prevent a crash on empty ifp

[franco_opnsense.org]

Summary

Since 000c42faf375 when in6_setscope() is called very likely ifp == NULL
is unhandled which can cause the following panic caused PPPoE connectivity
acquire when IPv6 is activated and not everything is properly connected yet.

KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe0025b12620
vpanic() at vpanic+0x1a2/frame 0xfffffe0025b12670
panic() at panic+0x43/frame 0xfffffe0025b126d0
trap_fatal() at trap_fatal+0x39c/frame 0xfffffe0025b12730
trap_pfault() at trap_pfault+0x49/frame 0xfffffe0025b12790
trap() at trap+0x29f/frame 0xfffffe0025b128a0
calltrap() at calltrap+0x8/frame 0xfffffe0025b128a0

• trap 0xc, rip = 0xffffffff80fa18e6, rsp = 0xfffffe0025b12970, rbp = 0xfffffe0025b129c0 ---

in6_setscope() at in6_setscope+0xa6/frame 0xfffffe0025b129c0
ip6_forward() at ip6_forward+0x359/frame 0xfffffe0025b12b10
pf_test6() at pf_test6+0x1cb5/frame 0xfffffe0025b12ca0
pf_check6_out() at pf_check6_out+0x3f/frame 0xfffffe0025b12cd0
pfil_run_hooks() at pfil_run_hooks+0x87/frame 0xfffffe0025b12d60
ip6_output() at ip6_output+0x1a06/frame 0xfffffe0025b12ff0
icmp6_reflect() at icmp6_reflect+0x2f0/frame 0xfffffe0025b130a0
icmp6_error() at icmp6_error+0x4aa/frame 0xfffffe0025b130f0
ip6_forward() at ip6_forward+0xc58/frame 0xfffffe0025b13240
ip6_input() at ip6_input+0xdf6/frame 0xfffffe0025b13330
netisr_dispatch_src() at netisr_dispatch_src+0xcf/frame 0xfffffe0025b13380
ng_iface_rcvdata() at ng_iface_rcvdata+0x14d/frame 0xfffffe0025b133c0
ng_apply_item() at ng_apply_item+0x2bd/frame 0xfffffe0025b13450
ng_snd_item() at ng_snd_item+0x186/frame 0xfffffe0025b13490
ng_apply_item() at ng_apply_item+0x2bd/frame 0xfffffe0025b13520
ng_snd_item() at ng_snd_item+0x186/frame 0xfffffe0025b13560
ng_apply_item() at ng_apply_item+0x2bd/frame 0xfffffe0025b135f0
ng_snd_item() at ng_snd_item+0x186/frame 0xfffffe0025b13630
ng_pppoe_rcvdata_ether() at ng_pppoe_rcvdata_ether+0x195/frame 0xfffffe0025b136c0
ng_apply_item() at ng_apply_item+0x2bd/frame 0xfffffe0025b13750
ng_snd_item() at ng_snd_item+0x186/frame 0xfffffe0025b13790
ether_demux() at ether_demux+0x207/frame 0xfffffe0025b137c0
ether_nh_input() at ether_nh_input+0x346/frame 0xfffffe0025b13820
netisr_dispatch_src() at netisr_dispatch_src+0xcf/frame 0xfffffe0025b13870
ether_input() at ether_input+0x4b/frame 0xfffffe0025b138a0
vlan_input() at vlan_input+0x1f8/frame 0xfffffe0025b138f0
ether_demux() at ether_demux+0x122/frame 0xfffffe0025b13920
ether_nh_input() at ether_nh_input+0x346/frame 0xfffffe0025b13980
netisr_dispatch_src() at netisr_dispatch_src+0xcf/frame 0xfffffe0025b139d0
ether_input() at ether_input+0x4b/frame 0xfffffe0025b13a00
iflib_rxeof() at iflib_rxeof+0xacb/frame 0xfffffe0025b13ae0
_task_fn_rx() at _task_fn_rx+0xc0/frame 0xfffffe0025b13b20
gtaskqueue_run_locked() at gtaskqueue_run_locked+0x144/frame 0xfffffe0025b13b80
gtaskqueue_thread_loop() at gtaskqueue_thread_loop+0x98/frame 0xfffffe0025b13bb0
fork_exit() at fork_exit+0x83/frame 0xfffffe0025b13bf0
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe0025b13bf0

[glebius]

Is the in6_selectroute() KPI allowed to return success and set ifp to NULL? If it is not allowed, then the fix is incorrect, just hiding a problem.

Franco, do you have a crash core available?

[franco_opnsense.org]

I don't have a crash core and this only happened once on a customer device in FreeBSD 12.

Either way the current code is wrong if ifp cannot be null we shouldn't protect mtu read or we should be careful as handled in the patch. I'm fine with either way :)

When I was investigating this I think the loophole somewhere around https://github.com/freebsd/freebsd-src/blob/e50e40684aa61526327be713de512e0f9934477f/sys/netinet6/in6_src.c#L806 where potentially due to AND condition ifp could be NULL and return value zero and I'm not comfortable changing the network plumbing down there.

[melifaro]

As a part of D35117, I'm working on the changes here that will ensure ifp is non-NULL. Currently writing tests to check variety of possible conditions, hope to publish an updated version a a couple of days.

[franco_opnsense.org]

D35117 looks reasonable, let me abandon this then :)