Skip to content

soundwire: intel: fix memory leak with stream name #1795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
plbossart merged 1 commit into from
Feb 18, 2020
Merged

soundwire: intel: fix memory leak with stream name #1795

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions drivers/soundwire/intel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,7 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)

dma->hw_params = NULL;
dma->pdi = NULL;
kfree(dma->stream->name);

@lyakh lyakh Feb 18, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sdw_release_stream() below just does a kfree(stream) so after it dma->stream contains an invalid pointer. Does the dma object get freed after that too? If not, maybe it would be good to set dma->stream = NULL; after calling sdw_release_stream()? In fact, I see that dma gets freed in intel_shutdown(). Both dma and name are allocated in sdw_stream_setup() so, maybe they should also be freed in one function, that's its counterpart? intel_hw_free() seems more like a counterpart to intel_prepare() and intel_hw_params() to me?

@kv2019i kv2019i Feb 18, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, @lyakh you raise a good point. startup() should be a pair with shutdown() and prepare() with hw_free(). So if you follow that, the stream (and the name) should not be free here, but rather in shutdown.

@plbossart plbossart Feb 18, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the 'name' is not allocated as part of the stream allocation

sdw_release_stream(dma->stream);

return 0;
Expand Down