Re: [PATCH] wifi: carl9170: do not ping device which has failed to load firmware

[Christian Lamparter]

Hi,

On 6/13/25 10:19 PM, Fedor Pchelkin wrote:
> 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?

The driver has many more WARN_ON_(ONCE). Most of them are from "back in the day". I think
carl9170 predates Syzkaller by something like 5 years or less.

In this case, it would be good to know if this only happens with syzkaller, or with some
dogy device (be it the hci, or maybe the ar9170 device itself - they are getting old by now).
I mean Garbage In => Garbage Out. But yes, it shouldn't crash.

> 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.

I bet there is already a precedence for this, if someone knows it or has previous decisions:
Please join in!

Regards,
Christian