diff mbox series

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

Message ID 20201210152541.191728-1-amadeuszx.slawinski@linux.intel.com (mailing list archive)
State Accepted
Commit 631c78ed72bbf852cc09b24e4e4e412ed88770f2
Headers show
Series [1/2] ASoC: topology: Fix wrong size check | expand

Commit Message

Amadeusz Sławiński Dec. 10, 2020, 3:25 p.m. UTC
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(-)

Comments

Mark Brown Dec. 11, 2020, 5:49 p.m. UTC | #1
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
diff mbox series

Patch

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)