Re: [PATCH v2 3/6] ASoC: topology: Check return value of soc_tplg_*_create

Re: [PATCH v2 3/6] ASoC: topology: Check return value of soc_tplg_*_create

[Pierre-Louis Bossart]

On 3/27/20 3:47 PM, Amadeusz Sławiński wrote:
> Functions soc_tplg_denum_create, soc_tplg_dmixer_create,
> soc_tplg_dbytes_create can fail, so their return values should be
> checked and error should be propagated.
> 
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> ---
> 
>   v2:
>    Added this patch
> 
>   sound/soc/soc-topology.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
> index c3811dd66b68..860bced933d6 100644
> --- a/sound/soc/soc-topology.c
> +++ b/sound/soc/soc-topology.c
> @@ -1124,6 +1124,7 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
>   	struct snd_soc_tplg_hdr *hdr)
>   {
>   	struct snd_soc_tplg_ctl_hdr *control_hdr;
> +	int ret;
>   	int i;
>   
>   	if (tplg->pass != SOC_TPLG_PASS_MIXER) {
> @@ -1152,25 +1153,30 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
>   		case SND_SOC_TPLG_CTL_RANGE:
>   		case SND_SOC_TPLG_DAPM_CTL_VOLSW:
>   		case SND_SOC_TPLG_DAPM_CTL_PIN:
> -			soc_tplg_dmixer_create(tplg, 1,
> -					       le32_to_cpu(hdr->payload_size));
> +			ret = soc_tplg_dmixer_create(tplg, 1,
> +					le32_to_cpu(hdr->payload_size));
>   			break;
>   		case SND_SOC_TPLG_CTL_ENUM:
>   		case SND_SOC_TPLG_CTL_ENUM_VALUE:
>   		case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE:
>   		case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT:
>   		case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE:
> -			soc_tplg_denum_create(tplg, 1,
> -					      le32_to_cpu(hdr->payload_size));
> +			ret = soc_tplg_denum_create(tplg, 1,
> +					le32_to_cpu(hdr->payload_size));
>   			break;
>   		case SND_SOC_TPLG_CTL_BYTES:
> -			soc_tplg_dbytes_create(tplg, 1,
> -					       le32_to_cpu(hdr->payload_size));
> +			ret = soc_tplg_dbytes_create(tplg, 1,
> +					le32_to_cpu(hdr->payload_size));
>   			break;
>   		default:
>   			soc_bind_err(tplg, control_hdr, i);
>   			return -EINVAL;
>   		}
> +		if (ret < 0) {
> +			dev_err(tplg->dev, "ASoC: invalid control\n");
> +			return ret;
> +		}

Sounds good, but this happens in a loop, so would all the memory 
previously allocated by denum/dbytes/dmixer_create leak, or is it freed 
automatically somewhere else?

> +
>   	}
>   
>   	return 0;
> 

[PATCH v2 0/6] ASoC: topology: Propagate error appropriately

[Amadeusz Sławiński]

v1: 
  Check if kstrdup succeeded.

v2:
  Remove unneeded freeing, which is performed in another place by dobj
  handlers.

  Additionally for functions which have return status which was ignored,
  perform success checks and handle failures in appropriate way.

Amadeusz Sławiński (6):
  ASoC: topology: Add missing memory checks
  ASoC: topology: Check return value of soc_tplg_create_tlv
  ASoC: topology: Check return value of soc_tplg_*_create
  ASoC: topology: Check soc_tplg_add_route return value
  ASoC: topology: Check return value of pcm_new_ver
  ASoC: topology: Check return value of soc_tplg_dai_config

 sound/soc/soc-topology.c | 113 ++++++++++++++++++++++++++++++---------
 1 file changed, 88 insertions(+), 25 deletions(-)

-- 
2.17.1

[PATCH v2 1/6] ASoC: topology: Add missing memory checks

[Amadeusz Sławiński]

kstrdup is an allocation function and it can fail, so its return value
should be checked and handled appropriately.

In order to check all cases, we need to modify set_stream_info to return
a value, so check that everything went correctly when doing kstrdup().
Later add proper checks and error handlers.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---

 v2:
  Remove unneeded freeing, which is performed in another place by dobj
  handlers.

 sound/soc/soc-topology.c | 62 +++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 13 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 1f81cd2d29cf..434e152ac8ee 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1766,10 +1766,13 @@ static int soc_tplg_dapm_complete(struct soc_tplg *tplg)
 	return 0;
 }
 
