soundwire: intel: fix memory leak with stream name - #1795
Conversation
We free the stream name on errors but not in the regular case. Fix. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
|
|
||
| dma->hw_params = NULL; | ||
| dma->pdi = NULL; | ||
| kfree(dma->stream->name); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
the 'name' is not allocated as part of the stream allocation
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Catching up with how sdw stack works, but it seems to follow ASoC DAI ops semantics, the alloc/frees are not paired correctly.
|
|
||
| dma->hw_params = NULL; | ||
| dma->pdi = NULL; | ||
| kfree(dma->stream->name); |
There was a problem hiding this comment.
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.
|
SOFCI TEST |
We do not use struct socket in unix_autobind() and pass struct sock to unix_bind_bsd() and unix_bind_abstract(). Let's pass it to unix_autobind() as well. Also, this patch fixes these errors by checkpatch.pl. ERROR: do not use assignment in if condition thesofproject#1795: FILE: net/unix/af_unix.c:1795: + if (test_bit(SOCK_PASSCRED, &sock->flags) && !u->addr CHECK: Logical continuations should be on the previous line thesofproject#1796: FILE: net/unix/af_unix.c:1796: + if (test_bit(SOCK_PASSCRED, &sock->flags) && !u->addr + && (err = unix_autobind(sock)) != 0) Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
6 participants
We free the stream name on errors but not in the regular case. Fix.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com