[PATCH 1/2] ASoC: topology: Fix wrong size check

[PATCH 1/2] ASoC: topology: Fix wrong size check

[Amadeusz Sławiński]

Dan reported that smatch reports wrong size check and after analysis it
is confirmed that we are comparing wrong value: pointer size instead of
array size. However the check itself is problematic as in UAPI header
there are two fields:

struct snd_soc_tplg_enum_control {
    (...)
    char texts[SND_SOC_TPLG_NUM_TEXTS][SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
    __le32 values[SND_SOC_TPLG_NUM_TEXTS * SNDRV_CTL_ELEM_ID_NAME_MAXLEN / 4];

the texts field is for names and the values one for values assigned to
those named fields, after analysis it becomes clear that there is quite
a lot overhead values than we may possibly name. So instead of changing
check to ARRAY_SIZE(ec->values), as it was first suggested, use
hardcoded value of SND_SOC_TPLG_NUM_TEXTS.

Link: https://lore.kernel.org/alsa-devel/X9B0eDcKy+9B6kZl@mwanda/
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index eb2633dd6454..7fb3a87ab860 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -889,10 +889,16 @@ static int soc_tplg_denum_create_values(struct soc_tplg *tplg, struct soc_enum *
 {
 	int i;
 
-	if (le32_to_cpu(ec->items) > sizeof(*ec->values))
+	/*
+	 * Following "if" checks if we have at most SND_SOC_TPLG_NUM_TEXTS
+	 * values instead of using ARRAY_SIZE(ec->values) due to the fact that
+	 * it is oversized for its purpose. Additionally it is done so because
+	 * it is defined in UAPI header where it can't be easily changed.
+	 */
+	if (le32_to_cpu(ec->items) > SND_SOC_TPLG_NUM_TEXTS)
 		return -EINVAL;
 
-	se->dobj.control.dvalues = devm_kzalloc(tplg->dev, le32_to_cpu(ec->items) *
+	se->dobj.control.dvalues = devm_kcalloc(tplg->dev, le32_to_cpu(ec->items),
 					   sizeof(u32),
 					   GFP_KERNEL);
 	if (!se->dobj.control.dvalues)
-- 
2.25.1

[PATCH 2/2] ASoC: topology: Add missing size check

[Amadeusz Sławiński]

When we parse "values" we perform check if there is correct number of
them. However similar check is missing in case of "texts", add it.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 7fb3a87ab860..950c45008e24 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -856,6 +856,9 @@ static int soc_tplg_denum_create_texts(struct soc_tplg *tplg, struct soc_enum *s
 {
 	int i, ret;
 
+	if (le32_to_cpu(ec->items) > ARRAY_SIZE(ec->texts))
+		return -EINVAL;
+
 	se->dobj.control.dtexts =
 		devm_kcalloc(tplg->dev, le32_to_cpu(ec->items), sizeof(char *), GFP_KERNEL);
 	if (se->dobj.control.dtexts == NULL)
-- 
2.25.1

Re: [PATCH 2/2] ASoC: topology: Add missing size check

[Ranjani Sridharan]

On Thu, 2020-12-10 at 10:25 -0500, Amadeusz Sławiński wrote:
> When we parse "values" we perform check if there is correct number of
> them. However similar check is missing in case of "texts", add it.
> 
> Signed-off-by: Amadeusz Sławiński <
> amadeuszx.slawinski@linux.intel.com>
Both patches look good, Amadeusz!

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Re: [PATCH 1/2] ASoC: topology: Fix wrong size check

[Mark Brown]

On Thu, 10 Dec 2020 10:25:40 -0500, Amadeusz Sławiński wrote:
> Dan reported that smatch reports wrong size check and after analysis it
> is confirmed that we are comparing wrong value: pointer size instead of
> array size. However the check itself is problematic as in UAPI header
> there are two fields:
> 
> struct snd_soc_tplg_enum_control {
>     (...)
>     char texts[SND_SOC_TPLG_NUM_TEXTS][SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
>     __le32 values[SND_SOC_TPLG_NUM_TEXTS * SNDRV_CTL_ELEM_ID_NAME_MAXLEN / 4];
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: topology: Fix wrong size check
      commit: 631c78ed72bbf852cc09b24e4e4e412ed88770f2
[2/2] ASoC: topology: Add missing size check
      commit: f5824e5ce1cdba523a357a4d3ffbe0876a27330f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark