[PATCH] Revert "ASoC: SOF: icp3-dtrace: Fix wrong kfree() usage"

[PATCH] Revert "ASoC: SOF: icp3-dtrace: Fix wrong kfree() usage"

[Peter Ujfalusi]

This reverts commit 8c91ca76f44804868d12aed20ebdbc2f89aa7d60.

The offending patch introduces memory leak when there is no error, the
memory allocated for the temporary storage is not freed up.

As I have commented, the original code was correct and cleaner to
follow but it was not obvious from the patch that it will introduce
regression.

Link: https://lore.kernel.org/all/aec61f67-6b4f-49e6-b458-c332983a0ad6@linux.intel.com/
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/sof/ipc3-dtrace.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/sof/ipc3-dtrace.c b/sound/soc/sof/ipc3-dtrace.c
index 93b189c2d2ee..0dca139322f3 100644
--- a/sound/soc/sof/ipc3-dtrace.c
+++ b/sound/soc/sof/ipc3-dtrace.c
@@ -137,7 +137,6 @@ static int trace_filter_parse(struct snd_sof_dev *sdev, char *string,
 			dev_err(sdev->dev,
 				"Parsing filter entry '%s' failed with %d\n",
 				entry, entry_len);
-			kfree(*out);
 			return -EINVAL;
 		}
 	}
@@ -209,13 +208,13 @@ static ssize_t dfsentry_trace_filter_write(struct file *file, const char __user
 		ret = ipc3_trace_update_filter(sdev, num_elems, elems);
 		if (ret < 0) {
 			dev_err(sdev->dev, "Filter update failed: %d\n", ret);
-			kfree(elems);
 			goto error;
 		}
 	}
 	ret = count;
 error:
 	kfree(string);
+	kfree(elems);
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH] Revert "ASoC: SOF: icp3-dtrace: Fix wrong kfree() usage"

[Péter Ujfalusi]

Hi Mark,

On 14/12/2023 09:29, Peter Ujfalusi wrote:
> This reverts commit 8c91ca76f44804868d12aed20ebdbc2f89aa7d60.
> 
> The offending patch introduces memory leak when there is no error, the
> memory allocated for the temporary storage is not freed up.
> 
> As I have commented, the original code was correct and cleaner to
> follow but it was not obvious from the patch that it will introduce
> regression.

Sorry to dump this, but can you take a look and pick this up?
We are leaking memory in linux-next because of this.

Thanks,
Péter

> Link: https://lore.kernel.org/all/aec61f67-6b4f-49e6-b458-c332983a0ad6@linux.intel.com/
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> ---
>  sound/soc/sof/ipc3-dtrace.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/sound/soc/sof/ipc3-dtrace.c b/sound/soc/sof/ipc3-dtrace.c
> index 93b189c2d2ee..0dca139322f3 100644
> --- a/sound/soc/sof/ipc3-dtrace.c
> +++ b/sound/soc/sof/ipc3-dtrace.c
> @@ -137,7 +137,6 @@ static int trace_filter_parse(struct snd_sof_dev *sdev, char *string,
>  			dev_err(sdev->dev,
>  				"Parsing filter entry '%s' failed with %d\n",
>  				entry, entry_len);
> -			kfree(*out);
>  			return -EINVAL;
>  		}
>  	}
> @@ -209,13 +208,13 @@ static ssize_t dfsentry_trace_filter_write(struct file *file, const char __user
>  		ret = ipc3_trace_update_filter(sdev, num_elems, elems);
>  		if (ret < 0) {
>  			dev_err(sdev->dev, "Filter update failed: %d\n", ret);
> -			kfree(elems);
>  			goto error;
>  		}
>  	}
>  	ret = count;
>  error:
>  	kfree(string);
> +	kfree(elems);
>  	return ret;
>  }
>