| Message ID | 20201210152541.191728-1-amadeuszx.slawinski@linux.intel.com |
|---|---|
| State | Accepted |
| Commit | 631c78ed72bbf852cc09b24e4e4e412ed88770f2 |
| Headers | show |
| Series | [1/2] ASoC: topology: Fix wrong size check | expand |
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>
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 --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)
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(-)