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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <eda7bc56-2fb6-4566-aecf-3a18de4029c3@gmail.com>
Date: Sat, 14 Jun 2025 18:33:28 +0200
From: Christian Lamparter <chunkeey@...il.com>
To: Fedor Pchelkin <pchelkin@...ras.ru>, Dmitry Antipov <dmantipov@...dex.ru>
Cc: Christian Lamparter <chunkeey@...glemail.com>,
 linux-wireless@...r.kernel.org, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] wifi: carl9170: do not ping device which has failed to
 load firmware

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@...dex.ru>
>> ---
>>   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