-static void set_stream_info(struct snd_soc_pcm_stream *stream,
+static int set_stream_info(struct snd_soc_pcm_stream *stream,
 	struct snd_soc_tplg_stream_caps *caps)
 {
 	stream->stream_name = kstrdup(caps->name, GFP_KERNEL);
+	if (!stream->stream_name)
+		return -ENOMEM;
+
 	stream->channels_min = le32_to_cpu(caps->channels_min);
 	stream->channels_max = le32_to_cpu(caps->channels_max);
 	stream->rates = le32_to_cpu(caps->rates);
@@ -1777,6 +1780,8 @@ static void set_stream_info(struct snd_soc_pcm_stream *stream,
 	stream->rate_max = le32_to_cpu(caps->rate_max);
 	stream->formats = le64_to_cpu(caps->formats);
 	stream->sig_bits = le32_to_cpu(caps->sig_bits);
+
+	return 0;
 }
 
 static void set_dai_flags(struct snd_soc_dai_driver *dai_drv,
@@ -1812,20 +1817,29 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	if (dai_drv == NULL)
 		return -ENOMEM;
 
-	if (strlen(pcm->dai_name))
+	if (strlen(pcm->dai_name)) {
 		dai_drv->name = kstrdup(pcm->dai_name, GFP_KERNEL);
+		if (!dai_drv->name) {
+			ret = -ENOMEM;
+			goto err;
+		}
+	}
 	dai_drv->id = le32_to_cpu(pcm->dai_id);
 
 	if (pcm->playback) {
 		stream = &dai_drv->playback;
 		caps = &pcm->caps[SND_SOC_TPLG_STREAM_PLAYBACK];
-		set_stream_info(stream, caps);
+		ret = set_stream_info(stream, caps);
+		if (ret < 0)
+			goto err;
 	}
 
 	if (pcm->capture) {
 		stream = &dai_drv->capture;
 		caps = &pcm->caps[SND_SOC_TPLG_STREAM_CAPTURE];
-		set_stream_info(stream, caps);
+		ret = set_stream_info(stream, caps);
+		if (ret < 0)
+			goto err;
 	}
 
 	if (pcm->compress)
@@ -1835,11 +1849,7 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	ret = soc_tplg_dai_load(tplg, dai_drv, pcm, NULL);
 	if (ret < 0) {
 		dev_err(tplg->comp->dev, "ASoC: DAI loading failed\n");
-		kfree(dai_drv->playback.stream_name);
-		kfree(dai_drv->capture.stream_name);
-		kfree(dai_drv->name);
-		kfree(dai_drv);
-		return ret;
+		goto err;
 	}
 
 	dai_drv->dobj.index = tplg->index;
@@ -1860,6 +1870,14 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 		return ret;
 	}
 
+	return 0;
+
+err:
+	kfree(dai_drv->playback.stream_name);
+	kfree(dai_drv->capture.stream_name);
+	kfree(dai_drv->name);
+	kfree(dai_drv);
+
 	return ret;
 }
 
@@ -1916,11 +1934,20 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg,
 	if (strlen(pcm->pcm_name)) {
 		link->name = kstrdup(pcm->pcm_name, GFP_KERNEL);
 		link->stream_name = kstrdup(pcm->pcm_name, GFP_KERNEL);
+		if (!link->name || !link->stream_name) {
+			ret = -ENOMEM;
+			goto err;
+		}
 	}
 	link->id = le32_to_cpu(pcm->pcm_id);
 
-	if (strlen(pcm->dai_name))
+	if (strlen(pcm->dai_name)) {
 		link->cpus->dai_name = kstrdup(pcm->dai_name, GFP_KERNEL);
+		if (!link->cpus->dai_name) {
+			ret = -ENOMEM;
+			goto err;
+		}
+	}
 
 	link->codecs->name = "snd-soc-dummy";
 	link->codecs->dai_name = "snd-soc-dummy-dai";
@@ -2436,13 +2463,17 @@ static int soc_tplg_dai_config(struct soc_tplg *tplg,
 	if (d->playback) {
 		stream = &dai_drv->playback;
 		caps = &d->caps[SND_SOC_TPLG_STREAM_PLAYBACK];
-		set_stream_info(stream, caps);
+		ret = set_stream_info(stream, caps);
+		if (ret < 0)
+			goto err;
 	}
 
 	if (d->capture) {
 		stream = &dai_drv->capture;
 		caps = &d->caps[SND_SOC_TPLG_STREAM_CAPTURE];
-		set_stream_info(stream, caps);
+		ret = set_stream_info(stream, caps);
+		if (ret < 0)
+			goto err;
 	}
 
 	if (d->flag_mask)
@@ -2454,10 +2485,15 @@ static int soc_tplg_dai_config(struct soc_tplg *tplg,
 	ret = soc_tplg_dai_load(tplg, dai_drv, NULL, dai);
 	if (ret < 0) {
 		dev_err(tplg->comp->dev, "ASoC: DAI loading failed\n");
-		return ret;
+		goto err;
 	}
 
 	return 0;
+
+err:
+	kfree(dai_drv->playback.stream_name);
+	kfree(dai_drv->capture.stream_name);
+	return ret;
 }
 
 /* load physical DAI elements */
-- 
2.17.1

[PATCH v2 2/6] ASoC: topology: Check return value of soc_tplg_create_tlv

[Amadeusz Sławiński]

Function soc_tplg_create_tlv can fail, so we should check if it succeded
or not and proceed appropriately.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---

 v2:
  Added this patch

 sound/soc/soc-topology.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 434e152ac8ee..c3811dd66b68 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -894,7 +894,13 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
 		}
 
 		/* create any TLV data */
-		soc_tplg_create_tlv(tplg, &kc, &mc->hdr);
+		err = soc_tplg_create_tlv(tplg, &kc, &mc->hdr);
+		if (err < 0) {
+			dev_err(tplg->dev, "ASoC: failed to create TLV %s\n",
+				mc->hdr.name);
+			kfree(sm);
+			continue;
+		}
 
 		/* pass control to driver for optional further init */
 		err = soc_tplg_init_kcontrol(tplg, &kc,
@@ -1355,7 +1361,13 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dmixer_create(
 		}
 
 		/* create any TLV data */
-		soc_tplg_create_tlv(tplg, &kc[i], &mc->hdr);
+		err = soc_tplg_create_tlv(tplg, &kc[i], &mc->hdr);
+		if (err < 0) {
+			dev_err(tplg->dev, "ASoC: failed to create TLV %s\n",
+				mc->hdr.name);
+			kfree(sm);
+			continue;
+		}
 
 		/* pass control to driver for optional further init */
 		err = soc_tplg_init_kcontrol(tplg, &kc[i],
-- 
2.17.1

[PATCH v2 3/6] ASoC: topology: Check return value of soc_tplg_*_create

[Amadeusz Sławiński]

Functions soc_tplg_denum_create, soc_tplg_dmixer_create,
soc_tplg_dbytes_create can fail, so their return values should be
checked and error should be propagated.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---

 v2:
  Added this patch

 sound/soc/soc-topology.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index c3811dd66b68..860bced933d6 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1124,6 +1124,7 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
 	struct snd_soc_tplg_hdr *hdr)
 {
 	struct snd_soc_tplg_ctl_hdr *control_hdr;
+	int ret;
 	int i;
 
 	if (tplg->pass != SOC_TPLG_PASS_MIXER) {
@@ -1152,25 +1153,30 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
 		case SND_SOC_TPLG_CTL_RANGE:
 		case SND_SOC_TPLG_DAPM_CTL_VOLSW:
 		case SND_SOC_TPLG_DAPM_CTL_PIN:
-			soc_tplg_dmixer_create(tplg, 1,
-					       le32_to_cpu(hdr->payload_size));
+			ret = soc_tplg_dmixer_create(tplg, 1,
+					le32_to_cpu(hdr->payload_size));
 			break;
 		case SND_SOC_TPLG_CTL_ENUM:
 		case SND_SOC_TPLG_CTL_ENUM_VALUE:
 		case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE:
 		case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT:
 		case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE:
-			soc_tplg_denum_create(tplg, 1,
-					      le32_to_cpu(hdr->payload_size));
+			ret = soc_tplg_denum_create(tplg, 1,
+					le32_to_cpu(hdr->payload_size));
 			break;
 		case SND_SOC_TPLG_CTL_BYTES:
-			soc_tplg_dbytes_create(tplg, 1,
-					       le32_to_cpu(hdr->payload_size));
+			ret = soc_tplg_dbytes_create(tplg, 1,
+					le32_to_cpu(hdr->payload_size));
 			break;
 		default:
 			soc_bind_err(tplg, control_hdr, i);
 			return -EINVAL;
 		}
+		if (ret < 0) {
+			dev_err(tplg->dev, "ASoC: invalid control\n");
+			return ret;
+		}
+
 	}
 
 	return 0;
-- 
2.17.1

[PATCH v2 4/6] ASoC: topology: Check soc_tplg_add_route return value

[Amadeusz Sławiński]

Function soc_tplg_add_route can propagate error code from callback, we
should check its return value and handle fail in correct way.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---

 v2:
  Added this patch

 sound/soc/soc-topology.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 860bced933d6..78d10b9b725b 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1284,7 +1284,9 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
 		routes[i]->dobj.index = tplg->index;
 		list_add(&routes[i]->dobj.list, &tplg->comp->dobj_list);
 
-		soc_tplg_add_route(tplg, routes[i]);
+		ret = soc_tplg_add_route(tplg, routes[i]);
+		if (ret < 0)
+			break;
 
 		/* add route, but keep going if some fail */
 		snd_soc_dapm_add_routes(dapm, routes[i], 1);
-- 
2.17.1

[PATCH v2 5/6] ASoC: topology: Check return value of pcm_new_ver

[Amadeusz Sławiński]

Function pcm_new_ver can fail, so we should check it's return value and
handle possible error.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---

 v2:
  Added this patch

 sound/soc/soc-topology.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 78d10b9b725b..dade27588e51 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2135,7 +2135,9 @@ static int soc_tplg_pcm_elems_load(struct soc_tplg *tplg,
 			_pcm = pcm;
 		} else {
 			abi_match = false;
-			pcm_new_ver(tplg, pcm, &_pcm);
+			ret = pcm_new_ver(tplg, pcm, &_pcm);
+			if (ret < 0)
+				return ret;
 		}
 
 		/* create the FE DAIs and DAI links */
-- 
2.17.1

[PATCH v2 6/6] ASoC: topology: Check return value of soc_tplg_dai_config

[Amadeusz Sławiński]

Function soc_tplg_dai_config can fail, check for and handle possible
failure.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---

 v2:
  Added this patch

 sound/soc/soc-topology.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index dade27588e51..9d6b266ccef8 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2524,7 +2524,7 @@ static int soc_tplg_dai_elems_load(struct soc_tplg *tplg,
 {
 	struct snd_soc_tplg_dai *dai;
 	int count;
-	int i;
+	int i, ret;
 
 	count = le32_to_cpu(hdr->count);
 
@@ -2539,7 +2539,12 @@ static int soc_tplg_dai_elems_load(struct soc_tplg *tplg,
 			return -EINVAL;
 		}
 
-		soc_tplg_dai_config(tplg, dai);
+		ret = soc_tplg_dai_config(tplg, dai);
+		if (ret < 0) {
+			dev_err(tplg->dev, "ASoC: failed to configure DAI\n");
+			return ret;
+		}
+
 		tplg->pos += (sizeof(*dai) + le32_to_cpu(dai->priv.size));
 	}
 
-- 
2.17.1

Re: [PATCH v2 3/6] ASoC: topology: Check return value of soc_tplg_*_create

[Amadeusz Sławiński]

On 3/27/2020 7:56 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 3/27/20 3:47 PM, Amadeusz Sławiński wrote:
>> Functions soc_tplg_denum_create, soc_tplg_dmixer_create,
>> soc_tplg_dbytes_create can fail, so their return values should be
>> checked and error should be propagated.
>>
>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>> ---
>>
>>   v2:
>>    Added this patch
>>
>>   sound/soc/soc-topology.c | 18 ++++++++++++------
>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
>> index c3811dd66b68..860bced933d6 100644
>> --- a/sound/soc/soc-topology.c
>> +++ b/sound/soc/soc-topology.c
>> @@ -1124,6 +1124,7 @@ static int soc_tplg_kcontrol_elems_load(struct 
>> soc_tplg *tplg,
>>       struct snd_soc_tplg_hdr *hdr)
>>   {
>>       struct snd_soc_tplg_ctl_hdr *control_hdr;
>> +    int ret;
>>       int i;
>>       if (tplg->pass != SOC_TPLG_PASS_MIXER) {
>> @@ -1152,25 +1153,30 @@ static int soc_tplg_kcontrol_elems_load(struct 
>> soc_tplg *tplg,
>>           case SND_SOC_TPLG_CTL_RANGE:
>>           case SND_SOC_TPLG_DAPM_CTL_VOLSW:
>>           case SND_SOC_TPLG_DAPM_CTL_PIN:
>> -            soc_tplg_dmixer_create(tplg, 1,
>> -                           le32_to_cpu(hdr->payload_size));
>> +            ret = soc_tplg_dmixer_create(tplg, 1,
>> +                    le32_to_cpu(hdr->payload_size));
>>               break;
>>           case SND_SOC_TPLG_CTL_ENUM:
>>           case SND_SOC_TPLG_CTL_ENUM_VALUE:
>>           case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE:
>>           case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT:
>>           case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE:
>> -            soc_tplg_denum_create(tplg, 1,
>> -                          le32_to_cpu(hdr->payload_size));
>> +            ret = soc_tplg_denum_create(tplg, 1,
>> +                    le32_to_cpu(hdr->payload_size));
>>               break;
>>           case SND_SOC_TPLG_CTL_BYTES:
>> -            soc_tplg_dbytes_create(tplg, 1,
>> -                           le32_to_cpu(hdr->payload_size));
>> +            ret = soc_tplg_dbytes_create(tplg, 1,
>> +                    le32_to_cpu(hdr->payload_size));
>>               break;
>>           default:
>>               soc_bind_err(tplg, control_hdr, i);
>>               return -EINVAL;
>>           }
>> +        if (ret < 0) {
>> +            dev_err(tplg->dev, "ASoC: invalid control\n");
>> +            return ret;
>> +        }
> 
> Sounds good, but this happens in a loop, so would all the memory 
> previously allocated by denum/dbytes/dmixer_create leak, or is it freed 
> automatically somewhere else?
> 

Well, now that error is propagated, snd_soc_tplg_component_remove() 
should be called by snd_soc_tplg_component_load() in case of errors 
while parsing. From quick look it seems like it should be able to free 
it up correctly by calling remove_enum/bytes/mixer.

Thanks,
Amadeusz

Re: [PATCH v2 3/6] ASoC: topology: Check return value of soc_tplg_*_create

[Pierre-Louis Bossart]

>>>       if (tplg->pass != SOC_TPLG_PASS_MIXER) {
>>> @@ -1152,25 +1153,30 @@ static int 
>>> soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
>>>           case SND_SOC_TPLG_CTL_RANGE:
>>>           case SND_SOC_TPLG_DAPM_CTL_VOLSW:
>>>           case SND_SOC_TPLG_DAPM_CTL_PIN:
>>> -            soc_tplg_dmixer_create(tplg, 1,
>>> -                           le32_to_cpu(hdr->payload_size));
>>> +            ret = soc_tplg_dmixer_create(tplg, 1,
>>> +                    le32_to_cpu(hdr->payload_size));
>>>               break;
>>>           case SND_SOC_TPLG_CTL_ENUM:
>>>           case SND_SOC_TPLG_CTL_ENUM_VALUE:
>>>           case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE:
>>>           case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT:
>>>           case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE:
>>> -            soc_tplg_denum_create(tplg, 1,
>>> -                          le32_to_cpu(hdr->payload_size));
>>> +            ret = soc_tplg_denum_create(tplg, 1,
>>> +                    le32_to_cpu(hdr->payload_size));
>>>               break;
>>>           case SND_SOC_TPLG_CTL_BYTES:
>>> -            soc_tplg_dbytes_create(tplg, 1,
>>> -                           le32_to_cpu(hdr->payload_size));
>>> +            ret = soc_tplg_dbytes_create(tplg, 1,
>>> +                    le32_to_cpu(hdr->payload_size));
>>>               break;
>>>           default:
>>>               soc_bind_err(tplg, control_hdr, i);
>>>               return -EINVAL;
>>>           }
>>> +        if (ret < 0) {
>>> +            dev_err(tplg->dev, "ASoC: invalid control\n");
>>> +            return ret;
>>> +        }
>>
>> Sounds good, but this happens in a loop, so would all the memory 
>> previously allocated by denum/dbytes/dmixer_create leak, or is it 
>> freed automatically somewhere else?
>>
> 
> Well, now that error is propagated, snd_soc_tplg_component_remove() 
> should be called by snd_soc_tplg_component_load() in case of errors 
> while parsing. From quick look it seems like it should be able to free 
> it up correctly by calling remove_enum/bytes/mixer.

I am not sure what you meant by 'should be called', if it's a 
recommendation for a future change or a description of the existing 
behavior.
Just to be clear, are you saying the existing code will take care of 
this error flow or that a new patch is needed?

Re: [PATCH v2 3/6] ASoC: topology: Check return value of soc_tplg_*_create

[Amadeusz Sławiński]

>>> Sounds good, but this happens in a loop, so would all the memory 
>>> previously allocated by denum/dbytes/dmixer_create leak, or is it 
>>> freed automatically somewhere else?
>>>
>>
>> Well, now that error is propagated, snd_soc_tplg_component_remove() 
>> should be called by snd_soc_tplg_component_load() in case of errors 
>> while parsing. From quick look it seems like it should be able to free 
>> it up correctly by calling remove_enum/bytes/mixer.
> 
> I am not sure what you meant by 'should be called', if it's a 
> recommendation for a future change or a description of the existing 
> behavior.
> Just to be clear, are you saying the existing code will take care of 
> this error flow or that a new patch is needed?

Existing code should handle this properly.
No new code is needed.

Re: [PATCH v2 3/6] ASoC: topology: Check return value of soc_tplg_*_create

[Pierre-Louis Bossart]

On 3/30/20 11:10 AM, Amadeusz Sławiński wrote:
> 
>>>> Sounds good, but this happens in a loop, so would all the memory 
>>>> previously allocated by denum/dbytes/dmixer_create leak, or is it 
>>>> freed automatically somewhere else?
>>>>
>>>
>>> Well, now that error is propagated, snd_soc_tplg_component_remove() 
>>> should be called by snd_soc_tplg_component_load() in case of errors 
>>> while parsing. From quick look it seems like it should be able to 
>>> free it up correctly by calling remove_enum/bytes/mixer.
>>
>> I am not sure what you meant by 'should be called', if it's a 
>> recommendation for a future change or a description of the existing 
>> behavior.
>> Just to be clear, are you saying the existing code will take care of 
>> this error flow or that a new patch is needed?
> 
> Existing code should handle this properly.
> No new code is needed.

Sounds good, thanks.

Re: [PATCH v2 0/6] ASoC: topology: Propagate error appropriately

[Pierre-Louis Bossart]

On 3/27/20 3:47 PM, Amadeusz Sławiński wrote:
> v1:
>    Check if kstrdup succeeded.
> 
> v2:
>    Remove unneeded freeing, which is performed in another place by dobj
>    handlers.
> 
>    Additionally for functions which have return status which was ignored,
>    perform success checks and handle failures in appropriate way.
> 
> Amadeusz Sławiński (6):
>    ASoC: topology: Add missing memory checks
>    ASoC: topology: Check return value of soc_tplg_create_tlv
>    ASoC: topology: Check return value of soc_tplg_*_create
>    ASoC: topology: Check soc_tplg_add_route return value
>    ASoC: topology: Check return value of pcm_new_ver
>    ASoC: topology: Check return value of soc_tplg_dai_config

Looks good to me

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

We probably want Ranjani to double-check this series though.

> 
>   sound/soc/soc-topology.c | 113 ++++++++++++++++++++++++++++++---------
>   1 file changed, 88 insertions(+), 25 deletions(-)
> 

Re: [PATCH v2 0/6] ASoC: topology: Propagate error appropriately

[Sridharan, Ranjani]

On Fri, Mar 27, 2020 at 11:40 AM Amadeusz Sławiński <
amadeuszx.slawinski@linux.intel.com> wrote:

> v1:
>   Check if kstrdup succeeded.
>
> v2:
>   Remove unneeded freeing, which is performed in another place by dobj
>   handlers.
>
>   Additionally for functions which have return status which was ignored,
>   perform success checks and handle failures in appropriate way.
>
> Amadeusz Sławiński (6):
>   ASoC: topology: Add missing memory checks
>   ASoC: topology: Check return value of soc_tplg_create_tlv
>   ASoC: topology: Check return value of soc_tplg_*_create
>   ASoC: topology: Check soc_tplg_add_route return value
>   ASoC: topology: Check return value of pcm_new_ver
>   ASoC: topology: Check return value of soc_tplg_dai_config

Thanks, Amadeusz. LGTM
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Re: [PATCH v2 0/6] ASoC: topology: Propagate error appropriately

[Amadeusz Sławiński]

On 3/30/2020 6:38 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 3/27/20 3:47 PM, Amadeusz Sławiński wrote:
>> v1:
>>    Check if kstrdup succeeded.
>>
>> v2:
>>    Remove unneeded freeing, which is performed in another place by dobj
>>    handlers.
>>
>>    Additionally for functions which have return status which was ignored,
>>    perform success checks and handle failures in appropriate way.
>>
>> Amadeusz Sławiński (6):
>>    ASoC: topology: Add missing memory checks
>>    ASoC: topology: Check return value of soc_tplg_create_tlv
>>    ASoC: topology: Check return value of soc_tplg_*_create
>>    ASoC: topology: Check soc_tplg_add_route return value
>>    ASoC: topology: Check return value of pcm_new_ver
>>    ASoC: topology: Check return value of soc_tplg_dai_config
> 
> Looks good to me
> 
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> We probably want Ranjani to double-check this series though.
> 

Hi Ranjani, can you take another look, I would like for this to get 
merged before I forget about it ;)

>>
>>   sound/soc/soc-topology.c | 113 ++++++++++++++++++++++++++++++---------
>>   1 file changed, 88 insertions(+), 25 deletions(-)
>>

Re: [PATCH v2 0/6] ASoC: topology: Propagate error appropriately

[Pierre-Louis Bossart]

>> Looks good to me
>>
>> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>
>> We probably want Ranjani to double-check this series though.
>>
> 
> Hi Ranjani, can you take another look, I would like for this to get 
> merged before I forget about it ;)

Ranjani provided her Reviewed-by tag on March 30 - likely our emails 
crossed.

Re: [PATCH v2 0/6] ASoC: topology: Propagate error appropriately

[Amadeusz Sławiński]

On 4/8/2020 4:20 PM, Pierre-Louis Bossart wrote:
> 
>>> Looks good to me
>>>
>>> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>
>>> We probably want Ranjani to double-check this series though.
>>>
>>
>> Hi Ranjani, can you take another look, I would like for this to get 
>> merged before I forget about it ;)
> 
> Ranjani provided her Reviewed-by tag on March 30 - likely our emails 
> crossed.

That's probably what happened, I only asked, because "double-check" 
comment above caused me to think it may need another look. Thanks, for 
confirming it's good.

Re: [PATCH v2 0/6] ASoC: topology: Propagate error appropriately

[Ranjani Sridharan]

On Wed, 2020-04-08 at 16:46 +0200, Amadeusz Sławiński wrote:
> 
> On 4/8/2020 4:20 PM, Pierre-Louis Bossart wrote:
> > 
> > > > Looks good to me
> > > > 
> > > > Reviewed-by: Pierre-Louis Bossart <
> > > > pierre-louis.bossart@linux.intel.com>
> > > > 
> > > > We probably want Ranjani to double-check this series though.
> > > > 
> > > 
> > > Hi Ranjani, can you take another look, I would like for this to
> > > get 
> > > merged before I forget about it ;)
> > 
> > Ranjani provided her Reviewed-by tag on March 30 - likely our
> > emails 
> > crossed.
> 
> That's probably what happened, I only asked, because "double-check" 
> comment above caused me to think it may need another look. Thanks,
> for 
> confirming it's good.
Sorry for the confusion, Amadeusz/Pierre. Yes, I didnt want to reply
and add noise as I had already reviewed the series.

Thanks,
Ranjani

Applied "ASoC: topology: Check return value of soc_tplg_dai_config" to the asoc tree

[Mark Brown]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2555 bytes --]

