On Thu, Apr 02, 2020 at 12:40:46PM +0100, Raf Czlonka wrote:
> On Thu, Apr 02, 2020 at 12:30:15PM BST, Stefan Sperling wrote:
> > On Thu, Apr 02, 2020 at 11:43:34AM +0100, Raf Czlonka wrote:
> > > I've just tested it but, whilst I don't get a panic when I *un*plug
> > > the USB NIC, I *do* get the same panic when I *plug* it back in :^(
> >
> > Insufficient detail. Please show the panic.
> >
> > We need to know what you are actually seeing, not your interpretation of
> > what you are seeing. Even if your interpretation happens to be correct,
> > letting us see the same data allows for independent verification.
>
> Hi Stefan,
>
> Sure, better be thorough so here it is:
>
> panic: Data modified on freelist: word 10 of object 0xffff80000099a000
> size 0x2000 previous type devbuf (0xdead410f != 0xdead4110)
>
> I hadn't copy-pasted it - I typed it in form a photo taken this morning.
Thank you.
My suspicion is that ic->ic_bss gets freed in ieee80211_ifdetach(), and
afterwards ic->ic_bss->ni_refcnt gets decremented in zyd_free_tx_list().
Note that (0xdead4110 - 1) == 0xdead410f.
If my theory is correct then when the bug occurs the values of
ic->ic_bss->ni_refcnt are:
before ieee80211_ifdetach(): ic->ic_bss->ni_refcnt == 1
after ieee80211_ifdetach(): ic->ic_bss->ni_refcnt == 0xdead4110
after zyd_free_tx_list(): ic->ic_bss->ni_refcnt == 0xdead410f
Can you please try the following diff?
In addition to the previous diff which fixes a potential use-after-free
when an error happens during Tx of a frame, this changes the order of
operations during detach such that the result should be:
before zyd_free_tx_list(): ic->ic_bss->ni_refcnt == 1
after zyd_free_tx_list(): ic->ic_bss->ni_refcnt == 0
after ieee80211_ifdetach(): ic->ic_bss->ni_refcnt == 0xdead4110
No panic expected since malloc will see the expected pattern written by free.
diff 9d8a60ff1dae84d5c890bdc5040be018a3bdd3dc /usr/src
blob - 61dc357bf69a56d1ce282d505eecd343b0eb2465
file + sys/dev/usb/if_zyd.c
--- sys/dev/usb/if_zyd.c
+++ sys/dev/usb/if_zyd.c
@@ -434,14 +434,14 @@ zyd_detach(struct device *self, int flags)
return 0;
}
+ zyd_free_rx_list(sc);
+ zyd_free_tx_list(sc);
+
if (ifp->if_softc != NULL) {
ieee80211_ifdetach(ifp);
if_detach(ifp);
}
- zyd_free_rx_list(sc);
- zyd_free_tx_list(sc);
-
sc->attached = 0;
splx(s);
@@ -2216,6 +2216,7 @@ zyd_tx(struct zyd_softc *sc, struct mbuf *m, struct ie
ZYD_TX_TIMEOUT, zyd_txeof);
error = usbd_transfer(data->xfer);
if (error != USBD_IN_PROGRESS && error != 0) {
+ data->ni = NULL;
ifp->if_oerrors++;
return EIO;
}
