[PATCH 06/21] soundwire: intel_ace2x: Protect bus.bpt_stream with bus_lock

[Lee Jones]

intel_ace2x_bpt_open_stream() checks and assigns cdns->bus.bpt_stream
without holding cdns->bus.bus_lock.  Concurrent BPT open requests can
both observe a NULL cdns->bus.bpt_stream pointer, allocate separate
streams, and overwrite cdns->bus.bpt_stream, leaking the first stream.
In addition, failures after sdw_slave_bpt_stream_add() jump past
sdw_stream_remove_master(), leaving the master runtime allocated by
sdw_stream_add_slave() on the bus list.

Hold cdns->bus.bus_lock when checking, assigning, and clearing
cdns->bus.bpt_stream, and remove both slave and master stream runtimes
on open failure.

Fixes: b2c9f1d5a7eb ("soundwire: Intel: test bus.bpt_stream before assigning it")
Closes: https://sashiko.dev/#/bug/linux-88535ed9-b7e9-4ede-93c7-59950c524c0b
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
---
 drivers/soundwire/intel_ace2x.c | 36 ++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/soundwire/intel_ace2x.c b/drivers/soundwire/intel_ace2x.c
index 642b33ab5526..3623fc9f4451 100644
--- a/drivers/soundwire/intel_ace2x.c
+++ b/drivers/soundwire/intel_ace2x.c
@@ -83,17 +83,19 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 	int len;
 	int i;
 
-	if (cdns->bus.bpt_stream) {
-		dev_err(cdns->dev, "%s: BPT stream already exists\n", __func__);
-		return -EAGAIN;
+	scoped_guard(mutex, &cdns->bus.bus_lock) {
+		if (cdns->bus.bpt_stream) {
+			dev_err(cdns->dev, "%s: BPT stream already exists\n", __func__);
+			return -EAGAIN;
+		}
+
+		stream = sdw_alloc_stream("BPT", SDW_STREAM_BPT);
+		if (!stream)
+			return -ENOMEM;
+
+		cdns->bus.bpt_stream = stream;
 	}
 
-	stream = sdw_alloc_stream("BPT", SDW_STREAM_BPT);
-	if (!stream)
-		return -ENOMEM;
-
-	cdns->bus.bpt_stream = stream;
-
 	ret = sdw_slave_bpt_stream_add(slave, stream);
 	if (ret < 0)
 		goto release_stream;
@@ -105,7 +107,7 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 	if (!pdi0) {
 		dev_err(cdns->dev, "%s: sdw_cdns_alloc_pdi0 failed\n", __func__);
 		ret = -EINVAL;
-		goto remove_slave;
+		goto remove_master;
 	}
 
 	sdw_cdns_config_stream(cdns, 1, dir, pdi0);
@@ -117,7 +119,7 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 	if (!pdi1) {
 		dev_err(cdns->dev, "%s: sdw_cdns_alloc_pdi1 failed\n", __func__);
 		ret = -EINVAL;
-		goto remove_slave;
+		goto remove_master;
 	}
 
 	sdw_cdns_config_stream(cdns, 1, dir, pdi1);
@@ -136,7 +138,7 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 	pconfig = kzalloc_objs(*pconfig, 2);
 	if (!pconfig) {
 		ret =  -ENOMEM;
-		goto remove_slave;
+		goto remove_master;
 	}
 
 	for (i = 0; i < 2 /* num_pdi */; i++) {
@@ -149,7 +151,7 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 
 	if (ret < 0) {
 		dev_err(cdns->dev, "add master to stream failed:%d\n", ret);
-		goto remove_slave;
+		goto remove_master;
 	}
 
 	ret = sdw_prepare_stream(cdns->bus.bpt_stream);
@@ -293,15 +295,16 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 		dev_err(cdns->dev, "%s: remove master failed: %d\n",
 			__func__, ret1);
 
-remove_slave:
 	ret1 = sdw_stream_remove_slave(slave, cdns->bus.bpt_stream);
 	if (ret1 < 0)
 		dev_err(cdns->dev, "%s: remove slave failed: %d\n",
 			__func__, ret1);
 
 release_stream:
-	sdw_release_stream(cdns->bus.bpt_stream);
-	cdns->bus.bpt_stream = NULL;
+	scoped_guard(mutex, &cdns->bus.bus_lock) {
+		sdw_release_stream(cdns->bus.bpt_stream);
+		cdns->bus.bpt_stream = NULL;
+	}
 
 	return ret;
 }
@@ -335,6 +338,7 @@ static void intel_ace2x_bpt_close_stream(struct sdw_intel *sdw, struct sdw_slave
 		dev_err(cdns->dev, "%s: remove slave failed: %d\n",
 			__func__, ret);
 
+	guard(mutex)(&cdns->bus.bus_lock);
 	sdw_release_stream(cdns->bus.bpt_stream);
 	cdns->bus.bpt_stream = NULL;
 }
-- 
2.56.0.317.g454f8302e1-goog