The patch

   ASoC: topology: Check return value of soc_tplg_dai_config

has been applied to the asoc tree at

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

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

From dd8e871d4e560eeb8d22af82dde91457ad835a63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Amadeusz=20S=C5=82awi=C5=84ski?=
 <amadeuszx.slawinski@linux.intel.com>
Date: Fri, 27 Mar 2020 16:47:29 -0400
Subject: [PATCH] ASoC: topology: Check return value of soc_tplg_dai_config
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Function soc_tplg_dai_config can fail, check for and handle possible
failure.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200327204729.397-7-amadeuszx.slawinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-topology.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 818657b06799..33e8d189ba2f 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2524,7 +2524,7 @@ static int soc_tplg_dai_elems_load(struct soc_tplg *tplg,
 {
 	struct snd_soc_tplg_dai *dai;
 	int count;
-	int i;
+	int i, ret;
 
 	count = le32_to_cpu(hdr->count);
 
@@ -2539,7 +2539,12 @@ static int soc_tplg_dai_elems_load(struct soc_tplg *tplg,
 			return -EINVAL;
 		}
 
-		soc_tplg_dai_config(tplg, dai);
+		ret = soc_tplg_dai_config(tplg, dai);
+		if (ret < 0) {
+			dev_err(tplg->dev, "ASoC: failed to configure DAI\n");
+			return ret;
+		}
+
 		tplg->pos += (sizeof(*dai) + le32_to_cpu(dai->priv.size));
 	}
 
-- 
2.20.1

Applied "ASoC: topology: Check return value of pcm_new_ver" to the asoc tree

[Mark Brown]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2298 bytes --]

The patch

   ASoC: topology: Check return value of pcm_new_ver

has been applied to the asoc tree at

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

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

From b3677fc3d68dd942c92de52f0bd9dd8b472a40e6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Amadeusz=20S=C5=82awi=C5=84ski?=
 <amadeuszx.slawinski@linux.intel.com>
Date: Fri, 27 Mar 2020 16:47:28 -0400
Subject: [PATCH] ASoC: topology: Check return value of pcm_new_ver
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Function pcm_new_ver can fail, so we should check it's return value and
handle possible error.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200327204729.397-6-amadeuszx.slawinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-topology.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index cb43994089de..818657b06799 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2135,7 +2135,9 @@ static int soc_tplg_pcm_elems_load(struct soc_tplg *tplg,
 			_pcm = pcm;
 		} else {
 			abi_match = false;
-			pcm_new_ver(tplg, pcm, &_pcm);
+			ret = pcm_new_ver(tplg, pcm, &_pcm);
+			if (ret < 0)
+				return ret;
 		}
 
 		/* create the FE DAIs and DAI links */
-- 
2.20.1

Applied "ASoC: topology: Check soc_tplg_add_route return value" to the asoc tree

[Mark Brown]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]

The patch

   ASoC: topology: Check soc_tplg_add_route return value

has been applied to the asoc tree at

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

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

