Hi Dmitry, Dmitry Antipov wrote: > Syzkaller reports [1, 2] crashes caused by an attempts to ping > the device which has failed to load firmware. Since such a device > doesn't pass 'ieee80211_register_hw()', an internal workqueue > managed by 'ieee80211_queue_work()' is not yet created and an > attempt to queue work on it causes null-ptr-deref. > > [1] https://syzkaller.appspot.com/bug?extid=9a4aec827829942045ff > [2] https://syzkaller.appspot.com/bug?extid=0d8afba53e8fb2633217 > Fixes: e4a668c59080 ("carl9170: fix spurious restart due to high latency") > Signed-off-by: Dmitry Antipov <dmantipov@xxxxxxxxx> > --- > drivers/net/wireless/ath/carl9170/usb.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/wireless/ath/carl9170/usb.c b/drivers/net/wireless/ath/carl9170/usb.c > index a3e03580cd9f..a0bfa0c477ee 100644 > --- a/drivers/net/wireless/ath/carl9170/usb.c > +++ b/drivers/net/wireless/ath/carl9170/usb.c > @@ -438,13 +438,18 @@ static void carl9170_usb_rx_complete(struct urb *urb) > > if (atomic_read(&ar->rx_anch_urbs) == 0) { > /* > - * The system is too slow to cope with > - * the enormous workload. We have simply > - * run out of active rx urbs and this > - * unfortunately leads to an unpredictable > - * device. > + * At this point, either the system is too slow to > + * cope with the enormous workload (so we have simply > + * run out of active rx urbs and this unfortunately > + * leads to an unpredictable device), or the device > + * is not fully functional after an unsuccessful > + * firmware loading attempts (so it doesn't pass > + * ieee80211_register_hw() and there is no internal > + * workqueue at all). > */ > > + if (WARN_ON_ONCE(!ar->registered)) > + return; Is WARN justifiable here if it concerns handling a predefined error condition? I mean, yeah, it avoids a crash in the completion handler but kernels with panic_on_warn - the ones which Syzkaller runs - will still stumble here for no reason. > ieee80211_queue_work(ar->hw, &ar->ping_work); > } > } else { > -- > 2.49.0
