| Message ID | 20231214072954.3963-1-peter.ujfalusi@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 813c2f2925ee9c10dc4acd5aa7410cd3357e8da8 |
| Headers | show |
| Series | Revert "ASoC: SOF: icp3-dtrace: Fix wrong kfree() usage" | expand |
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; > } >
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; }
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(-)