Re: [PATCH 05/10] wifi: ath12k: fix SLUB BUG - Object already free in ath12k_reg_free()

Aditya Kumar Singh
2025-01-20T08:38:44Z

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