linux-kernel - Re: [PATCH 05/10] wifi: ath12k: fix SLUB BUG - Object already free in ath12k_reg

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

Message-ID: <086df6fc-c601-4e06-8433-f87e2bd01ec5@quicinc.com>
Date: Mon, 20 Jan 2025 14:08:32 +0530
From: Aditya Kumar Singh <quic_adisi@...cinc.com>
To: Jeff Johnson <jeff.johnson@....qualcomm.com>,
        Kalle Valo
	<kvalo@...nel.org>, Jeff Johnson <jjohnson@...nel.org>,
        "Karthikeyan
 Periyasamy" <quic_periyasa@...cinc.com>,
        Harshitha Prem
	<quic_hprem@...cinc.com>
CC: Kalle Valo <quic_kvalo@...cinc.com>, <linux-wireless@...r.kernel.org>,
        <ath12k@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 05/10] wifi: ath12k: fix SLUB BUG - Object already free in
 ath12k_reg_free()

On 1/14/25 00:51, Jeff Johnson wrote:
>> diff --git a/drivers/net/wireless/ath/ath12k/reg.c b/drivers/net/wireless/ath/ath12k/reg.c
>> index 439d61f284d89222e79c05d6cff8e85d0d315aad..b4d7fa1a04ca0e72728e8989c29b82d089171fc2 100644
>> --- a/drivers/net/wireless/ath/ath12k/reg.c
>> +++ b/drivers/net/wireless/ath/ath12k/reg.c
>> @@ -1,7 +1,7 @@
>>   // SPDX-License-Identifier: BSD-3-Clause-Clear
>>   /*
>>    * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved.
>> - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>   #include <linux/rtnetlink.h>
>>   #include "core.h"
>> @@ -777,8 +777,14 @@ void ath12k_reg_free(struct ath12k_base *ab)
>>   {
>>   	int i;
>>   
>> +	if (ab->regd_freed)
>> +		return;
>> +
>>   	for (i = 0; i < ab->hw_params->max_radios; i++) {
>>   		kfree(ab->default_regd[i]);
>>   		kfree(ab->new_regd[i]);
>> +		ab->default_regd[i] = NULL;
>> +		ab->new_regd[i] = NULL;
>> +		ab->regd_freed = true;
> since it is loop invariant, should this last assignment be outside the loop,
> either before or after the loop?
> 
> but then again, why is a flag needed since setting the pointers to NULL should
> already show they are freed, and any race conditions with those pointers would
> also exist with the new flag (which you have addressed with the locking change).

Well, looks like, this flag is not needed. I will remove this in next 
version. Thanks for pointing it out!

-- 
Aditya