[PATCH 0/3] ASoC: codecs: rt*-sdw: memory leaks and simplifications

[PATCH 0/3] ASoC: codecs: rt*-sdw: memory leaks and simplifications

[Pierre-Louis Bossart]

While debugging unrelated memory corruption errors, I detected issues
related to the interaction with the SoundWire and ASoC cores, here are
3 small patches to fix all this.

Pierre-Louis Bossart (3):
  ASoC: codecs: rt*-sdw: don't assign slave_ops
  ASoC: codecs: rt*-sdw: fix memory leak in set_sdw_stream()
  ASoC: codecs: rt1308-sdw: remove duplicate allocation

 sound/soc/codecs/rt1308-sdw.c | 11 +++--------
 sound/soc/codecs/rt5682-sdw.c |  3 ---
 sound/soc/codecs/rt5682.c     |  3 +++
 sound/soc/codecs/rt700-sdw.c  |  3 ---
 sound/soc/codecs/rt700.c      |  3 +++
 sound/soc/codecs/rt711-sdw.c  |  3 ---
 sound/soc/codecs/rt711.c      |  3 +++
 sound/soc/codecs/rt715-sdw.c  |  3 ---
 sound/soc/codecs/rt715.c      |  3 +++
 9 files changed, 15 insertions(+), 20 deletions(-)


base-commit: d731c1a0f935dbebf4a851e072f8c7309eb2b8c5
-- 
2.20.1

[PATCH 1/3] ASoC: codecs: rt*-sdw: don't assign slave_ops

[Pierre-Louis Bossart]

The SoundWire bus core already assigns the slave ops, no need to set
them a second time manually in each driver.

Cc: Oder Chiou <oder_chiou@realtek.com>
Cc: Shuming Fan <shumingf@realtek.com>
Cc: Jack Yu <jack.yu@realtek.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
---
 sound/soc/codecs/rt1308-sdw.c | 3 ---
 sound/soc/codecs/rt5682-sdw.c | 3 ---
 sound/soc/codecs/rt700-sdw.c  | 3 ---
 sound/soc/codecs/rt711-sdw.c  | 3 ---
 sound/soc/codecs/rt715-sdw.c  | 3 ---
 5 files changed, 15 deletions(-)

diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
index 1502a22b0d4a..4b88fa8efb27 100644
--- a/sound/soc/codecs/rt1308-sdw.c
+++ b/sound/soc/codecs/rt1308-sdw.c
@@ -684,9 +684,6 @@ static int rt1308_sdw_probe(struct sdw_slave *slave,
 {
 	struct regmap *regmap;
 
-	/* Assign ops */
-	slave->ops = &rt1308_slave_ops;
-
 	/* Regmap Initialization */
 	regmap = devm_regmap_init_sdw(slave, &rt1308_sdw_regmap);
 	if (!regmap)
diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c
index a2d1d3ae1e31..99dd48d2a1d6 100644
--- a/sound/soc/codecs/rt5682-sdw.c
+++ b/sound/soc/codecs/rt5682-sdw.c
@@ -241,9 +241,6 @@ static int rt5682_sdw_probe(struct sdw_slave *slave,
 {
 	struct regmap *regmap;
 
-	/* Assign ops */
-	slave->ops = &rt5682_slave_ops;
-
 	/* Regmap Initialization */
 	regmap = devm_regmap_init_sdw(slave, &rt5682_sdw_regmap);
 	if (IS_ERR(regmap))
diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c
index d4e0f953bcce..4d14048d1197 100644
--- a/sound/soc/codecs/rt700-sdw.c
+++ b/sound/soc/codecs/rt700-sdw.c
@@ -450,9 +450,6 @@ static int rt700_sdw_probe(struct sdw_slave *slave,
 {
 	struct regmap *sdw_regmap, *regmap;
 
-	/* Assign ops */
-	slave->ops = &rt700_slave_ops;
-
 	/* Regmap Initialization */
 	sdw_regmap = devm_regmap_init_sdw(slave, &rt700_sdw_regmap);
 	if (!sdw_regmap)
diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c
index fc3a3fa3d51b..45b928954b58 100644
--- a/sound/soc/codecs/rt711-sdw.c
+++ b/sound/soc/codecs/rt711-sdw.c
@@ -450,9 +450,6 @@ static int rt711_sdw_probe(struct sdw_slave *slave,
 {
 	struct regmap *sdw_regmap, *regmap;
 
-	/* Assign ops */
-	slave->ops = &rt711_slave_ops;
-
 	/* Regmap Initialization */
 	sdw_regmap = devm_regmap_init_sdw(slave, &rt711_sdw_regmap);
 	if (!sdw_regmap)
diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c
index 64ef56ef0318..d11b23d6b240 100644
--- a/sound/soc/codecs/rt715-sdw.c
+++ b/sound/soc/codecs/rt715-sdw.c
@@ -525,9 +525,6 @@ static int rt715_sdw_probe(struct sdw_slave *slave,
 {
 	struct regmap *sdw_regmap, *regmap;
 
-	/* Assign ops */
-	slave->ops = &rt715_slave_ops;
-
 	/* Regmap Initialization */
 	sdw_regmap = devm_regmap_init_sdw(slave, &rt715_sdw_regmap);
 	if (!sdw_regmap)
-- 
2.20.1

[PATCH 2/3] ASoC: codecs: rt*-sdw: fix memory leak in set_sdw_stream()

[Pierre-Louis Bossart]

Now that the sdw_stream is allocated in machine driver,
set_sdw_stream() is also called with a NULL argument during the
dailink shutdown.

In this case, the drivers should not allocate any memory, and just
return.

Detected with KASAN/kmemleak.

Cc: Oder Chiou <oder_chiou@realtek.com>
Cc: Shuming Fan <shumingf@realtek.com>
Cc: Jack Yu <jack.yu@realtek.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
---
 sound/soc/codecs/rt1308-sdw.c | 3 +++
 sound/soc/codecs/rt5682.c     | 3 +++
 sound/soc/codecs/rt700.c      | 3 +++
 sound/soc/codecs/rt711.c      | 3 +++
 sound/soc/codecs/rt715.c      | 3 +++
 5 files changed, 15 insertions(+)

diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
index 4b88fa8efb27..91cc5a15c211 100644
--- a/sound/soc/codecs/rt1308-sdw.c
+++ b/sound/soc/codecs/rt1308-sdw.c
@@ -482,6 +482,9 @@ static int rt1308_set_sdw_stream(struct snd_soc_dai *dai, void *sdw_stream,
 {
 	struct sdw_stream_data *stream;
 
+	if (!sdw_stream)
+		return 0;
+
 	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
 	if (!stream)
 		return -ENOMEM;
diff --git a/sound/soc/codecs/rt5682.c b/sound/soc/codecs/rt5682.c
index 5d3b11756a34..046e6110de73 100644
--- a/sound/soc/codecs/rt5682.c
+++ b/sound/soc/codecs/rt5682.c
@@ -2935,6 +2935,9 @@ static int rt5682_set_sdw_stream(struct snd_soc_dai *dai, void *sdw_stream,
 {
 	struct sdw_stream_data *stream;
 
+	if (!sdw_stream)
+		return 0;
+
 	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
 	if (!stream)
 		return -ENOMEM;
diff --git a/sound/soc/codecs/rt700.c b/sound/soc/codecs/rt700.c
index ff68f0e4f629..687ac2153666 100644
--- a/sound/soc/codecs/rt700.c
+++ b/sound/soc/codecs/rt700.c
@@ -860,6 +860,9 @@ static int rt700_set_sdw_stream(struct snd_soc_dai *dai, void *sdw_stream,
 {
 	struct sdw_stream_data *stream;
 
+	if (!sdw_stream)
+		return 0;
+
 	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
 	if (!stream)
 		return -ENOMEM;
diff --git a/sound/soc/codecs/rt711.c b/sound/soc/codecs/rt711.c
index 2daed7692a3b..65b59dbfb43c 100644
--- a/sound/soc/codecs/rt711.c
+++ b/sound/soc/codecs/rt711.c
@@ -906,6 +906,9 @@ static int rt711_set_sdw_stream(struct snd_soc_dai *dai, void *sdw_stream,
 {
 	struct sdw_stream_data *stream;
 
+	if (!sdw_stream)
+		return 0;
+
 	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
 	if (!stream)
 		return -ENOMEM;
diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c
index 2cbc57b16b13..099c8bd20006 100644
--- a/sound/soc/codecs/rt715.c
+++ b/sound/soc/codecs/rt715.c
@@ -530,6 +530,9 @@ static int rt715_set_sdw_stream(struct snd_soc_dai *dai, void *sdw_stream,
 
 	struct sdw_stream_data *stream;
 
+	if (!sdw_stream)
+		return 0;
+
 	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
 	if (!stream)
 		return -ENOMEM;
-- 
2.20.1

[PATCH 3/3] ASoC: codecs: rt1308-sdw: remove duplicate allocation

[Pierre-Louis Bossart]

The .read_prop callback is supposed to be called by the SoundWire core
only. Calling it again from this driver results in an additional
memory allocation for no good reason.

Cc: Oder Chiou <oder_chiou@realtek.com>
Cc: Shuming Fan <shumingf@realtek.com>
Cc: Jack Yu <jack.yu@realtek.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
---
 sound/soc/codecs/rt1308-sdw.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
index 91cc5a15c211..b0ba0d2acbdd 100644
--- a/sound/soc/codecs/rt1308-sdw.c
+++ b/sound/soc/codecs/rt1308-sdw.c
@@ -178,10 +178,6 @@ static int rt1308_io_init(struct device *dev, struct sdw_slave *slave)
 	if (rt1308->hw_init)
 		return 0;
 
-	ret = rt1308_read_prop(slave);
-	if (ret < 0)
-		goto _io_init_err_;
-
 	if (rt1308->first_hw_init) {
 		regcache_cache_only(rt1308->regmap, false);
 		regcache_cache_bypass(rt1308->regmap, true);
@@ -282,7 +278,6 @@ static int rt1308_io_init(struct device *dev, struct sdw_slave *slave)
 
 	dev_dbg(&slave->dev, "%s hw_init complete\n", __func__);
 
-_io_init_err_:
 	return ret;
 }
 
-- 
2.20.1

Re: [PATCH 0/3] ASoC: codecs: rt*-sdw: memory leaks and simplifications

[Mark Brown]

On Fri, 15 May 2020 16:15:28 -0500, Pierre-Louis Bossart wrote:
> While debugging unrelated memory corruption errors, I detected issues
> related to the interaction with the SoundWire and ASoC cores, here are
> 3 small patches to fix all this.
> 
> Pierre-Louis Bossart (3):
>   ASoC: codecs: rt*-sdw: don't assign slave_ops
>   ASoC: codecs: rt*-sdw: fix memory leak in set_sdw_stream()
>   ASoC: codecs: rt1308-sdw: remove duplicate allocation
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.8

Thanks!

[1/3] ASoC: codecs: rt*-sdw: don't assign slave_ops
      commit: b5dff6ec13260585164d4cd13d7a3ec79bd26acb
[2/3] ASoC: codecs: rt*-sdw: fix memory leak in set_sdw_stream()
      commit: 07b542fe831cbefce163ad1b3aa7292c8a6332b8
[3/3] ASoC: codecs: rt1308-sdw: remove duplicate allocation
      commit: ee5866222ab58531c988492ea54931c1346d4fd4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark