All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation
@ 2020-06-25 11:03 Piotr Maziarz
  2020-06-25 11:03 ` [PATCH alsa-lib 2/8] topology: decode: Fix adding texts field to enum control Piotr Maziarz
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Piotr Maziarz @ 2020-06-25 11:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski

Without proper memory allocation behaviour was undefined.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
---
 src/topology/ctl.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index 90241b6..c8c7e94 100644
--- a/src/topology/ctl.c
+++ b/src/topology/ctl.c
@@ -1330,7 +1330,6 @@ int tplg_decode_control_enum1(snd_tplg_t *tplg,
 			      void *bin, size_t size)
 {
 	struct snd_soc_tplg_enum_control *ec = bin;
-	struct snd_tplg_channel_map_template cmt;
 	int i;
 
 	if (size < sizeof(*ec)) {
@@ -1375,11 +1374,11 @@ int tplg_decode_control_enum1(snd_tplg_t *tplg,
 		}
 	}
 
-	et->map = &cmt;
-	memset(&cmt, 0, sizeof(cmt));
-	cmt.num_channels = ec->num_channels;
-	for (i = 0; i < cmt.num_channels; i++) {
-		struct snd_tplg_channel_elem *channel = &cmt.channel[i];
+	et->map = tplg_calloc(heap, sizeof(struct snd_tplg_channel_map_template));
+	et->map->num_channels = ec->num_channels;
+	for (i = 0; i < et->map->num_channels; i++) {
+		struct snd_tplg_channel_elem *channel = &et->map->channel[i];
+
 		tplg_log(tplg, 'D', pos + ((void *)&ec->channel[i] - (void *)ec),
 			 "enum: channel size %d", ec->channel[i].size);
 		channel->reg = ec->channel[i].reg;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH alsa-lib 2/8] topology: decode: Fix adding texts field to enum control
  2020-06-25 11:03 [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Piotr Maziarz
@ 2020-06-25 11:03 ` Piotr Maziarz
  2020-06-25 14:24   ` Pierre-Louis Bossart
  2020-06-25 11:03 ` [PATCH alsa-lib 3/8] topology: decode: Fix printing texts section Piotr Maziarz
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Piotr Maziarz @ 2020-06-25 11:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski

Iterating over texts in tplg_decode_control_enum1 was an infinite loop,
it needed to be fixed. Parsing values was removed since they are not added
to the UML file.
Also texts are separate section and therefore it needs to be added as
separate element.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
---
 src/topology/ctl.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index c8c7e94..24eadc8 100644
--- a/src/topology/ctl.c
+++ b/src/topology/ctl.c
@@ -1088,11 +1088,19 @@ int tplg_add_enum(snd_tplg_t *tplg, struct snd_tplg_enum_template *enum_ctl,
 	}
 
 	if (enum_ctl->texts != NULL) {
+		struct tplg_elem *texts = tplg_elem_new_common(tplg, NULL,
+						enum_ctl->hdr.name, SND_TPLG_TYPE_TEXT);
+
+		texts->texts->num_items = num_items;
 		for (i = 0; i < num_items; i++) {
-			if (enum_ctl->texts[i] != NULL)
-				snd_strlcpy(ec->texts[i], enum_ctl->texts[i],
-					    SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
+			if (!enum_ctl->texts[i])
+				continue;
+			snd_strlcpy(ec->texts[i], enum_ctl->texts[i],
+				    SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
+			snd_strlcpy(texts->texts->items[i], enum_ctl->texts[i],
+				    SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
 		}
+		tplg_ref_add(elem, SND_TPLG_TYPE_TEXT, enum_ctl->hdr.name);
 	}
 
 	if (enum_ctl->values != NULL) {
@@ -1367,11 +1375,8 @@ int tplg_decode_control_enum1(snd_tplg_t *tplg,
 		et->texts = tplg_calloc(heap, sizeof(char *) * ec->items);
 		if (!et->texts)
 			return -ENOMEM;
-		for (i = 0; ec->items; i++) {
-			unsigned int j = i * sizeof(int) * ENUM_VAL_SIZE;
+		for (i = 0; i < ec->items; i++)
 			et->texts[i] = ec->texts[i];
-			et->values[i] = (int *)&ec->values[j];
-		}
 	}
 
 	et->map = tplg_calloc(heap, sizeof(struct snd_tplg_channel_map_template));
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH alsa-lib 3/8] topology: decode: Fix printing texts section
  2020-06-25 11:03 [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Piotr Maziarz
  2020-06-25 11:03 ` [PATCH alsa-lib 2/8] topology: decode: Fix adding texts field to enum control Piotr Maziarz
@ 2020-06-25 11:03 ` Piotr Maziarz
  2020-06-25 11:03 ` [PATCH alsa-lib 4/8] topology: decode: Change declaration of enum decoding function Piotr Maziarz
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Piotr Maziarz @ 2020-06-25 11:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
---
 src/topology/text.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/topology/text.c b/src/topology/text.c
index 507c545..b899b28 100644
--- a/src/topology/text.c
+++ b/src/topology/text.c
@@ -103,7 +103,7 @@ int tplg_save_text(snd_tplg_t *tplg ATTRIBUTE_UNUSED,
 		return 0;
 	err = tplg_save_printf(dst, pfx, "'%s'.values [\n", elem->id);
 	for (i = 0; err >= 0 && i < texts->num_items; i++)
-		err = tplg_save_printf(dst, pfx, "\t'%s'\n", texts->items[i][0]);
+		err = tplg_save_printf(dst, pfx, "\t'%s'\n", texts->items[i]);
 	if (err >= 0)
 		err = tplg_save_printf(dst, pfx, "]\n");
 	return err;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH alsa-lib 4/8] topology: decode: Change declaration of enum decoding function
  2020-06-25 11:03 [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Piotr Maziarz
  2020-06-25 11:03 ` [PATCH alsa-lib 2/8] topology: decode: Fix adding texts field to enum control Piotr Maziarz
  2020-06-25 11:03 ` [PATCH alsa-lib 3/8] topology: decode: Fix printing texts section Piotr Maziarz
@ 2020-06-25 11:03 ` Piotr Maziarz
  2020-06-25 11:03 ` [PATCH alsa-lib 5/8] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Piotr Maziarz @ 2020-06-25 11:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski

Size constraints are always checked before invoking
tplg_decode_control_enum1. There is no need to validate it twice.
Alos moved debug print about size to invoking function, since now it's it
responsibility to check size.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
---
 src/topology/ctl.c        | 19 +++++--------------
 src/topology/dapm.c       |  3 +--
 src/topology/tplg_local.h |  2 +-
 3 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index 24eadc8..b5e3e34 100644
--- a/src/topology/ctl.c
+++ b/src/topology/ctl.c
@@ -1335,22 +1335,10 @@ int tplg_decode_control_enum1(snd_tplg_t *tplg,
 			      struct list_head *heap,
 			      struct snd_tplg_enum_template *et,
 			      size_t pos,
-			      void *bin, size_t size)
+			      struct snd_soc_tplg_enum_control *ec)
 {
-	struct snd_soc_tplg_enum_control *ec = bin;
 	int i;
 
-	if (size < sizeof(*ec)) {
-		SNDERR("enum: small size %d", size);
-		return -EINVAL;
-	}
-
-	tplg_log(tplg, 'D', pos, "enum: size %d private size %d",
-		 ec->size, ec->priv.size);
-	if (size != ec->size + ec->priv.size) {
-		SNDERR("enum: unexpected element size %d", size);
-		return -EINVAL;
-	}
 	if (ec->num_channels > SND_TPLG_MAX_CHAN ||
 	    ec->num_channels > SND_SOC_TPLG_MAX_CHAN) {
 		SNDERR("enum: unexpected channel count %d", ec->num_channels);
@@ -1425,7 +1413,10 @@ next:
 		return -EINVAL;
 	}
 
-	err = tplg_decode_control_enum1(tplg, &heap, &et, pos, bin, size);
+	tplg_log(tplg, 'D', pos, "enum: size %d private size %d",
+		 ec->size, ec->priv.size);
+
+	err = tplg_decode_control_enum1(tplg, &heap, &et, pos, ec);
 	if (err >= 0) {
 		t.enum_ctl = &et;
 		err = snd_tplg_add_object(tplg, &t);
diff --git a/src/topology/dapm.c b/src/topology/dapm.c
index cd1a877..73a9390 100644
--- a/src/topology/dapm.c
+++ b/src/topology/dapm.c
@@ -972,8 +972,7 @@ next:
 				err = -EINVAL;
 				goto retval;
 			}
-			err = tplg_decode_control_enum1(tplg, &heap, et, pos,
-							bin, size2);
+			err = tplg_decode_control_enum1(tplg, &heap, et, pos, ec);
 			break;
 		case SND_SOC_TPLG_TYPE_BYTES:
 			bt = tplg_calloc(&heap, sizeof(*bt));
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 5ace0d1..acb01a8 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -398,7 +398,7 @@ int tplg_decode_control_enum1(snd_tplg_t *tplg,
 			      struct list_head *heap,
 			      struct snd_tplg_enum_template *et,
 			      size_t pos,
-			      void *bin, size_t size);
+			      struct snd_soc_tplg_enum_control *ec);
 int tplg_decode_control_enum(snd_tplg_t *tplg, size_t pos,
 			     struct snd_soc_tplg_hdr *hdr,
 			     void *bin, size_t size);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH alsa-lib 5/8] topology: decode: Fix decoding PCM formats and rates
  2020-06-25 11:03 [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Piotr Maziarz
                   ` (2 preceding siblings ...)
  2020-06-25 11:03 ` [PATCH alsa-lib 4/8] topology: decode: Change declaration of enum decoding function Piotr Maziarz
@ 2020-06-25 11:03 ` Piotr Maziarz
  2020-06-25 11:03 ` [PATCH alsa-lib 6/8] topology: decode: Print sig_bits field in PCM capabilities section Piotr Maziarz
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Piotr Maziarz @ 2020-06-25 11:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski

Not checking _LAST format and rate, which are valid indexes in arrays,
makes data loss while converting binary to UML.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
---
 src/topology/pcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/topology/pcm.c b/src/topology/pcm.c
index b15b950..db40114 100644
--- a/src/topology/pcm.c
+++ b/src/topology/pcm.c
@@ -549,7 +549,7 @@ int tplg_save_stream_caps(snd_tplg_t *tplg ATTRIBUTE_UNUSED,
 	if (err >= 0 && sc->formats) {
 		err = tplg_save_printf(dst, pfx, "\tformats '");
 		first = 1;
-		for (i = 0; err >= 0 && i < SND_PCM_FORMAT_LAST; i++) {
+		for (i = 0; err >= 0 && i <= SND_PCM_FORMAT_LAST; i++) {
 			if (sc->formats & (1ULL << i)) {
 				s = snd_pcm_format_name(i);
 				err = tplg_save_printf(dst, NULL, "%s%s",
@@ -563,7 +563,7 @@ int tplg_save_stream_caps(snd_tplg_t *tplg ATTRIBUTE_UNUSED,
 	if (err >= 0 && sc->rates) {
 		err = tplg_save_printf(dst, pfx, "\trates '");
 		first = 1;
-		for (i = 0; err >= 0 && i < SND_PCM_RATE_LAST; i++) {
+		for (i = 0; err >= 0 && i <= SND_PCM_RATE_LAST; i++) {
 			if (sc->rates & (1ULL << i)) {
 				s = get_rate_name(i);
 				err = tplg_save_printf(dst, NULL, "%s%s",
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH alsa-lib 6/8] topology: decode: Print sig_bits field in PCM capabilities section
  2020-06-25 11:03 [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Piotr Maziarz
                   ` (3 preceding siblings ...)
  2020-06-25 11:03 ` [PATCH alsa-lib 5/8] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
@ 2020-06-25 11:03 ` Piotr Maziarz
  2020-06-25 11:03 ` [PATCH alsa-lib 7/8] topology: decode: Add DAI name printing Piotr Maziarz
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Piotr Maziarz @ 2020-06-25 11:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski

Not printing this field makes data loss while converting from binary
to UML.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
---
 src/topology/pcm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/topology/pcm.c b/src/topology/pcm.c
index db40114..49c5eab 100644
--- a/src/topology/pcm.c
+++ b/src/topology/pcm.c
@@ -604,6 +604,9 @@ int tplg_save_stream_caps(snd_tplg_t *tplg ATTRIBUTE_UNUSED,
 	if (err >= 0 && sc->buffer_size_max)
 		err = tplg_save_printf(dst, pfx, "\tbuffer_size_max %u\n",
 				       sc->buffer_size_max);
+	if (err >= 0 && sc->sig_bits)
+		err = tplg_save_printf(dst, pfx, "\tsig_bits %u\n",
+				       sc->sig_bits);
 	if (err >= 0)
 		err = tplg_save_printf(dst, pfx, "}\n");
 	return err;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH alsa-lib 7/8] topology: decode: Add DAI name printing
  2020-06-25 11:03 [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Piotr Maziarz
                   ` (4 preceding siblings ...)
  2020-06-25 11:03 ` [PATCH alsa-lib 6/8] topology: decode: Print sig_bits field in PCM capabilities section Piotr Maziarz
@ 2020-06-25 11:03 ` Piotr Maziarz
  2020-06-25 11:03 ` [PATCH alsa-lib 8/8] topology: Make buffer for saving dynamic size Piotr Maziarz
  2020-06-25 14:16 ` [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Pierre-Louis Bossart
  7 siblings, 0 replies; 12+ messages in thread
From: Piotr Maziarz @ 2020-06-25 11:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski

DAI name is a part of topology binary. Not printing makes data loss while
converting from binary to UML.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
---
 src/topology/pcm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/topology/pcm.c b/src/topology/pcm.c
index 49c5eab..5a54e15 100644
--- a/src/topology/pcm.c
+++ b/src/topology/pcm.c
@@ -781,7 +781,9 @@ int tplg_save_fe_dai(snd_tplg_t *tplg ATTRIBUTE_UNUSED,
 	struct snd_soc_tplg_pcm *pcm = elem->pcm;
 	int err = 0;
 
-	if (pcm->dai_id > 0)
+	if (strlen(pcm->dai_name))
+		err = tplg_save_printf(dst, pfx, "dai.'%s'.id %u\n", pcm->dai_name, pcm->dai_id);
+	else if (pcm->dai_id > 0)
 		err = tplg_save_printf(dst, pfx, "dai.0.id %u\n", pcm->dai_id);
 	return err;
 }
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH alsa-lib 8/8] topology: Make buffer for saving dynamic size
  2020-06-25 11:03 [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Piotr Maziarz
                   ` (5 preceding siblings ...)
  2020-06-25 11:03 ` [PATCH alsa-lib 7/8] topology: decode: Add DAI name printing Piotr Maziarz
@ 2020-06-25 11:03 ` Piotr Maziarz
  2020-06-25 14:31   ` Pierre-Louis Bossart
  2020-06-25 14:16 ` [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Pierre-Louis Bossart
  7 siblings, 1 reply; 12+ messages in thread
From: Piotr Maziarz @ 2020-06-25 11:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski

Some fields can exceed size limit, e.g. private data has no size
restriction. Therefore it needs to be dynamically increased.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
---
 src/topology/save.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/src/topology/save.c b/src/topology/save.c
index 4ecf86c..d6ee8b6 100644
--- a/src/topology/save.c
+++ b/src/topology/save.c
@@ -19,22 +19,43 @@
 #include "tplg_local.h"
 
 #define SAVE_ALLOC_SHIFT	(13)	/* 8192 bytes */
+#define PRINT_BUF_SIZE		(1024)
+#define PRINT_BUF_SIZE_MAX	(1024 * 1024)
 
 int tplg_save_printf(char **dst, const char *pfx, const char *fmt, ...)
 {
 	va_list va;
-	char buf[1024], *s;
-	size_t n, l, t, pl;
+	char *buf, *s;
+	size_t n, alloc_size = PRINT_BUF_SIZE, l, t, pl;
+	int ret = 0;
+
+	buf = malloc(alloc_size);
+	if (!buf)
+		return -ENOMEM;
 
 	if (pfx == NULL)
 		pfx = "";
 
+again:
 	va_start(va, fmt);
-	n = vsnprintf(buf, sizeof(buf), fmt, va);
+	n = vsnprintf(buf, alloc_size, fmt, va);
 	va_end(va);
 
-	if (n >= sizeof(buf))
-		return -EOVERFLOW;
+	if (n >= PRINT_BUF_SIZE_MAX) {
+		ret = -EOVERFLOW;
+		goto end;
+	}
+
+	if (n >= alloc_size) {
+		char *tmp = realloc(buf, n + 1);
+		if (!tmp) {
+			ret = -ENOMEM;
+			goto end;
+		}
+		buf = tmp;
+		alloc_size = n + 1;
+		goto again;
+	}
 
 	pl = strlen(pfx);
 	l = *dst ? strlen(*dst) : 0;
@@ -47,7 +68,8 @@ int tplg_save_printf(char **dst, const char *pfx, const char *fmt, ...)
 		if (s == NULL) {
 			free(*dst);
 			*dst = NULL;
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto end;
 		}
 	} else {
 		s = *dst;
@@ -57,6 +79,8 @@ int tplg_save_printf(char **dst, const char *pfx, const char *fmt, ...)
 		strcpy(s + l, pfx);
 	strcpy(s + l + pl, buf);
 	*dst = s;
+end:
+	free(buf);
 	return 0;
 }
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation
  2020-06-25 11:03 [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Piotr Maziarz
                   ` (6 preceding siblings ...)
  2020-06-25 11:03 ` [PATCH alsa-lib 8/8] topology: Make buffer for saving dynamic size Piotr Maziarz
@ 2020-06-25 14:16 ` Pierre-Louis Bossart
  7 siblings, 0 replies; 12+ messages in thread
From: Pierre-Louis Bossart @ 2020-06-25 14:16 UTC (permalink / raw)
  To: Piotr Maziarz, alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski



On 6/25/20 6:03 AM, Piotr Maziarz wrote:
> Without proper memory allocation behaviour was undefined.

Maybe elaborate to explain that memory allocated on the stack was 
referenced outside of the function scope?

> Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
> ---
>   src/topology/ctl.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/src/topology/ctl.c b/src/topology/ctl.c
> index 90241b6..c8c7e94 100644
> --- a/src/topology/ctl.c
> +++ b/src/topology/ctl.c
> @@ -1330,7 +1330,6 @@ int tplg_decode_control_enum1(snd_tplg_t *tplg,
>   			      void *bin, size_t size)
>   {
>   	struct snd_soc_tplg_enum_control *ec = bin;
> -	struct snd_tplg_channel_map_template cmt;
>   	int i;
>   
>   	if (size < sizeof(*ec)) {
> @@ -1375,11 +1374,11 @@ int tplg_decode_control_enum1(snd_tplg_t *tplg,
>   		}
>   	}
>   
> -	et->map = &cmt;
> -	memset(&cmt, 0, sizeof(cmt));
> -	cmt.num_channels = ec->num_channels;
> -	for (i = 0; i < cmt.num_channels; i++) {
> -		struct snd_tplg_channel_elem *channel = &cmt.channel[i];
> +	et->map = tplg_calloc(heap, sizeof(struct snd_tplg_channel_map_template));

if (!et->map)
     return -ENOMEM;

> +	et->map->num_channels = ec->num_channels;
> +	for (i = 0; i < et->map->num_channels; i++) {
> +		struct snd_tplg_channel_elem *channel = &et->map->channel[i];
> +
>   		tplg_log(tplg, 'D', pos + ((void *)&ec->channel[i] - (void *)ec),
>   			 "enum: channel size %d", ec->channel[i].size);
>   		channel->reg = ec->channel[i].reg;
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH alsa-lib 2/8] topology: decode: Fix adding texts field to enum control
  2020-06-25 11:03 ` [PATCH alsa-lib 2/8] topology: decode: Fix adding texts field to enum control Piotr Maziarz
@ 2020-06-25 14:24   ` Pierre-Louis Bossart
  0 siblings, 0 replies; 12+ messages in thread
From: Pierre-Louis Bossart @ 2020-06-25 14:24 UTC (permalink / raw)
  To: Piotr Maziarz, alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski



On 6/25/20 6:03 AM, Piotr Maziarz wrote:
> Iterating over texts in tplg_decode_control_enum1 was an infinite loop,

that should probably be a separate fix?

> it needed to be fixed. Parsing values was removed since they are not added
> to the UML file.

What does this mean? first time I hear about UML for topology.

> Also texts are separate section and therefore it needs to be added as
> separate element.

separate patch?

> Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
> ---
>   src/topology/ctl.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/src/topology/ctl.c b/src/topology/ctl.c
> index c8c7e94..24eadc8 100644
> --- a/src/topology/ctl.c
> +++ b/src/topology/ctl.c
> @@ -1088,11 +1088,19 @@ int tplg_add_enum(snd_tplg_t *tplg, struct snd_tplg_enum_template *enum_ctl,
>   	}
>   
>   	if (enum_ctl->texts != NULL) {
> +		struct tplg_elem *texts = tplg_elem_new_common(tplg, NULL,
> +						enum_ctl->hdr.name, SND_TPLG_TYPE_TEXT);
> +
> +		texts->texts->num_items = num_items;
>   		for (i = 0; i < num_items; i++) {
> -			if (enum_ctl->texts[i] != NULL)
> -				snd_strlcpy(ec->texts[i], enum_ctl->texts[i],
> -					    SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
> +			if (!enum_ctl->texts[i])
> +				continue;
> +			snd_strlcpy(ec->texts[i], enum_ctl->texts[i],
> +				    SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
> +			snd_strlcpy(texts->texts->items[i], enum_ctl->texts[i],
> +				    SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
>   		}
> +		tplg_ref_add(elem, SND_TPLG_TYPE_TEXT, enum_ctl->hdr.name);
>   	}
>   
>   	if (enum_ctl->values != NULL) {
> @@ -1367,11 +1375,8 @@ int tplg_decode_control_enum1(snd_tplg_t *tplg,
>   		et->texts = tplg_calloc(heap, sizeof(char *) * ec->items);
>   		if (!et->texts)
>   			return -ENOMEM;
> -		for (i = 0; ec->items; i++) {
> -			unsigned int j = i * sizeof(int) * ENUM_VAL_SIZE;
> +		for (i = 0; i < ec->items; i++)

This is the infinite loop, this should not be buried in unrelated changes.

>   			et->texts[i] = ec->texts[i];
> -			et->values[i] = (int *)&ec->values[j];
> -		}
>   	}
>   
>   	et->map = tplg_calloc(heap, sizeof(struct snd_tplg_channel_map_template));
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH alsa-lib 8/8] topology: Make buffer for saving dynamic size
  2020-06-25 11:03 ` [PATCH alsa-lib 8/8] topology: Make buffer for saving dynamic size Piotr Maziarz
@ 2020-06-25 14:31   ` Pierre-Louis Bossart
  2020-07-02 15:04     ` Piotr Maziarz
  0 siblings, 1 reply; 12+ messages in thread
From: Pierre-Louis Bossart @ 2020-06-25 14:31 UTC (permalink / raw)
  To: Piotr Maziarz, alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski



On 6/25/20 6:03 AM, Piotr Maziarz wrote:
> Some fields can exceed size limit, e.g. private data has no size
> restriction. Therefore it needs to be dynamically increased.
> 
> Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
> ---
>   src/topology/save.c | 36 ++++++++++++++++++++++++++++++------
>   1 file changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/src/topology/save.c b/src/topology/save.c
> index 4ecf86c..d6ee8b6 100644
> --- a/src/topology/save.c
> +++ b/src/topology/save.c
> @@ -19,22 +19,43 @@
>   #include "tplg_local.h"
>   
>   #define SAVE_ALLOC_SHIFT	(13)	/* 8192 bytes */
> +#define PRINT_BUF_SIZE		(1024)
> +#define PRINT_BUF_SIZE_MAX	(1024 * 1024)
>   
>   int tplg_save_printf(char **dst, const char *pfx, const char *fmt, ...)
>   {
>   	va_list va;
> -	char buf[1024], *s;
> -	size_t n, l, t, pl;
> +	char *buf, *s;
> +	size_t n, alloc_size = PRINT_BUF_SIZE, l, t, pl;
> +	int ret = 0;
> +
> +	buf = malloc(alloc_size);
> +	if (!buf)
> +		return -ENOMEM;
>   
>   	if (pfx == NULL)
>   		pfx = "";
>   
> +again:
>   	va_start(va, fmt);
> -	n = vsnprintf(buf, sizeof(buf), fmt, va);
> +	n = vsnprintf(buf, alloc_size, fmt, va);
>   	va_end(va);
>   
> -	if (n >= sizeof(buf))
> -		return -EOVERFLOW;
> +	if (n >= PRINT_BUF_SIZE_MAX) {
> +		ret = -EOVERFLOW;
> +		goto end;
> +	}

what this patch does is change the allocation limit from 1KB to 1MB, but 
the data still has no size restriction. At what point do we decide to 
throw an error?

> +
> +	if (n >= alloc_size) {
> +		char *tmp = realloc(buf, n + 1);
> +		if (!tmp) {
> +			ret = -ENOMEM;
> +			goto end;
> +		}
> +		buf = tmp;
> +		alloc_size = n + 1;
> +		goto again;
> +	}
>   
>   	pl = strlen(pfx);
>   	l = *dst ? strlen(*dst) : 0;
> @@ -47,7 +68,8 @@ int tplg_save_printf(char **dst, const char *pfx, const char *fmt, ...)
>   		if (s == NULL) {
>   			free(*dst);
>   			*dst = NULL;
> -			return -ENOMEM;
> +			ret = -ENOMEM;
> +			goto end;
>   		}
>   	} else {
>   		s = *dst;
> @@ -57,6 +79,8 @@ int tplg_save_printf(char **dst, const char *pfx, const char *fmt, ...)
>   		strcpy(s + l, pfx);
>   	strcpy(s + l + pl, buf);
>   	*dst = s;
> +end:
> +	free(buf);
>   	return 0;
>   }
>   
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH alsa-lib 8/8] topology: Make buffer for saving dynamic size
  2020-06-25 14:31   ` Pierre-Louis Bossart
@ 2020-07-02 15:04     ` Piotr Maziarz
  0 siblings, 0 replies; 12+ messages in thread
From: Piotr Maziarz @ 2020-07-02 15:04 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski

On 2020-06-25 16:31, Pierre-Louis Bossart wrote:
> 
> 
> On 6/25/20 6:03 AM, Piotr Maziarz wrote:
>> Some fields can exceed size limit, e.g. private data has no size
>> restriction. Therefore it needs to be dynamically increased.
>>
>> Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
>> ---
>>   src/topology/save.c | 36 ++++++++++++++++++++++++++++++------
>>   1 file changed, 30 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/topology/save.c b/src/topology/save.c
>> index 4ecf86c..d6ee8b6 100644
>> --- a/src/topology/save.c
>> +++ b/src/topology/save.c
>> @@ -19,22 +19,43 @@
>>   #include "tplg_local.h"
>>   #define SAVE_ALLOC_SHIFT    (13)    /* 8192 bytes */
>> +#define PRINT_BUF_SIZE        (1024)
>> +#define PRINT_BUF_SIZE_MAX    (1024 * 1024)
>>   int tplg_save_printf(char **dst, const char *pfx, const char *fmt, ...)
>>   {
>>       va_list va;
>> -    char buf[1024], *s;
>> -    size_t n, l, t, pl;
>> +    char *buf, *s;
>> +    size_t n, alloc_size = PRINT_BUF_SIZE, l, t, pl;
>> +    int ret = 0;
>> +
>> +    buf = malloc(alloc_size);
>> +    if (!buf)
>> +        return -ENOMEM;
>>       if (pfx == NULL)
>>           pfx = "";
>> +again:
>>       va_start(va, fmt);
>> -    n = vsnprintf(buf, sizeof(buf), fmt, va);
>> +    n = vsnprintf(buf, alloc_size, fmt, va);
>>       va_end(va);
>> -    if (n >= sizeof(buf))
>> -        return -EOVERFLOW;
>> +    if (n >= PRINT_BUF_SIZE_MAX) {
>> +        ret = -EOVERFLOW;
>> +        goto end;
>> +    }
> 
> what this patch does is change the allocation limit from 1KB to 1MB, but 
> the data still has no size restriction. At what point do we decide to 
> throw an error?
> 
If needed size is bigger than PRINT_BUF_SIZE_MAX there will be an error 
thrown so I don't know what should I change here. Or do you mean that 
private data size in binary should be restricted?


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-07-02 15:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-25 11:03 [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Piotr Maziarz
2020-06-25 11:03 ` [PATCH alsa-lib 2/8] topology: decode: Fix adding texts field to enum control Piotr Maziarz
2020-06-25 14:24   ` Pierre-Louis Bossart
2020-06-25 11:03 ` [PATCH alsa-lib 3/8] topology: decode: Fix printing texts section Piotr Maziarz
2020-06-25 11:03 ` [PATCH alsa-lib 4/8] topology: decode: Change declaration of enum decoding function Piotr Maziarz
2020-06-25 11:03 ` [PATCH alsa-lib 5/8] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
2020-06-25 11:03 ` [PATCH alsa-lib 6/8] topology: decode: Print sig_bits field in PCM capabilities section Piotr Maziarz
2020-06-25 11:03 ` [PATCH alsa-lib 7/8] topology: decode: Add DAI name printing Piotr Maziarz
2020-06-25 11:03 ` [PATCH alsa-lib 8/8] topology: Make buffer for saving dynamic size Piotr Maziarz
2020-06-25 14:31   ` Pierre-Louis Bossart
2020-07-02 15:04     ` Piotr Maziarz
2020-06-25 14:16 ` [PATCH alsa-lib 1/8] topology: decode: fix channel map memory allocation Pierre-Louis Bossart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.