[PATCH v2 2/3] soundwire: fix port_ready[] dynamic allocation in mipi_disco

From: Bard Liao
Date: Fri Aug 21 2020 - 02:20:43 EST


From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>

The existing code allocates memory for the total number of ports.
This only works if the ports are contiguous, but will break if e.g. a
Devices uses port0, 1, and 14. The port_ready[] array would contain 3
elements, which would lead to an out-of-bounds access. Conversely in
other cases, the wrong port index would be used leading to timeouts on
prepare.

This can be fixed by allocating for the worst-case of 15
ports (DP0..DP14). In addition since the number is now fixed, we can
use an array instead of a dynamic allocation.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
Reviewed-by: Rander Wang <rander.wang@xxxxxxxxxxxxxxx>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxxxxxxxx>
Signed-off-by: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>
---
drivers/soundwire/mipi_disco.c | 18 +-----------------
drivers/soundwire/slave.c | 4 ++++
include/linux/soundwire/sdw.h | 2 +-
3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
index 4ae62b452b8c..55a9c51c84c1 100644
--- a/drivers/soundwire/mipi_disco.c
+++ b/drivers/soundwire/mipi_disco.c
@@ -289,7 +289,7 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
struct sdw_slave_prop *prop = &slave->prop;
struct device *dev = &slave->dev;
struct fwnode_handle *port;
- int num_of_ports, nval, i, dp0 = 0;
+ int nval;

device_property_read_u32(dev, "mipi-sdw-sw-interface-revision",
&prop->mipi_revision);
@@ -352,7 +352,6 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
return -ENOMEM;

sdw_slave_read_dp0(slave, port, prop->dp0_prop);
- dp0 = 1;
}

/*
@@ -383,21 +382,6 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
sdw_slave_read_dpn(slave, prop->sink_dpn_prop, nval,
prop->sink_ports, "sink");

- /* some ports are bidirectional so check total ports by ORing */
- nval = prop->source_ports | prop->sink_ports;
- num_of_ports = hweight32(nval) + dp0; /* add DP0 */
-
- /* Allocate port_ready based on num_of_ports */
- slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
- sizeof(*slave->port_ready),
- GFP_KERNEL);
- if (!slave->port_ready)
- return -ENOMEM;
-
- /* Initialize completion */
- for (i = 0; i < num_of_ports; i++)
- init_completion(&slave->port_ready[i]);
-
return 0;
}
EXPORT_SYMBOL(sdw_slave_read_prop);
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index 0839445ee07b..a762ee24e6fa 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -25,6 +25,7 @@ static int sdw_slave_add(struct sdw_bus *bus,
{
struct sdw_slave *slave;
int ret;
+ int i;

slave = kzalloc(sizeof(*slave), GFP_KERNEL);
if (!slave)
@@ -58,6 +59,9 @@ static int sdw_slave_add(struct sdw_bus *bus,
init_completion(&slave->probe_complete);
slave->probed = false;

+ for (i = 0; i < SDW_MAX_PORTS; i++)
+ init_completion(&slave->port_ready[i]);
+
mutex_lock(&bus->bus_lock);
list_add_tail(&slave->node, &bus->slaves);
mutex_unlock(&bus->bus_lock);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 0aa4c6af7554..63e71645fd13 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -619,7 +619,7 @@ struct sdw_slave {
struct dentry *debugfs;
#endif
struct list_head node;
- struct completion *port_ready;
+ struct completion port_ready[SDW_MAX_PORTS];
enum sdw_clk_stop_mode curr_clk_stop_mode;
u16 dev_num;
u16 dev_num_sticky;
--
2.17.1