Blob Blame History Raw
From 631c78ed72bbf852cc09b24e4e4e412ed88770f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Amadeusz=20S=C5=82awi=C5=84ski?= <amadeuszx.slawinski@linux.intel.com>
Date: Thu, 10 Dec 2020 10:25:40 -0500
Subject: [PATCH] ASoC: topology: Fix wrong size check
Mime-version: 1.0
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8bit
Git-commit: 631c78ed72bbf852cc09b24e4e4e412ed88770f2
Patch-mainline: v5.11-rc1
References: jsc#SLE-16518

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>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/20201210152541.191728-1-amadeuszx.slawinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Acked-by: Takashi Iwai <tiwai@suse.de>

---
 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 e85b52798bc5..950c45008e24 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -892,10 +892,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.26.2