From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: <broonie@kernel.org>
Cc: <lgirdwood@gmail.com>, <cezary.rojewski@intel.com>,
<peter.ujfalusi@linux.intel.com>,
<ranjani.sridharan@linux.intel.com>,
<kai.vehmanen@linux.intel.com>, <yung-chuan.liao@linux.intel.com>,
<pierre-louis.bossart@linux.intel.com>,
<alsa-devel@alsa-project.org>, <patches@opensource.cirrus.com>
Subject: [PATCH 08/12] ASoC: intel: sof_sdw: Move range check of codec_conf into inner loop
Date: Tue, 8 Aug 2023 14:20:09 +0100 [thread overview]
Message-ID: <20230808132013.889419-8-ckeepax@opensource.cirrus.com> (raw)
In-Reply-To: <20230808132013.889419-1-ckeepax@opensource.cirrus.com>
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);
--
2.30.2
next prev parent reply other threads:[~2023-08-08 13:22 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-08 13:20 [PATCH 01/12] ASoC: intel: sof_sdw: Printk's should end with a newline Charles Keepax
2023-08-08 13:20 ` [PATCH 02/12] ASoC: intel: sof_sdw: Remove duplicate NULL check on adr_link Charles Keepax
2023-08-08 13:20 ` [PATCH 03/12] ASoC: intel: sof_sdw: Check link mask validity in get_dailink_info Charles Keepax
2023-08-08 13:20 ` [PATCH 04/12] ASoC: intel: sof-sdw: Move check for valid group id to get_dailink_info Charles Keepax
2023-08-08 13:20 ` [PATCH 05/12] ASoC: intel: sof_sdw: Add helper to create a single codec DLC Charles Keepax
2023-08-08 13:20 ` [PATCH 06/12] ASoC: intel: sof_sdw: Pull device loop up into create_sdw_dailink Charles Keepax
2023-08-08 13:20 ` [PATCH 07/12] ASoC: intel: sof_sdw: Update DLC index each time one is added Charles Keepax
2023-08-08 13:20 ` Charles Keepax [this message]
2023-08-08 13:20 ` [PATCH 09/12] ASoC: intel: sof_sdw: Device loop should not always start at adr_index Charles Keepax
2023-08-08 13:20 ` [PATCH 10/12] ASoC: intel: sof_sdw: Support multiple groups on the same link Charles Keepax
2023-08-08 13:20 ` [PATCH 11/12] ASoC: intel: sof_sdw: Allow different devices " Charles Keepax
2023-08-08 13:20 ` [PATCH 12/12] ASoC: intel: sof_sdw: Simplify get_slave_info Charles Keepax
2023-08-08 20:32 ` [PATCH 01/12] ASoC: intel: sof_sdw: Printk's should end with a newline Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230808132013.889419-8-ckeepax@opensource.cirrus.com \
--to=ckeepax@opensource.cirrus.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=cezary.rojewski@intel.com \
--cc=kai.vehmanen@linux.intel.com \
--cc=lgirdwood@gmail.com \
--cc=patches@opensource.cirrus.com \
--cc=peter.ujfalusi@linux.intel.com \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=ranjani.sridharan@linux.intel.com \
--cc=yung-chuan.liao@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).