netdev - Re: [PATCH 2/2] ath9k: fix ath_tx_process_buffer() potential null ptr dereference

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0fd9a538-e269-e10e-a7f9-02d4c5848420@nbd.name>
Date:   Tue, 16 Feb 2021 08:53:51 +0100
From:   Felix Fietkau <nbd@....name>
To:     Kalle Valo <kvalo@...eaurora.org>,
        Shuah Khan <skhan@...uxfoundation.org>
Cc:     davem@...emloft.net, kuba@...nel.org, ath9k-devel@....qualcomm.com,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] ath9k: fix ath_tx_process_buffer() potential null ptr
 dereference


On 2021-02-16 08:03, Kalle Valo wrote:
> Shuah Khan <skhan@...uxfoundation.org> wrote:
> 
>> ath_tx_process_buffer() references ieee80211_find_sta_by_ifaddr()
>> return pointer (sta) outside null check. Fix it by moving the code
>> block under the null check.
>> 
>> This problem was found while reviewing code to debug RCU warn from
>> ath10k_wmi_tlv_parse_peer_stats_info() and a subsequent manual audit
>> of other callers of ieee80211_find_sta_by_ifaddr() that don't hold
>> RCU read lock.
>> 
>> Signed-off-by: Shuah Khan <skhan@...uxfoundation.org>
>> Signed-off-by: Kalle Valo <kvalo@...eaurora.org>
> 
> Patch applied to ath-next branch of ath.git, thanks.
> 
> a56c14bb21b2 ath9k: fix ath_tx_process_buffer() potential null ptr dereference
I just took another look at this patch, and it is completely bogus.
Not only does the stated reason not make any sense (sta is simply passed
to other functions, not dereferenced without checks), but this also
introduces a horrible memory leak by skipping buffer completion if sta
is NULL.
Please drop it, the code is fine as-is.

- Felix