Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <10c35486-55f4-f907-cbe3-c1a33a0710aa@gmail.com>
Date:   Wed, 16 Mar 2022 15:38:06 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.2
Subject: Re: [PATCH] ath11k: acquire ab->base_lock in unassign when finding
 the peer by addr
Content-Language: en-US
To:     Kalle Valo <kvalo@kernel.org>
Cc:     ath11k@lists.infradead.org, linux-wireless@vger.kernel.org,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Carl Huang <cjhuang@codeaurora.org>
References: <20220314215253.92658-1-dossche.niels@gmail.com>
 <87o826pgqr.fsf@kernel.org> <c0874b06-116d-7471-3b50-21099c729c3d@gmail.com>
 <87k0cuotjo.fsf@kernel.org>
From:   Niels Dossche <dossche.niels@gmail.com>
In-Reply-To: <87k0cuotjo.fsf@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 16/03/2022 15:34, Kalle Valo wrote:
> Niels Dossche <dossche.niels@gmail.com> writes:
> 
>> On 3/16/22 07:13, Kalle Valo wrote:
>>> Niels Dossche <dossche.niels@gmail.com> writes:
>>>
>>>> ath11k_peer_find_by_addr states via lockdep that ab->base_lock must be
>>>> held when calling that function in order to protect the list. All
>>>> callers except ath11k_mac_op_unassign_vif_chanctx have that lock
>>>> acquired when calling ath11k_peer_find_by_addr. That lock is also not
>>>> transitively held by a path towards ath11k_mac_op_unassign_vif_chanctx.
>>>> The solution is to acquire the lock when calling
>>>> ath11k_peer_find_by_addr inside ath11k_mac_op_unassign_vif_chanctx.
>>>>
>>>> Fixes: 701e48a43e15 ("ath11k: add packet log support for QCA6390")
>>>> Signed-off-by: Niels Dossche <dossche.niels@gmail.com>
>>>
>>> On what hardware and firmware version did you test this?
>>>
>>
>> Thanks for your reply.
>> I am currently working on a static analyser to detect missing locks.
>> This was a reported case. I manually verified the report by looking
>> at the code, so that I do not send wrong information or patches.
>> After concluding that this seems to be a true positive, I created this patch.
>> However, as I do not in fact have this particular hardware, I was unable to test it.
> 
> Ah, I didn't realise this. If you are using a tool to find errors in the
> code it's always a good idea to document that in the commit log. I'll
> add an edited version of what wrote you above in the commit log, ok?
> 

I will make sure to write that in future commits, sorry for the inconvenience.
Adding an edited version of what I write to the commit log is okay for me, thanks!