From 6856e887eae3efc0fe56899cb3f969fe063171c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Amadeusz=20S=C5=82awi=C5=84ski?=
 <amadeuszx.slawinski@linux.intel.com>
Date: Fri, 27 Mar 2020 16:47:27 -0400
Subject: [PATCH] ASoC: topology: Check soc_tplg_add_route return value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Function soc_tplg_add_route can propagate error code from callback, we
should check its return value and handle fail in correct way.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200327204729.397-5-amadeuszx.slawinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-topology.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 3ada769cf823..cb43994089de 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1284,7 +1284,9 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
 		routes[i]->dobj.index = tplg->index;
 		list_add(&routes[i]->dobj.list, &tplg->comp->dobj_list);
 
-		soc_tplg_add_route(tplg, routes[i]);
+		ret = soc_tplg_add_route(tplg, routes[i]);
+		if (ret < 0)
+			break;
 
 		/* add route, but keep going if some fail */
 		snd_soc_dapm_add_routes(dapm, routes[i], 1);
-- 
2.20.1

Applied "ASoC: topology: Check return value of soc_tplg_*_create" to the asoc tree

[Mark Brown]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3457 bytes --]

The patch

   ASoC: topology: Check return value of soc_tplg_*_create

has been applied to the asoc tree at

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

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

From 2ae548f30d7f6973388fc3769bb3c2f6fd13652b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Amadeusz=20S=C5=82awi=C5=84ski?=
 <amadeuszx.slawinski@linux.intel.com>
Date: Fri, 27 Mar 2020 16:47:26 -0400
Subject: [PATCH] ASoC: topology: Check return value of soc_tplg_*_create
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Functions soc_tplg_denum_create, soc_tplg_dmixer_create,
soc_tplg_dbytes_create can fail, so their return values should be
checked and error should be propagated.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200327204729.397-4-amadeuszx.slawinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-topology.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index f37a72aebb5a..3ada769cf823 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1124,6 +1124,7 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
 	struct snd_soc_tplg_hdr *hdr)
 {
 	struct snd_soc_tplg_ctl_hdr *control_hdr;
+	int ret;
 	int i;
 
 	if (tplg->pass != SOC_TPLG_PASS_MIXER) {
@@ -1152,25 +1153,30 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
 		case SND_SOC_TPLG_CTL_RANGE:
 		case SND_SOC_TPLG_DAPM_CTL_VOLSW:
 		case SND_SOC_TPLG_DAPM_CTL_PIN:
-			soc_tplg_dmixer_create(tplg, 1,
-					       le32_to_cpu(hdr->payload_size));
+			ret = soc_tplg_dmixer_create(tplg, 1,
+					le32_to_cpu(hdr->payload_size));
 			break;
 		case SND_SOC_TPLG_CTL_ENUM:
 		case SND_SOC_TPLG_CTL_ENUM_VALUE:
 		case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE:
 		case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT:
 		case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE:
-			soc_tplg_denum_create(tplg, 1,
-					      le32_to_cpu(hdr->payload_size));
+			ret = soc_tplg_denum_create(tplg, 1,
+					le32_to_cpu(hdr->payload_size));
 			break;
 		case SND_SOC_TPLG_CTL_BYTES:
-			soc_tplg_dbytes_create(tplg, 1,
-					       le32_to_cpu(hdr->payload_size));
+			ret = soc_tplg_dbytes_create(tplg, 1,
+					le32_to_cpu(hdr->payload_size));
 			break;
 		default:
 			soc_bind_err(tplg, control_hdr, i);
 			return -EINVAL;
 		}
+		if (ret < 0) {
+			dev_err(tplg->dev, "ASoC: invalid control\n");
+			return ret;
+		}
+
 	}
 
 	return 0;
-- 
2.20.1

Applied "ASoC: topology: Check return value of soc_tplg_create_tlv" to the asoc tree

[Mark Brown]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2979 bytes --]

The patch

   ASoC: topology: Check return value of soc_tplg_create_tlv

has been applied to the asoc tree at

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

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

From 482db55ae87f3749db05810a38b1d618dfd4407c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Amadeusz=20S=C5=82awi=C5=84ski?=
 <amadeuszx.slawinski@linux.intel.com>
Date: Fri, 27 Mar 2020 16:47:25 -0400
Subject: [PATCH] ASoC: topology: Check return value of soc_tplg_create_tlv
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Function soc_tplg_create_tlv can fail, so we should check if it succeded
or not and proceed appropriately.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200327204729.397-3-amadeuszx.slawinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-topology.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 73fc304c9aca..f37a72aebb5a 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -894,7 +894,13 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
 		}
 
 		/* create any TLV data */
-		soc_tplg_create_tlv(tplg, &kc, &mc->hdr);
+		err = soc_tplg_create_tlv(tplg, &kc, &mc->hdr);
+		if (err < 0) {
+			dev_err(tplg->dev, "ASoC: failed to create TLV %s\n",
+				mc->hdr.name);
+			kfree(sm);
+			continue;
+		}
 
 		/* pass control to driver for optional further init */
 		err = soc_tplg_init_kcontrol(tplg, &kc,
@@ -1355,7 +1361,13 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dmixer_create(
 		}
 
 		/* create any TLV data */
-		soc_tplg_create_tlv(tplg, &kc[i], &mc->hdr);
+		err = soc_tplg_create_tlv(tplg, &kc[i], &mc->hdr);
+		if (err < 0) {
+			dev_err(tplg->dev, "ASoC: failed to create TLV %s\n",
+				mc->hdr.name);
+			kfree(sm);
+			continue;
+		}
 
 		/* pass control to driver for optional further init */
 		err = soc_tplg_init_kcontrol(tplg, &kc[i],
-- 
2.20.1

Applied "ASoC: topology: Add missing memory checks" to the asoc tree

[Mark Brown]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 6105 bytes --]

The patch

   ASoC: topology: Add missing memory checks

has been applied to the asoc tree at

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

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

From abc3caac24501008465fdb55c5e89e16d58d5a3d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Amadeusz=20S=C5=82awi=C5=84ski?=
 <amadeuszx.slawinski@linux.intel.com>
Date: Fri, 27 Mar 2020 16:47:24 -0400
Subject: [PATCH] ASoC: topology: Add missing memory checks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

kstrdup is an allocation function and it can fail, so its return value
should be checked and handled appropriately.

In order to check all cases, we need to modify set_stream_info to return
a value, so check that everything went correctly when doing kstrdup().
Later add proper checks and error handlers.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200327204729.397-2-amadeuszx.slawinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-topology.c | 62 +++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 13 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 87f75edba3dc..73fc304c9aca 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1766,10 +1766,13 @@ static int soc_tplg_dapm_complete(struct soc_tplg *tplg)
 	return 0;
 }
 
-static void set_stream_info(struct snd_soc_pcm_stream *stream,
+static int set_stream_info(struct snd_soc_pcm_stream *stream,
 	struct snd_soc_tplg_stream_caps *caps)
 {
 	stream->stream_name = kstrdup(caps->name, GFP_KERNEL);
+	if (!stream->stream_name)
+		return -ENOMEM;
+
 	stream->channels_min = le32_to_cpu(caps->channels_min);
 	stream->channels_max = le32_to_cpu(caps->channels_max);
 	stream->rates = le32_to_cpu(caps->rates);
@@ -1777,6 +1780,8 @@ static void set_stream_info(struct snd_soc_pcm_stream *stream,
 	stream->rate_max = le32_to_cpu(caps->rate_max);
 	stream->formats = le64_to_cpu(caps->formats);
 	stream->sig_bits = le32_to_cpu(caps->sig_bits);
+
+	return 0;
 }
 
 static void set_dai_flags(struct snd_soc_dai_driver *dai_drv,
@@ -1812,20 +1817,29 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	if (dai_drv == NULL)
 		return -ENOMEM;
 
-	if (strlen(pcm->dai_name))
+	if (strlen(pcm->dai_name)) {
 		dai_drv->name = kstrdup(pcm->dai_name, GFP_KERNEL);
+		if (!dai_drv->name) {
+			ret = -ENOMEM;
+			goto err;
+		}
+	}
 	dai_drv->id = le32_to_cpu(pcm->dai_id);
 
 	if (pcm->playback) {
 		stream = &dai_drv->playback;
 		caps = &pcm->caps[SND_SOC_TPLG_STREAM_PLAYBACK];
-		set_stream_info(stream, caps);
+		ret = set_stream_info(stream, caps);
+		if (ret < 0)
+			goto err;
 	}
 
 	if (pcm->capture) {
 		stream = &dai_drv->capture;
 		caps = &pcm->caps[SND_SOC_TPLG_STREAM_CAPTURE];
-		set_stream_info(stream, caps);
+		ret = set_stream_info(stream, caps);
+		if (ret < 0)
+			goto err;
 	}
 
 	if (pcm->compress)
@@ -1835,11 +1849,7 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	ret = soc_tplg_dai_load(tplg, dai_drv, pcm, NULL);
 	if (ret < 0) {
 		dev_err(tplg->comp->dev, "ASoC: DAI loading failed\n");
-		kfree(dai_drv->playback.stream_name);
-		kfree(dai_drv->capture.stream_name);
-		kfree(dai_drv->name);
-		kfree(dai_drv);
-		return ret;
+		goto err;
 	}
 
 	dai_drv->dobj.index = tplg->index;
@@ -1860,6 +1870,14 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 		return ret;
 	}
 
+	return 0;
+
+err:
+	kfree(dai_drv->playback.stream_name);
+	kfree(dai_drv->capture.stream_name);
+	kfree(dai_drv->name);
+	kfree(dai_drv);
+
 	return ret;
 }
 
@@ -1916,11 +1934,20 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg,
 	if (strlen(pcm->pcm_name)) {
 		link->name = kstrdup(pcm->pcm_name, GFP_KERNEL);
 		link->stream_name = kstrdup(pcm->pcm_name, GFP_KERNEL);
+		if (!link->name || !link->stream_name) {
+			ret = -ENOMEM;
+			goto err;
+		}
 	}
 	link->id = le32_to_cpu(pcm->pcm_id);
 
-	if (strlen(pcm->dai_name))
+	if (strlen(pcm->dai_name)) {
 		link->cpus->dai_name = kstrdup(pcm->dai_name, GFP_KERNEL);
+		if (!link->cpus->dai_name) {
+			ret = -ENOMEM;
+			goto err;
+		}
+	}
 
 	link->codecs->name = "snd-soc-dummy";
 	link->codecs->dai_name = "snd-soc-dummy-dai";
@@ -2436,13 +2463,17 @@ static int soc_tplg_dai_config(struct soc_tplg *tplg,
 	if (d->playback) {
 		stream = &dai_drv->playback;
 		caps = &d->caps[SND_SOC_TPLG_STREAM_PLAYBACK];
-		set_stream_info(stream, caps);
+		ret = set_stream_info(stream, caps);
+		if (ret < 0)
+			goto err;
 	}
 
 	if (d->capture) {
 		stream = &dai_drv->capture;
 		caps = &d->caps[SND_SOC_TPLG_STREAM_CAPTURE];
-		set_stream_info(stream, caps);
+		ret = set_stream_info(stream, caps);
+		if (ret < 0)
+			goto err;
 	}
 
 	if (d->flag_mask)
@@ -2454,10 +2485,15 @@ static int soc_tplg_dai_config(struct soc_tplg *tplg,
 	ret = soc_tplg_dai_load(tplg, dai_drv, NULL, dai);
 	if (ret < 0) {
 		dev_err(tplg->comp->dev, "ASoC: DAI loading failed\n");
-		return ret;
+		goto err;
 	}
 
 	return 0;
+
+err:
+	kfree(dai_drv->playback.stream_name);
+	kfree(dai_drv->capture.stream_name);
+	return ret;
 }
 
 /* load physical DAI elements */
-- 
2.20.1