Re: kern/53233: one-off kernel panic while connecting a urtwn device

[mlelstv%serpens.de@localhost (Michael van Elst)]

The following reply was made to PR kern/53233; it has been noted by GNATS.

From: mlelstv%serpens.de@localhost (Michael van Elst)
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/53233: one-off kernel panic while connecting a urtwn device
Date: Mon, 30 Apr 2018 05:00:11 -0000 (UTC)

 coypu%sdf.org@localhost writes:
 
 >fatal page fault in supervisor mode
 >trap type 6 code 0 rip 0xffffffff80976a36 cs 0x8 rflags 0x10286 cr2 0x8 ilevel 0x6 rsp 0xffff800064f6ae10
 >curlwp 0xffffe40137a09440 pid 0.6 lowest kstack 0xffff800064f672c0
 >panic: trap
 >cpu0: Begin traceback...
 >vpanic() at netbsd:vpanic+0x140
 >snprintf() at netbsd:snprintf
 >startlwp() at netbsd:startlwp
 >alltraps() at netbsd:alltraps+0xb7
 >mutex_vector_enter() at netbsd:mutex_vector_enter+0xc6
 >ieee80211_find_rxnode() at netbsd:ieee80211_find_rxnode+0x3e
 >urtwn_rxeof() at netbsd:urtwn_rxeof+0x29a
 >usb_transfer_complete() at netbsd:usb_transfer_complete+0x146
 >ehci_softintr() at netbsd:ehci_softintr+0x19c
 >usb_soft_intr() at netbsd:usb_soft_intr+0x1f
 >softint_dispatch() at netbsd:softint_dispatch+0xd9
 
 This is probably a race condition between urtwn_attach and something
 setting IFF_UP and affects almost all wifi devices. The attach code does
 
 if_attach(ifp)
 ieee80211_ifattach(ifp)
 ieee80211_media_init(ifp)
 
 Only the last call finishes the initialization and e.g. allocates mutexes
 that are used by ieee80211_find_rxnode. But the ifattach() already makes
 the interface globally visible and lets someone do ioctls.
 
 The better attach sequence might be:
 
 if_initialize(ifp)
 ieee80211_ifattach(ifp)
 ieee80211_media_init(ifp)
 ifp->if_percpuq = if_percpuq_create(ifp);
 if_register(ifp)
 
 The attach routine is supposed to be protected by KERNEL_LOCK. So the
 race can only happen if something between if_attach() and
 ieee80211_media_init() sleeps.
 
 However, that protection is missing in usb_subr.c:
 
 --- usb_subr.c  26 Dec 2017 18:44:52 -0000      1.223
 +++ usb_subr.c  30 Apr 2018 04:56:40 -0000
 @@ -858,7 +858,9 @@
         uaa.uaa_subclass = dd->bDeviceSubClass;
         uaa.uaa_proto = dd->bDeviceProtocol;
  
 +       KERNEL_LOCK(1, curlwp);
         dv = config_found_ia(parent, "usbroothubif", &uaa, 0);
 +       KERNEL_UNLOCK_ONE(curlwp);
         if (dv) {
                 dev->ud_subdevs = kmem_alloc(sizeof(dv), KM_SLEEP);
                 dev->ud_subdevs[0] = dv;
 
 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv%serpens.de@localhost
                                 "A potential Snark may lurk in every tree."