linux-kernel - [PATCH v2 5/5] soundwire: bus: Don't exit early if no device IDs were programmed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220907085259.3602-6-rf@opensource.cirrus.com>
Date:   Wed, 7 Sep 2022 09:52:59 +0100
From:   Richard Fitzgerald <rf@...nsource.cirrus.com>
To:     <vkoul@...nel.org>, <yung-chuan.liao@...ux.intel.com>,
        <pierre-louis.bossart@...ux.intel.com>, <sanyog.r.kale@...el.com>
CC:     <alsa-devel@...a-project.org>, <linux-kernel@...r.kernel.org>,
        <patches@...nsource.cirrus.com>,
        Richard Fitzgerald <rf@...nsource.cirrus.com>
Subject: [PATCH v2 5/5] soundwire: bus: Don't exit early if no device IDs were programmed

Only exit sdw_handle_slave_status() right after calling
sdw_program_device_num() if it actually programmed an ID into at
least one device.

sdw_handle_slave_status() should protect itself against phantom
device #0 ATTACHED indications. In that case there is no actual
device still on #0. The early exit relies on there being a status
change to ATTACHED on the reprogrammed device to trigger another
call to sdw_handle_slave_status() which will then handle the status
of all peripherals. If no device was actually programmed with an
ID there won't be a new ATTACHED indication. This can lead to the
status of other peripherals not being handled.

The status passed to sdw_handle_slave_status() is obviously always
from a point of time in the past, and may indicate accumulated
unhandled events (depending how the bus manager operates). It's
possible that a device ID is reprogrammed but the last PING status
captured state just before that, when it was still reporting on
ID #0. Then sdw_handle_slave_status() is called with this PING info,
just before a new PING status is available showing it now on its new
ID. So sdw_handle_slave_status() will receive a phantom report of a
device on #0, but it will not find one.

Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
---
 drivers/soundwire/bus.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 6e569a875a9b..0bcc2d161eb9 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -736,20 +736,19 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 	struct sdw_slave_id id;
 	struct sdw_msg msg;
 	bool found;
-	int count = 0, ret;
+	int count = 0, num_programmed = 0, ret;
 	u64 addr;
 
 	/* No Slave, so use raw xfer api */
 	ret = sdw_fill_msg(&msg, NULL, SDW_SCP_DEVID_0,
 			   SDW_NUM_DEV_ID_REGISTERS, 0, SDW_MSG_FLAG_READ, buf);
 	if (ret < 0)
-		return ret;
+		return 0;
 
 	do {
 		ret = sdw_transfer(bus, &msg);
 		if (ret == -ENODATA) { /* end of device id reads */
 			dev_dbg(bus->dev, "No more devices to enumerate\n");
-			ret = 0;
 			break;
 		}
 		if (ret < 0) {
@@ -781,7 +780,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 				 * assigned a device ID.
 				 */
 				if (slave->status != SDW_SLAVE_UNATTACHED)
-					return 0;
+					return num_programmed;
 
 				/*
 				 * Assign a new dev_num to this Slave and
@@ -794,9 +793,11 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 					dev_err(bus->dev,
 						"Assign dev_num failed:%d\n",
 						ret);
-					return ret;
+					return num_programmed;
 				}
 
+				++num_programmed;
+
 				break;
 			}
 		}
@@ -825,7 +826,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 
 	} while (ret == 0 && count < (SDW_MAX_DEVICES * 2));
 
-	return ret;
+	return num_programmed;
 }
 
 static void sdw_modify_slave_status(struct sdw_slave *slave,
@@ -1787,14 +1788,16 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 
 	if (status[0] == SDW_SLAVE_ATTACHED) {
 		dev_dbg(bus->dev, "Slave attached, programming device number\n");
-		ret = sdw_program_device_num(bus);
-		if (ret < 0)
-			dev_err(bus->dev, "Slave attach failed: %d\n", ret);
+
 		/*
-		 * programming a device number will have side effects,
-		 * so we deal with other devices at a later time
+		 * Programming a device number will have side effects,
+		 * so we deal with other devices at a later time.
+		 * But only if any devices were reprogrammed, because
+		 * this relies on its PING state changing to ATTACHED,
+		 * triggering a status change.
 		 */
-		return ret;
+		if (sdw_program_device_num(bus))
+			return 0;
 	}
 
 	/* Continue to check other slave statuses */
-- 
2.30.2