There are two problems with the current range check on the codec_conf array.
Firstly, adr_link_next->num_adr refers to the number of devices on the current SoundWire link, but adr_index refers to the first SoundWire link involved in the DAI link. This means that subtracting these two numbers is only meaningful on the first SoundWire link in the DAI and broken on later links.
Secondly, the intention of the range check is to add the number of remaining devices on the currently link to the current index and ensure enough space remains. However, this assumes that all remaining devices on the SoundWire link will be added to the current DAI link. Ideally this would not be the case, and devices could be grouped as the user desired.
Moving the range check into the inner loop both simplifies the code (no need to add and subtract offsets) and allows future refactoring such that devices on a single SoundWire link don't have to all be grouped onto a single DAI link. The check will be processed slightly more often since it is processed for each device rather each link but this is probe time and the numbers involved are very small here (4 links, likely no more than 2-4 devices per link).
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/intel/boards/sof_sdw.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index b381fb2619943..0401516f35de6 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -1380,12 +1380,6 @@ static int create_sdw_dailink(struct snd_soc_card *card, int *link_index, if (cpu_dai_id[i] != ffs(adr_link_next->mask) - 1) continue;
- /* sanity check */ - if (*codec_conf_index + adr_link_next->num_adr - adr_index > codec_count) { - dev_err(dev, "codec_conf: out-of-bounds access requested\n"); - return -EINVAL; - } - for (j = adr_index; j < adr_link_next->num_adr; j++) { int codec_index; u64 adr = adr_link_next->adr_d[j].adr; @@ -1399,6 +1393,12 @@ static int create_sdw_dailink(struct snd_soc_card *card, int *link_index, } _codec_index = codec_index;
+ /* sanity check */ + if (*codec_conf_index >= codec_count) { + dev_err(dev, "codec_conf array overflowed\n"); + return -EINVAL; + } + ret = fill_sdw_codec_dlc(dev, adr_link_next, &codecs[codec_dlc_index], codec_index, j, dai_index);