Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Kalle Valo <kvalo@kernel.org>
To:     Alexander Wetzel <alexander@wetzel-home.de>
Cc:     linux-wireless@vger.kernel.org, stable@vger.kernel.org,
        pa@panix.com
Subject: Re: [PATCH] rtl8180: Prevent using not initialized queues
References: <20220422145228.7567-1-alexander@wetzel-home.de>
        <87a6ccqpzg.fsf@tynnyri.adurom.net>
        <d6b8540f-da10-5103-8321-4c7e37e89314@wetzel-home.de>
Date:   Sat, 23 Apr 2022 12:48:47 +0300
In-Reply-To: <d6b8540f-da10-5103-8321-4c7e37e89314@wetzel-home.de> (Alexander
        Wetzel's message of "Sat, 23 Apr 2022 10:00:48 +0200")
Message-ID: <877d7gm8ow.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Alexander Wetzel <alexander@wetzel-home.de> writes:

> On 23.04.22 08:21, Kalle Valo wrote:
>> Alexander Wetzel <alexander@wetzel-home.de> writes:
>>
>>> Using not existing queues can panic the kernel with rtl8180/rtl8185
>>> cards. Ignore the skb priority for those cards, they only have one
>>> tx queue.
>>>
>>> Cc: stable@vger.kernel.org
>>> Reported-by: pa@panix.com
>>> Tested-by: pa@panix.com
>>> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
>>> ---
>>>
>>> Pierre Asselin (pa@panix.com) reported a kernel crash in the Gentoo forum:
>>> https://forums.gentoo.org/viewtopic-t-1147832-postdays-0-postorder-asc-start-25.html
>>> He also confirmed that this patch fixes the issue.
>>>
>>> In summary this happened:
>>> After updating wpa_supplicant from 2.9 to 2.10 the kernel crashed with a
>>> "divide error: 0000" when connecting to an AP.
>>> Control port tx now tries to use IEEE80211_AC_VO for the priority, which
>>> wpa_supplicants starts to use in 2.10.
>>>
>>> Since only the rtl8187se part of the driver supports QoS, the priority
>>> of the skb is set to IEEE80211_AC_BE (2) by mac80211 for rtl8180/rtl8185
>>> cards.
>>>
>>> rtl8180 is then unconditionally reading out the priority and finally crashes on
>>> drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c line 544 without this
>>> patch:
>>> 	idx = (ring->idx + skb_queue_len(&ring->queue)) % ring->entries
>>>
>>> "ring->entries" is zero for rtl8180/rtl8185 cards, tx_ring[2] never got
>>> initialized.
>>
>> All this after "---" line is very useful information but the actual
>> commit log is just two sentences. I would copy all to the commit log.
>> We don't need to limit the size of the commit log, on the contrary we
>> should include all the information in it.
>>
>
> I see what you mean, fine for me.
> If you prefer I can also make an update but feel to handle that at
> your convenience. If you e.g. see a better way to do that drop the
> patch and simply submit your version.

I can edit the commit log during commit, no need to resubmit because of
this.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches