linux-kernel - [PATCH] ASoC: cs-amp-lib: Replace __free(kfree) with normal kfree() cleanup

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

Message-ID: <20251201123906.86876-1-rf@opensource.cirrus.com>
Date: Mon,  1 Dec 2025 12:39:06 +0000
From: Richard Fitzgerald <rf@...nsource.cirrus.com>
To: broonie@...nel.org
Cc: linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
        patches@...nsource.cirrus.com
Subject: [PATCH] ASoC: cs-amp-lib: Replace __free(kfree) with normal kfree() cleanup

Replace the use of __free(kfree) in _cs_amp_set_efi_calibration_data()
with normal kfree() at the end of the function.

Krzysztof Kozlowski pointed out that __free() can be dangerous.
It can introduce new cleanup bugs. These are more subtle and difficult to
spot than a missing goto in traditional cleanup, because they are triggered
by writing regular idiomatic C code instead of using C++ conventions. As
it's regular C style it's more likely to be missed because the code is as
would be expected for C. The traditional goto also more obviously flags
to anyone changing the code in the future that they must be careful about
the cleanup.

Arguably the traditional approach is more readable, and it avoids the
manipulation of __free() pointers when the buffer is reallocated for
resizing.

Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
---
 sound/soc/codecs/cs-amp-lib.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/cs-amp-lib.c b/sound/soc/codecs/cs-amp-lib.c
index d8f8b0259cd1..b4d183e7501d 100644
--- a/sound/soc/codecs/cs-amp-lib.c
+++ b/sound/soc/codecs/cs-amp-lib.c
@@ -452,7 +452,7 @@ static int _cs_amp_set_efi_calibration_data(struct device *dev, int amp_index, i
 {
 	u64 cal_target = cs_amp_cal_target_u64(in_data);
 	unsigned long num_entries;
-	struct cirrus_amp_efi_data *data __free(kfree) = NULL;
+	struct cirrus_amp_efi_data *data;
 	efi_char16_t *name = CIRRUS_LOGIC_CALIBRATION_EFI_NAME;
 	efi_guid_t *guid = &CIRRUS_LOGIC_CALIBRATION_EFI_GUID;
 	u32 attr = CS_AMP_CAL_DEFAULT_EFI_ATTR;
@@ -515,28 +515,33 @@ static int _cs_amp_set_efi_calibration_data(struct device *dev, int amp_index, i
 
 	num_entries = max(num_amps, amp_index + 1);
 	if (!data || (data->count < num_entries)) {
-		struct cirrus_amp_efi_data *old_data __free(kfree) = no_free_ptr(data);
+		struct cirrus_amp_efi_data *new_data;
 		unsigned int new_data_size = struct_size(data, data, num_entries);
 
-		data = kzalloc(new_data_size, GFP_KERNEL);
-		if (!data)
-			return -ENOMEM;
+		new_data = kzalloc(new_data_size, GFP_KERNEL);
+		if (!new_data) {
+			ret = -ENOMEM;
+			goto err;
+		}
 
-		if (old_data)
-			memcpy(data, old_data, struct_size(old_data, data, old_data->count));
+		if (data) {
+			memcpy(new_data, data, struct_size(data, data, data->count));
+			kfree(data);
+		}
 
+		data = new_data;
 		data->count = num_entries;
 		data->size = new_data_size;
 	}
 
 	data->data[amp_index] = *in_data;
 	ret = cs_amp_set_cal_efi_buffer(dev, name, guid, attr, data);
-	if (ret) {
+	if (ret)
 		dev_err(dev, "Failed writing calibration to EFI: %d\n", ret);
-		return ret;
-	}
+err:
+	kfree(data);
 
-	return 0;
+	return ret;
 }
 
 /**
-- 
2.47.3