* [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation
@ 2020-08-30 13:27 Bard Liao
2020-08-30 13:27 ` [PATCH v3 1/3] soundwire: add definition for maximum number of ports Bard Liao
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Bard Liao @ 2020-08-30 13:27 UTC (permalink / raw)
To: alsa-devel, vkoul
Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
pierre-louis.bossart, sanyog.r.kale, mengdong.lin, bard.liao
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.
Changes in v3:
- Add ASoC tag in the cover letter title.
- Edit the title and commit message of the third patch for better
understanding.
Changes in v2:
- Split patches into sdw and asoc patches. Please note that "soundwire:
fix port_ready[] dynamic allocation in mipi_disco" and "ASoC: codecs:
fix port_ready[] dynamic allocation in ASoC codecs" should be merged
at the same time.
Pierre-Louis Bossart (3):
soundwire: add definition for maximum number of ports
soundwire: fix port_ready[] dynamic allocation in mipi_disco
ASoC: codecs: soundwire: remove port_ready[] usage from codecs.
drivers/soundwire/mipi_disco.c | 18 +-----------------
drivers/soundwire/slave.c | 4 ++++
include/linux/soundwire/sdw.h | 5 +++--
sound/soc/codecs/max98373-sdw.c | 15 +--------------
sound/soc/codecs/rt1308-sdw.c | 14 +-------------
sound/soc/codecs/rt5682-sdw.c | 15 +--------------
sound/soc/codecs/rt700-sdw.c | 15 +--------------
sound/soc/codecs/rt711-sdw.c | 15 +--------------
sound/soc/codecs/rt715-sdw.c | 33 +--------------------------------
9 files changed, 14 insertions(+), 120 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/3] soundwire: add definition for maximum number of ports
2020-08-30 13:27 [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation Bard Liao
@ 2020-08-30 13:27 ` Bard Liao
2020-08-30 13:27 ` [PATCH v3 2/3] soundwire: fix port_ready[] dynamic allocation in mipi_disco Bard Liao
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Bard Liao @ 2020-08-30 13:27 UTC (permalink / raw)
To: alsa-devel, vkoul
Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
pierre-louis.bossart, sanyog.r.kale, mengdong.lin, bard.liao
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
A Device may have at most 15 physical ports (DP0, DP1..DP14).
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
include/linux/soundwire/sdw.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 76052f12c9f7..0aa4c6af7554 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -38,7 +38,8 @@ struct sdw_slave;
#define SDW_FRAME_CTRL_BITS 48
#define SDW_MAX_DEVICES 11
-#define SDW_VALID_PORT_RANGE(n) ((n) <= 14 && (n) >= 1)
+#define SDW_MAX_PORTS 15
+#define SDW_VALID_PORT_RANGE(n) ((n) < SDW_MAX_PORTS && (n) >= 1)
enum {
SDW_PORT_DIRN_SINK = 0,
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] soundwire: fix port_ready[] dynamic allocation in mipi_disco
2020-08-30 13:27 [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation Bard Liao
2020-08-30 13:27 ` [PATCH v3 1/3] soundwire: add definition for maximum number of ports Bard Liao
@ 2020-08-30 13:27 ` Bard Liao
2020-08-30 13:27 ` [PATCH v3 3/3] ASoC: codecs: soundwire: remove port_ready[] usage from codecs Bard Liao
2020-08-31 15:12 ` [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation Pierre-Louis Bossart
3 siblings, 0 replies; 6+ messages in thread
From: Bard Liao @ 2020-08-30 13:27 UTC (permalink / raw)
To: alsa-devel, vkoul
Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
pierre-louis.bossart, sanyog.r.kale, mengdong.lin, bard.liao
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
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@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] ASoC: codecs: soundwire: remove port_ready[] usage from codecs.
2020-08-30 13:27 [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation Bard Liao
2020-08-30 13:27 ` [PATCH v3 1/3] soundwire: add definition for maximum number of ports Bard Liao
2020-08-30 13:27 ` [PATCH v3 2/3] soundwire: fix port_ready[] dynamic allocation in mipi_disco Bard Liao
@ 2020-08-30 13:27 ` Bard Liao
2020-08-31 15:12 ` [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation Pierre-Louis Bossart
3 siblings, 0 replies; 6+ messages in thread
From: Bard Liao @ 2020-08-30 13:27 UTC (permalink / raw)
To: alsa-devel, vkoul
Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
pierre-louis.bossart, sanyog.r.kale, mengdong.lin, bard.liao
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
As port_ready is now changed to a fixed array in sdw.h, and all
initialization work is done at soundwire side. So remove them from
codec drivers.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
sound/soc/codecs/max98373-sdw.c | 15 +--------------
sound/soc/codecs/rt1308-sdw.c | 14 +-------------
sound/soc/codecs/rt5682-sdw.c | 15 +--------------
sound/soc/codecs/rt700-sdw.c | 15 +--------------
sound/soc/codecs/rt711-sdw.c | 15 +--------------
sound/soc/codecs/rt715-sdw.c | 33 +--------------------------------
6 files changed, 6 insertions(+), 101 deletions(-)
diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c
index 5fe724728e84..a3ec92775ea7 100644
--- a/sound/soc/codecs/max98373-sdw.c
+++ b/sound/soc/codecs/max98373-sdw.c
@@ -282,7 +282,7 @@ static const struct dev_pm_ops max98373_pm = {
static int max98373_read_prop(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = &slave->prop;
- int nval, i, num_of_ports;
+ int nval, i;
u32 bit;
unsigned long addr;
struct sdw_dpn_prop *dpn;
@@ -295,7 +295,6 @@ static int max98373_read_prop(struct sdw_slave *slave)
prop->clk_stop_timeout = 20;
nval = hweight32(prop->source_ports);
- num_of_ports = nval;
prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->src_dpn_prop),
GFP_KERNEL);
@@ -315,7 +314,6 @@ static int max98373_read_prop(struct sdw_slave *slave)
/* do this again for sink now */
nval = hweight32(prop->sink_ports);
- num_of_ports += nval;
prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->sink_dpn_prop),
GFP_KERNEL);
@@ -333,17 +331,6 @@ static int max98373_read_prop(struct sdw_slave *slave)
i++;
}
- /* 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]);
-
/* set the timeout values */
prop->clk_stop_timeout = 20;
diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
index b0ba0d2acbdd..09c69dbab12a 100644
--- a/sound/soc/codecs/rt1308-sdw.c
+++ b/sound/soc/codecs/rt1308-sdw.c
@@ -118,7 +118,7 @@ static int rt1308_clock_config(struct device *dev)
static int rt1308_read_prop(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = &slave->prop;
- int nval, i, num_of_ports = 1;
+ int nval, i;
u32 bit;
unsigned long addr;
struct sdw_dpn_prop *dpn;
@@ -131,7 +131,6 @@ static int rt1308_read_prop(struct sdw_slave *slave)
/* for sink */
nval = hweight32(prop->sink_ports);
- num_of_ports += nval;
prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->sink_dpn_prop),
GFP_KERNEL);
@@ -149,17 +148,6 @@ static int rt1308_read_prop(struct sdw_slave *slave)
i++;
}
- /* 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]);
-
/* set the timeout values */
prop->clk_stop_timeout = 20;
diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c
index 94bf6bee78e6..b7c97aba7f17 100644
--- a/sound/soc/codecs/rt5682-sdw.c
+++ b/sound/soc/codecs/rt5682-sdw.c
@@ -537,7 +537,7 @@ static int rt5682_update_status(struct sdw_slave *slave,
static int rt5682_read_prop(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = &slave->prop;
- int nval, i, num_of_ports = 1;
+ int nval, i;
u32 bit;
unsigned long addr;
struct sdw_dpn_prop *dpn;
@@ -549,7 +549,6 @@ static int rt5682_read_prop(struct sdw_slave *slave)
prop->sink_ports = 0x2; /* BITMAP: 00000010 */
nval = hweight32(prop->source_ports);
- num_of_ports += nval;
prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->src_dpn_prop),
GFP_KERNEL);
@@ -569,7 +568,6 @@ static int rt5682_read_prop(struct sdw_slave *slave)
/* do this again for sink now */
nval = hweight32(prop->sink_ports);
- num_of_ports += nval;
prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->sink_dpn_prop),
GFP_KERNEL);
@@ -587,17 +585,6 @@ static int rt5682_read_prop(struct sdw_slave *slave)
i++;
}
- /* 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]);
-
/* set the timeout values */
prop->clk_stop_timeout = 20;
diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c
index 4d14048d1197..b19fbcc12c69 100644
--- a/sound/soc/codecs/rt700-sdw.c
+++ b/sound/soc/codecs/rt700-sdw.c
@@ -333,7 +333,7 @@ static int rt700_update_status(struct sdw_slave *slave,
static int rt700_read_prop(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = &slave->prop;
- int nval, i, num_of_ports = 1;
+ int nval, i;
u32 bit;
unsigned long addr;
struct sdw_dpn_prop *dpn;
@@ -345,7 +345,6 @@ static int rt700_read_prop(struct sdw_slave *slave)
prop->sink_ports = 0xA; /* BITMAP: 00001010 */
nval = hweight32(prop->source_ports);
- num_of_ports += nval;
prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->src_dpn_prop),
GFP_KERNEL);
@@ -365,7 +364,6 @@ static int rt700_read_prop(struct sdw_slave *slave)
/* do this again for sink now */
nval = hweight32(prop->sink_ports);
- num_of_ports += nval;
prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->sink_dpn_prop),
GFP_KERNEL);
@@ -383,17 +381,6 @@ static int rt700_read_prop(struct sdw_slave *slave)
i++;
}
- /* 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]);
-
/* set the timeout values */
prop->clk_stop_timeout = 20;
diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c
index 45b928954b58..dc4a2b482462 100644
--- a/sound/soc/codecs/rt711-sdw.c
+++ b/sound/soc/codecs/rt711-sdw.c
@@ -337,7 +337,7 @@ static int rt711_update_status(struct sdw_slave *slave,
static int rt711_read_prop(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = &slave->prop;
- int nval, i, num_of_ports = 1;
+ int nval, i;
u32 bit;
unsigned long addr;
struct sdw_dpn_prop *dpn;
@@ -349,7 +349,6 @@ static int rt711_read_prop(struct sdw_slave *slave)
prop->sink_ports = 0x8; /* BITMAP: 00001000 */
nval = hweight32(prop->source_ports);
- num_of_ports += nval;
prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->src_dpn_prop),
GFP_KERNEL);
@@ -369,7 +368,6 @@ static int rt711_read_prop(struct sdw_slave *slave)
/* do this again for sink now */
nval = hweight32(prop->sink_ports);
- num_of_ports += nval;
prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->sink_dpn_prop),
GFP_KERNEL);
@@ -387,17 +385,6 @@ static int rt711_read_prop(struct sdw_slave *slave)
i++;
}
- /* 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]);
-
/* set the timeout values */
prop->clk_stop_timeout = 20;
diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c
index d11b23d6b240..d8ed07305ffc 100644
--- a/sound/soc/codecs/rt715-sdw.c
+++ b/sound/soc/codecs/rt715-sdw.c
@@ -431,7 +431,7 @@ static int rt715_update_status(struct sdw_slave *slave,
static int rt715_read_prop(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = &slave->prop;
- int nval, i, num_of_ports = 1;
+ int nval, i;
u32 bit;
unsigned long addr;
struct sdw_dpn_prop *dpn;
@@ -443,7 +443,6 @@ static int rt715_read_prop(struct sdw_slave *slave)
prop->sink_ports = 0x0; /* BITMAP: 00000000 */
nval = hweight32(prop->source_ports);
- num_of_ports += nval;
prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
sizeof(*prop->src_dpn_prop),
GFP_KERNEL);
@@ -460,36 +459,6 @@ static int rt715_read_prop(struct sdw_slave *slave)
i++;
}
- /* do this again for sink now */
- nval = hweight32(prop->sink_ports);
- num_of_ports += nval;
- prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
- sizeof(*prop->sink_dpn_prop),
- GFP_KERNEL);
- if (!prop->sink_dpn_prop)
- return -ENOMEM;
-
- dpn = prop->sink_dpn_prop;
- i = 0;
- addr = prop->sink_ports;
- for_each_set_bit(bit, &addr, 32) {
- dpn[i].num = bit;
- dpn[i].simple_ch_prep_sm = true;
- dpn[i].ch_prep_timeout = 10;
- i++;
- }
-
- /* 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]);
-
/* set the timeout values */
prop->clk_stop_timeout = 20;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation
2020-08-30 13:27 [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation Bard Liao
` (2 preceding siblings ...)
2020-08-30 13:27 ` [PATCH v3 3/3] ASoC: codecs: soundwire: remove port_ready[] usage from codecs Bard Liao
@ 2020-08-31 15:12 ` Pierre-Louis Bossart
2020-09-01 1:32 ` Bard liao
3 siblings, 1 reply; 6+ messages in thread
From: Pierre-Louis Bossart @ 2020-08-31 15:12 UTC (permalink / raw)
To: Bard Liao, alsa-devel, vkoul
Cc: vinod.koul, tiwai, gregkh, linux-kernel, ranjani.sridharan,
hui.wang, broonie, srinivas.kandagatla, jank, mengdong.lin,
sanyog.r.kale, rander.wang, bard.liao
On 8/30/20 8:27 AM, Bard Liao wrote:
> 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.
Bard, the order of patches will break git bisect, no? I tried on my side
and get the following error after applying patch1
sound/soc/codecs/max98373-sdw.c:337:20: error: assignment to expression
with array type
337 | slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
|
the order should be
ASoC: codecs: soundwire: remove port_ready[] usage from codecs.
soundwire: add definition for maximum number of ports
soundwire: fix port_ready[] dynamic allocation in mipi_disco
> Changes in v3:
> - Add ASoC tag in the cover letter title.
> - Edit the title and commit message of the third patch for better
> understanding.
>
> Changes in v2:
> - Split patches into sdw and asoc patches. Please note that "soundwire:
> fix port_ready[] dynamic allocation in mipi_disco" and "ASoC: codecs:
> fix port_ready[] dynamic allocation in ASoC codecs" should be merged
> at the same time.
>
> Pierre-Louis Bossart (3):
> soundwire: add definition for maximum number of ports
> soundwire: fix port_ready[] dynamic allocation in mipi_disco
> ASoC: codecs: soundwire: remove port_ready[] usage from codecs.
>
> drivers/soundwire/mipi_disco.c | 18 +-----------------
> drivers/soundwire/slave.c | 4 ++++
> include/linux/soundwire/sdw.h | 5 +++--
> sound/soc/codecs/max98373-sdw.c | 15 +--------------
> sound/soc/codecs/rt1308-sdw.c | 14 +-------------
> sound/soc/codecs/rt5682-sdw.c | 15 +--------------
> sound/soc/codecs/rt700-sdw.c | 15 +--------------
> sound/soc/codecs/rt711-sdw.c | 15 +--------------
> sound/soc/codecs/rt715-sdw.c | 33 +--------------------------------
> 9 files changed, 14 insertions(+), 120 deletions(-)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation
2020-08-31 15:12 ` [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation Pierre-Louis Bossart
@ 2020-09-01 1:32 ` Bard liao
0 siblings, 0 replies; 6+ messages in thread
From: Bard liao @ 2020-09-01 1:32 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel, vkoul
Cc: vinod.koul, tiwai, gregkh, linux-kernel, ranjani.sridharan,
hui.wang, broonie, srinivas.kandagatla, jank, mengdong.lin,
sanyog.r.kale, rander.wang, bard.liao
On 8/31/2020 11:12 PM, Pierre-Louis Bossart wrote:
>
>
> On 8/30/20 8:27 AM, Bard Liao wrote:
>> 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.
>
> Bard, the order of patches will break git bisect, no? I tried on my
> side and get the following error after applying patch1
>
> sound/soc/codecs/max98373-sdw.c:337:20: error: assignment to
> expression with array type
> 337 | slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
> |
>
> the order should be
>
> ASoC: codecs: soundwire: remove port_ready[] usage from codecs.
> soundwire: add definition for maximum number of ports
> soundwire: fix port_ready[] dynamic allocation in mipi_disco
Thanks Pierre. I will send a new version soon.
>
>> Changes in v3:
>> - Add ASoC tag in the cover letter title.
>> - Edit the title and commit message of the third patch for better
>> understanding.
>>
>> Changes in v2:
>> - Split patches into sdw and asoc patches. Please note that "soundwire:
>> fix port_ready[] dynamic allocation in mipi_disco" and "ASoC: codecs:
>> fix port_ready[] dynamic allocation in ASoC codecs" should be merged
>> at the same time.
>>
>> Pierre-Louis Bossart (3):
>> soundwire: add definition for maximum number of ports
>> soundwire: fix port_ready[] dynamic allocation in mipi_disco
>> ASoC: codecs: soundwire: remove port_ready[] usage from codecs.
>>
>> drivers/soundwire/mipi_disco.c | 18 +-----------------
>> drivers/soundwire/slave.c | 4 ++++
>> include/linux/soundwire/sdw.h | 5 +++--
>> sound/soc/codecs/max98373-sdw.c | 15 +--------------
>> sound/soc/codecs/rt1308-sdw.c | 14 +-------------
>> sound/soc/codecs/rt5682-sdw.c | 15 +--------------
>> sound/soc/codecs/rt700-sdw.c | 15 +--------------
>> sound/soc/codecs/rt711-sdw.c | 15 +--------------
>> sound/soc/codecs/rt715-sdw.c | 33 +--------------------------------
>> 9 files changed, 14 insertions(+), 120 deletions(-)
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-01 1:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-30 13:27 [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation Bard Liao
2020-08-30 13:27 ` [PATCH v3 1/3] soundwire: add definition for maximum number of ports Bard Liao
2020-08-30 13:27 ` [PATCH v3 2/3] soundwire: fix port_ready[] dynamic allocation in mipi_disco Bard Liao
2020-08-30 13:27 ` [PATCH v3 3/3] ASoC: codecs: soundwire: remove port_ready[] usage from codecs Bard Liao
2020-08-31 15:12 ` [PATCH v3 0/3] ASoC: soundwire: fix port_ready[] dynamic allocation Pierre-Louis Bossart
2020-09-01 1:32 ` Bard liao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).