kern/118719: 'Giant not owned at ...' with 're' interface

[Dan Lukes]

>Number:         118719
>Category:       kern
>Synopsis:       'Giant not owned at ...' with 're' interface
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Dec 15 13:20:01 UTC 2007
>Closed-Date:
>Last-Modified:
>Originator:     Dan Lukes
>Release:        FreeBSD 6.3-PRERELEASE i386
>Organization:
Obludarium
>Environment:
FreeBSD 6.3-PRERELEASE i386
src/sys/net/if.c,v 1.234.2.21 2007/07/13 01:26:44 thompsa
src/sys/security/mac/mac_net.c,v 1.117 2005/07/05 23:39:50 rwatson
sys/net/bpf.c,v 1.153.2.12 2007/11/03 17:13:16 csjp
src/sys/net/if_ethersubr.c,v 1.193.2.15 2007/09/17 17:50:49 julian
src/sys/net/bpfdesc.h,v 1.29.2.3 2007/01/19 23:01:31 jhb
src/sys/sys/mutex.h,v 1.79.2.4 2006/08/01 18:38:35 jhb
src/sys/dev/re/if_re.c,v 1.46.2.36 2007/12/06 06:01:47 yongari
src/sys/kern/subr_bus.c,v 1.184.2.6 2007/11/05 11:49:44 phk
src/sys/kern/kern_intr.c,v 1.124.2.8 2007/10/29 21:10:03 emaste

Kernel compiled with IPSEC (=> MPSAFE network stack forced disabled; IPSEC require Giant)
Kernel compiled with INVARIANTS and INVARIANT_SUPPORT so missing lock trigger panic

 ****  ****  ****  ****  ****  ****  ****  ****
NOTE: This doesn't apply for CURRENT, but may be significant for 6.3-RELEASE / 6-STABLE
 ****  ****  ****  ****  ****  ****  ****  ****

>Description:
Panic triggered every time ( a packet arrive on 're' interface) AND (a bpf is active)
(for example - dhclient use bpf)

panic: mutex Giant not owned at 
#if MAC option compiled in
	mac_net.c:382
#else
	bpf.c:1345
#fi

On both places it is the BPFD_LOCK_ASSERT macro that call the panic()

NOTE - there seems not to be changes in the if_re.c in past few weeks 
that can cause that problem - so the problem may affect all 
MP_SAFE network card drivers - but I'm didn't test it.

Unfortunatelly, the kernel locks during memory dump, so I have no exact backtrace, but I hope I can reconstruct the possible flow by self.

mac_net.c:382 = mac_check_bpfdesc_receive()
bpf.c:1345 = catchpacket()

they are called from bpf_mtap() or bpf_mtap2() 
they are called from ether_vlan_mtop() which is part of ETHER_BPF_MTAP macro
the macro is used within device driver if_input method
this method is called from receiving interrupt service routine

in the 're' driver is interrupt declared as MP_SAFE so the Giant is not
locked by the core. Is it isn't acquired later as weel, the panic will be 
triggered.

>How-To-Repeat:
Compile kernel with INVARIANTS/INVARIANT_SUPPORT + (IPSEC or set mpsafenet to 0)

use dhclient or tcpdump on 're' interface. Wait for a packet

>Fix:

Workaround is simple:
don't use IPSEC or BPF or use a IFF_NEEDSGIANT devices only

you may also remove INVARIANTS to avoid panic on error, 
but race contition may occur if the Giant is really needed here

Fix:
we need to have Giant acquired in the packet input path, unless someone smarter
than me claimed it's not necesarry to have it - then we need correct the
within bpf's catchpacket() and mac's mac_check_bpfdesc_received routines

The re's bus_setup_intr() is called with INTR_MPSAFE | INTR_FAST flags
The INTR_MPSAFE is cleared by bus_setup_intr's logic, but INTR_FAST remain active.

Then the re's ISR routine will be called without Giant

unfortunatelly, I don know where is exact point of problem - if mpsafenet inactive we need either
 -----------
[1] bus_setup_intr shall clear the INTR_FAST as well

* or *

[2] Giant shall be acquired before calling of driver's ISR routine even if IH_FAST active

* or *

[3] correct the driver's ISR as it is responsible to acquire Giant by self
 -----------

in the later case I would like to note the msk and em drivers may have the same problem 
as they use INTR_FAST also but I didn't tried them

>Release-Note:
>Audit-Trail:
>Unformatted: