ASoC: stm32: sai: fix device and OF node leaks on probe

Make sure to drop the reference taken when looking up the sync provider device and its driver data during DAI probe on probe failures and on unbind.

Note that holding a reference to a device does not prevent its driver data from going away so there is no point in keeping the reference.

Fixes 7dd0d835582f ("ASoC: stm32: sai: simplify sync modes management")

Fixes 1c3816a19487 ("ASoC: stm32: sai: add missing put_device()")

Cc [email protected] # 4.16: 1c3816a19487

Cc olivier moysan <[email protected]>

Cc Wen Yang <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
  sound/soc/stm/stm32_sai.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

sound/soc/stm/stm32_sai.c +1 -1

index fa821e3fb427..7065aeb0e524 100644
@@ -143,6 +143,7 @@ static int stm32_sai_set_sync(struct stm32_sai_data *sai_client,
  	}
 
  	sai_provider = platform_get_drvdata(pdev);
+	put_device(&pdev->dev);
  	if (!sai_provider) {
  		dev_err(&sai_client->pdev->dev,
  			"SAI sync provider data not found\n");

@@ -159,7 +160,6 @@ static int stm32_sai_set_sync(struct stm32_sai_data *sai_client,
  	ret = stm32_sai_sync_conf_provider(sai_provider, synco);
 
  error:
-	put_device(&pdev->dev);
  	of_node_put(np_provider);
  	return ret;
  }

Make sure to unprepare the parent clock also on probe failures (e.g. probe deferral).

Fixes a14bf98c045b ("ASoC: stm32: sai: fix possible circular locking")

Cc [email protected] # 5.5

Cc Olivier Moysan <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
  sound/soc/stm/stm32_sai_sub.c | 28 +++++++++++++++++++++-------
  1 file changed, 21 insertions(+), 7 deletions(-)

sound/soc/stm/stm32_sai_sub.c +21 -7

index 0ae1eae2a59e..7a005b4ad304 100644
@@ -1634,14 +1634,21 @@ static int stm32_sai_sub_parse_of(struct platform_device *pdev,
  	if (of_property_present(np, "#clock-cells")) {
  		ret = stm32_sai_add_mclk_provider(sai);
  		if (ret < 0)
-			return ret;
+			goto err_unprepare_pclk;
  	} else {
  		sai->sai_mclk = devm_clk_get_optional(&pdev->dev, "MCLK");
-		if (IS_ERR(sai->sai_mclk))
-			return PTR_ERR(sai->sai_mclk);
+		if (IS_ERR(sai->sai_mclk)) {
+			ret = PTR_ERR(sai->sai_mclk);
+			goto err_unprepare_pclk;
+		}
  	}
 
  	return 0;
+
+err_unprepare_pclk:
+	clk_unprepare(sai->pdata->pclk);
+
+	return ret;
  }
 
  static int stm32_sai_sub_probe(struct platform_device *pdev)

@@ -1688,26 +1695,33 @@ static int stm32_sai_sub_probe(struct platform_device *pdev)
  			       IRQF_SHARED, dev_name(&pdev->dev), sai);
  	if (ret) {
  		dev_err(&pdev->dev, "IRQ request returned %d\n", ret);
-		return ret;
+		goto err_unprepare_pclk;
  	}
 
  	if (STM_SAI_PROTOCOL_IS_SPDIF(sai))
  		conf = &stm32_sai_pcm_config_spdif;
 
  	ret = snd_dmaengine_pcm_register(&pdev->dev, conf, 0);
-	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "Could not register pcm dma\n");
+	if (ret) {
+		ret = dev_err_probe(&pdev->dev, ret, "Could not register pcm dma\n");
+		goto err_unprepare_pclk;
+	}
 
  	ret = snd_soc_register_component(&pdev->dev, &stm32_component,
  					 &sai->cpu_dai_drv, 1);
  	if (ret) {
  		snd_dmaengine_pcm_unregister(&pdev->dev);
-		return ret;
+		goto err_unprepare_pclk;
  	}
 
  	pm_runtime_enable(&pdev->dev);
 
  	return 0;
+
+err_unprepare_pclk:
+	clk_unprepare(sai->pdata->pclk);
+
+	return ret;
  }
 
  static void stm32_sai_sub_remove(struct platform_device *pdev)

The reference taken to the sync provider OF node when probing the platform device is currently only dropped if the set_sync() callback fails during DAI probe.

Make sure to drop the reference on platform probe failures (e.g. probe deferral) and on driver unbind.

This also avoids a potential use-after-free in case the DAI is ever reprobed without first rebinding the platform driver.

Fixes 5914d285f6b7 ("ASoC: stm32: sai: Add synchronization support")

Fixes d4180b4c02e7 ("ASoC: stm32: sai: fix set_sync service")

Cc Olivier Moysan <[email protected]>

Cc [email protected] # 4.16: d4180b4c02e7

Signed-off-by Johan Hovold <[email protected]>

---
  sound/soc/stm/stm32_sai.c     | 12 +++---------
  sound/soc/stm/stm32_sai_sub.c | 23 ++++++++++++++++-------
  2 files changed, 19 insertions(+), 16 deletions(-)

sound/soc/stm/stm32_sai.c +3 -9

index 7065aeb0e524..00cf24ceca2d 100644
@@ -138,7 +138,6 @@ static int stm32_sai_set_sync(struct stm32_sai_data *sai_client,
  	if (!pdev) {
  		dev_err(&sai_client->pdev->dev,
  			"Device not found for node %pOFn\n", np_provider);
-		of_node_put(np_provider);
  		return -ENODEV;
  	}
 

@@ -147,21 +146,16 @@ static int stm32_sai_set_sync(struct stm32_sai_data *sai_client,
  	if (!sai_provider) {
  		dev_err(&sai_client->pdev->dev,
  			"SAI sync provider data not found\n");
-		ret = -EINVAL;
-		goto error;
+		return -EINVAL;
  	}
 
  	/* Configure sync client */
  	ret = stm32_sai_sync_conf_client(sai_client, synci);
  	if (ret < 0)
-		goto error;
+		return ret;
 
  	/* Configure sync provider */
-	ret = stm32_sai_sync_conf_provider(sai_provider, synco);
-
-error:
-	of_node_put(np_provider);
-	return ret;
+	return stm32_sai_sync_conf_provider(sai_provider, synco);
  }
 
  static int stm32_sai_get_parent_clk(struct stm32_sai_data *sai)

sound/soc/stm/stm32_sai_sub.c +16 -7

index 7a005b4ad304..5ae4d2577f28 100644
@@ -1586,7 +1586,8 @@ static int stm32_sai_sub_parse_of(struct platform_device *pdev,
  				dev_err(&pdev->dev,
  					"External synchro not supported\n");
  				of_node_put(args.np);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto err_put_sync_provider;
  			}
  			sai->sync = SAI_SYNC_EXTERNAL;
 

@@ -1595,7 +1596,8 @@ static int stm32_sai_sub_parse_of(struct platform_device *pdev,
  			    (sai->synci > (SAI_GCR_SYNCIN_MAX + 1))) {
  				dev_err(&pdev->dev, "Wrong SAI index\n");
  				of_node_put(args.np);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto err_put_sync_provider;
  			}
 
  			if (of_property_match_string(args.np, "compatible",

@@ -1609,7 +1611,8 @@ static int stm32_sai_sub_parse_of(struct platform_device *pdev,
  			if (!sai->synco) {
  				dev_err(&pdev->dev, "Unknown SAI sub-block\n");
  				of_node_put(args.np);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto err_put_sync_provider;
  			}
  		}
 

@@ -1619,13 +1622,15 @@ static int stm32_sai_sub_parse_of(struct platform_device *pdev,
 
  	of_node_put(args.np);
  	sai->sai_ck = devm_clk_get(&pdev->dev, "sai_ck");
-	if (IS_ERR(sai->sai_ck))
-		return dev_err_probe(&pdev->dev, PTR_ERR(sai->sai_ck),
-				     "Missing kernel clock sai_ck\n");
+	if (IS_ERR(sai->sai_ck)) {
+		ret = dev_err_probe(&pdev->dev, PTR_ERR(sai->sai_ck),
+				    "Missing kernel clock sai_ck\n");
+		goto err_put_sync_provider;
+	}
 
  	ret = clk_prepare(sai->pdata->pclk);
  	if (ret < 0)
-		return ret;
+		goto err_put_sync_provider;
 
  	if (STM_SAI_IS_F4(sai->pdata))
  		return 0;

@@ -1647,6 +1652,8 @@ static int stm32_sai_sub_parse_of(struct platform_device *pdev,
 
  err_unprepare_pclk:
  	clk_unprepare(sai->pdata->pclk);
+err_put_sync_provider:
+	of_node_put(sai->np_sync_provider);
 
  	return ret;
  }

@@ -1720,6 +1727,7 @@ static int stm32_sai_sub_probe(struct platform_device *pdev)
 
  err_unprepare_pclk:
  	clk_unprepare(sai->pdata->pclk);
+	of_node_put(sai->np_sync_provider);
 
  	return ret;
  }

@@ -1732,6 +1740,7 @@ static void stm32_sai_sub_remove(struct platform_device *pdev)
  	snd_dmaengine_pcm_unregister(&pdev->dev);
  	snd_soc_unregister_component(&pdev->dev);
  	pm_runtime_disable(&pdev->dev);
+	of_node_put(sai->np_sync_provider);
  }
 
  static int stm32_sai_sub_suspend(struct device *dev)

Use an error label for deregistering also the PCM DMA for consistency.

Signed-off-by Johan Hovold <[email protected]>

---
  sound/soc/stm/stm32_sai_sub.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

sound/soc/stm/stm32_sai_sub.c +4 -4

index 5ae4d2577f28..450e1585edee 100644
@@ -1716,15 +1716,15 @@ static int stm32_sai_sub_probe(struct platform_device *pdev)
 
  	ret = snd_soc_register_component(&pdev->dev, &stm32_component,
  					 &sai->cpu_dai_drv, 1);
-	if (ret) {
-		snd_dmaengine_pcm_unregister(&pdev->dev);
-		goto err_unprepare_pclk;
-	}
+	if (ret)
+		goto err_deregister_pcm_dma;
 
  	pm_runtime_enable(&pdev->dev);
 
  	return 0;
 
+err_deregister_pcm_dma:
+	snd_dmaengine_pcm_unregister(&pdev->dev);
  err_unprepare_pclk:
  	clk_unprepare(sai->pdata->pclk);
  	of_node_put(sai->np_sync_provider);

This series fixes device and OF node reference leaks during probe and a clock prepare imbalance on probe failures.

Included is a related cleanup of an error path.

Johan

[...]

Quoted 5 lines

This series fixes device and OF node reference leaks during probe and a clock prepare imbalance on probe failures.

Included is a related cleanup of an error path.

Quoted 3 lines

Johan

[...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next