linux-kernel - Re: [PATCH] ASoC: cs-amp-lib: Use __free(kfree) instead of manual freeing

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: <b3d4e9da-4442-4599-bea9-0d65921fd988@kernel.org>
Date: Mon, 1 Dec 2025 12:40:55 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Richard Fitzgerald <rf@...nsource.cirrus.com>
Cc: linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
 patches@...nsource.cirrus.com, Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH] ASoC: cs-amp-lib: Use __free(kfree) instead of manual
 freeing

On 01/12/2025 12:30, Richard Fitzgerald wrote:
> On 01/12/2025 9:57 am, Richard Fitzgerald wrote:
>> On 29/11/2025 2:28 pm, Krzysztof Kozlowski wrote:
>>> On 27/11/2025 16:58, Richard Fitzgerald wrote:
>>>> Use the __free(kfree) cleanup to replace instances of manually
>>>> calling kfree(). Also make some code path simplifications that this
>>>> allows.
>>>>
>>>> Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
>>>> ---
>>>>   sound/soc/codecs/cs-amp-lib.c | 29 ++++++++++++-----------------
>>>>   1 file changed, 12 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/sound/soc/codecs/cs-amp-lib.c b/sound/soc/codecs/cs-amp- 
>>>> lib.c
>>>> index d8f8b0259cd1..8c9fd9980a7d 100644
>>>> --- a/sound/soc/codecs/cs-amp-lib.c
>>>> +++ b/sound/soc/codecs/cs-amp-lib.c
>>>> @@ -7,6 +7,7 @@
>>>>   #include <asm/byteorder.h>
>>>>   #include <kunit/static_stub.h>
>>>> +#include <linux/cleanup.h>
>>>>   #include <linux/debugfs.h>
>>>>   #include <linux/dev_printk.h>
>>>>   #include <linux/efi.h>
>>>> @@ -309,9 +310,8 @@ static struct cirrus_amp_efi_data 
>>>> *cs_amp_get_cal_efi_buffer(struct device *dev,
>>>>                                    efi_guid_t **guid,
>>>>                                    u32 *attr)
>>>>   {
>>>> -    struct cirrus_amp_efi_data *efi_data;
>>>> +    struct cirrus_amp_efi_data *efi_data __free(kfree) = NULL;
>>>
>>> This is an undesired syntax explicitly documented as one to avoid. You
>>> need here proper assignment, not NULL. Please don't use cleanup.h if you
>>> do not intend to follow it because it does not make the code simpler.
>>>
>>
>> LOL
>> The new system to improve cleanup introduces new cleanup bugs. :)
>>
>>>
>>> Best regards,
>>> Krzysztof
>>
> I found 119 other instances of this _free(kfree) something = NULL; idiom
> in sound/ and ~300 across the whole kernel. So you've got quite some
> code to fix.

In few cases, when allocation is within some if() block, this is a
correct approach but most likely 90% of these 300 are same cases of not
following cleanup.h. And then also mixing it up with gotos (another
explicitly documented rule which people ignore).

Best regards,
Krzysztof