All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/10] topology: decode: Various fixes
@ 2020-08-31  9:08 Piotr Maziarz
  2020-08-31  9:08 ` [PATCH v4 01/10] topology: decode: Fix channel map memory allocation Piotr Maziarz
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:08 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

This series fixes various problems with topology decoding mechanism.
Some of the problems were critical like improper memory management or
infinite loops that were causing undefined behaviour or program crashes,
while other resulted in losing some data during conversion.

Bugs found while testing with Intel SST topologies.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Changelog:
v2:
  -Divide into more patches, critical fixes are in separate patches now
  -More specific descriptions
  -Fix a typo UML to UCM
  -Add error reporting in topology: decode: fix channel map memory
   allocation
  -Remove goto again in topology: Make buffer for saving dynamic size
   for better readability

v3:
  -No functional changes
  -Changed UCM to more descriptive standard ALSA configuration files
  -removed Gerrit's Change-Id
  -Added missing signed-offs

v4:
  -Resend, added Reviewed-by tags

Piotr Maziarz (10):
  topology: decode: Fix channel map memory allocation
  topology: decode: Fix infinite loop in decoding enum control
  topology: decode: Remove decoding  values for enum control
  topology: decode: Add enum control texts as separate element
  topology: decode: Fix printing texts section
  topology: decode: Change declaration of enum decoding function
  topology: decode: Fix decoding PCM formats and rates
  topology: decode: Print sig_bits field in PCM capabilities section
  topology: decode: Add DAI name printing
  topology: Make buffer for saving dynamic size

 src/topology/ctl.c        | 51 ++++++++++++++++++++++-------------------------
 src/topology/dapm.c       |  3 +--
 src/topology/pcm.c        | 11 +++++++---
 src/topology/save.c       | 34 ++++++++++++++++++++++++++-----
 src/topology/text.c       |  2 +-
 src/topology/tplg_local.h |  2 +-
 6 files changed, 64 insertions(+), 39 deletions(-)

-- 
2.7.4


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

* [PATCH v4 01/10] topology: decode: Fix channel map memory allocation
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
@ 2020-08-31  9:08 ` Piotr Maziarz
  2020-08-31  9:08 ` [PATCH v4 02/10] topology: decode: Fix infinite loop in decoding enum control Piotr Maziarz
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:08 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

Memory allocated on the stack was referenced outside of the function scope
caused undefined behaviour.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 src/topology/ctl.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index 90241b6..6e6c1d1 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,13 @@ 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;
-- 
2.7.4


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

* [PATCH v4 02/10] topology: decode: Fix infinite loop in decoding enum control
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
  2020-08-31  9:08 ` [PATCH v4 01/10] topology: decode: Fix channel map memory allocation Piotr Maziarz
@ 2020-08-31  9:08 ` Piotr Maziarz
  2020-08-31  9:08 ` [PATCH v4 03/10] topology: decode: Remove decoding values for " Piotr Maziarz
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:08 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

Accessing memory outside of allocated boundaries caused segmentation fault.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 src/topology/ctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index 6e6c1d1..0aa49ab 100644
--- a/src/topology/ctl.c
+++ b/src/topology/ctl.c
@@ -1367,7 +1367,7 @@ 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++) {
+		for (i = 0; i < ec->items; i++) {
 			unsigned int j = i * sizeof(int) * ENUM_VAL_SIZE;
 			et->texts[i] = ec->texts[i];
 			et->values[i] = (int *)&ec->values[j];
-- 
2.7.4


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

* [PATCH v4 03/10] topology: decode: Remove decoding values for enum control
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
  2020-08-31  9:08 ` [PATCH v4 01/10] topology: decode: Fix channel map memory allocation Piotr Maziarz
  2020-08-31  9:08 ` [PATCH v4 02/10] topology: decode: Fix infinite loop in decoding enum control Piotr Maziarz
@ 2020-08-31  9:08 ` Piotr Maziarz
  2020-08-31  9:08 ` [PATCH v4 04/10] topology: decode: Add enum control texts as separate element Piotr Maziarz
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:08 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

Values have no representation in standard ALSA configuration files,
therefore there is no need to populate them. Also memory for values
wasn't allocated which was causing undefined behaviour.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 src/topology/ctl.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index 0aa49ab..02e482e 100644
--- a/src/topology/ctl.c
+++ b/src/topology/ctl.c
@@ -1367,11 +1367,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; i < 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] 14+ messages in thread

* [PATCH v4 04/10] topology: decode: Add enum control texts as separate element
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
                   ` (2 preceding siblings ...)
  2020-08-31  9:08 ` [PATCH v4 03/10] topology: decode: Remove decoding values for " Piotr Maziarz
@ 2020-08-31  9:08 ` Piotr Maziarz
  2020-08-31  9:08 ` [PATCH v4 05/10] topology: decode: Fix printing texts section Piotr Maziarz
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:08 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

Texts are separate sections that should referenced by enum control.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 src/topology/ctl.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index 02e482e..1f39846 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) {
-- 
2.7.4


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

* [PATCH v4 05/10] topology: decode: Fix printing texts section
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
                   ` (3 preceding siblings ...)
  2020-08-31  9:08 ` [PATCH v4 04/10] topology: decode: Add enum control texts as separate element Piotr Maziarz
@ 2020-08-31  9:08 ` Piotr Maziarz
  2020-08-31  9:08 ` [PATCH v4 06/10] topology: decode: Change declaration of enum decoding function Piotr Maziarz
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:08 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@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] 14+ messages in thread

* [PATCH v4 06/10] topology: decode: Change declaration of enum decoding function
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
                   ` (4 preceding siblings ...)
  2020-08-31  9:08 ` [PATCH v4 05/10] topology: decode: Fix printing texts section Piotr Maziarz
@ 2020-08-31  9:08 ` Piotr Maziarz
  2020-08-31  9:09 ` [PATCH v4 07/10] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:08 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

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>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@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 1f39846..47db400 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);
@@ -1427,7 +1415,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] 14+ messages in thread

* [PATCH v4 07/10] topology: decode: Fix decoding PCM formats and rates
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
                   ` (5 preceding siblings ...)
  2020-08-31  9:08 ` [PATCH v4 06/10] topology: decode: Change declaration of enum decoding function Piotr Maziarz
@ 2020-08-31  9:09 ` Piotr Maziarz
  2020-08-31  9:09 ` [PATCH v4 08/10] topology: decode: Print sig_bits field in PCM capabilities section Piotr Maziarz
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:09 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

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

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@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] 14+ messages in thread

* [PATCH v4 08/10] topology: decode: Print sig_bits field in PCM capabilities section
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
                   ` (6 preceding siblings ...)
  2020-08-31  9:09 ` [PATCH v4 07/10] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
@ 2020-08-31  9:09 ` Piotr Maziarz
  2020-08-31  9:09 ` [PATCH v4 09/10] topology: decode: Add DAI name printing Piotr Maziarz
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:09 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

Not printing this field makes data loss while converting from binary
to standard ALSA configuration file.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@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] 14+ messages in thread

* [PATCH v4 09/10] topology: decode: Add DAI name printing
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
                   ` (7 preceding siblings ...)
  2020-08-31  9:09 ` [PATCH v4 08/10] topology: decode: Print sig_bits field in PCM capabilities section Piotr Maziarz
@ 2020-08-31  9:09 ` Piotr Maziarz
  2020-08-31  9:09 ` [PATCH v4 10/10] topology: Make buffer for saving dynamic size Piotr Maziarz
  2020-08-31 11:11 ` [PATCH v4 00/10] topology: decode: Various fixes Jaroslav Kysela
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:09 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

DAI name is a part of topology binary. Not printing makes data loss while
converting from binary to standard ALSA configuration file.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@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] 14+ messages in thread

* [PATCH v4 10/10] topology: Make buffer for saving dynamic size
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
                   ` (8 preceding siblings ...)
  2020-08-31  9:09 ` [PATCH v4 09/10] topology: decode: Add DAI name printing Piotr Maziarz
@ 2020-08-31  9:09 ` Piotr Maziarz
  2020-08-31 11:11 ` [PATCH v4 00/10] topology: decode: Various fixes Jaroslav Kysela
  10 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-08-31  9:09 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, amadeuszx.slawinski, tiwai

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>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 src/topology/save.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/src/topology/save.c b/src/topology/save.c
index 4ecf86c..9c74735 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;
+	char *buf, *s;
 	size_t n, l, t, pl;
+	int ret = 0;
+
+	buf = malloc(PRINT_BUF_SIZE);
+	if (!buf)
+		return -ENOMEM;
 
 	if (pfx == NULL)
 		pfx = "";
 
 	va_start(va, fmt);
-	n = vsnprintf(buf, sizeof(buf), fmt, va);
+	n = vsnprintf(buf, PRINT_BUF_SIZE, fmt, va);
 	va_end(va);
 
-	if (n >= sizeof(buf))
-		return -EOVERFLOW;
+	if (n >= PRINT_BUF_SIZE_MAX) {
+		ret = -EOVERFLOW;
+		goto end;
+	}
+
+	if (n >= PRINT_BUF_SIZE) {
+		char *tmp = realloc(buf, n + 1);
+		if (!tmp) {
+			ret = -ENOMEM;
+			goto end;
+		}
+		buf = tmp;
+		va_start(va, fmt);
+		n = vsnprintf(buf, n + 1, fmt, va);
+		va_end(va);
+	}
 
 	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] 14+ messages in thread

* Re: [PATCH v4 00/10] topology: decode: Various fixes
  2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
                   ` (9 preceding siblings ...)
  2020-08-31  9:09 ` [PATCH v4 10/10] topology: Make buffer for saving dynamic size Piotr Maziarz
@ 2020-08-31 11:11 ` Jaroslav Kysela
  2020-08-31 17:57   ` Jaroslav Kysela
  10 siblings, 1 reply; 14+ messages in thread
From: Jaroslav Kysela @ 2020-08-31 11:11 UTC (permalink / raw)
  To: Piotr Maziarz, alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, tiwai, amadeuszx.slawinski

Dne 31. 08. 20 v 11:08 Piotr Maziarz napsal(a):
> This series fixes various problems with topology decoding mechanism.
> Some of the problems were critical like improper memory management or
> infinite loops that were causing undefined behaviour or program crashes,
> while other resulted in losing some data during conversion.
> 
> Bugs found while testing with Intel SST topologies.

Thank you for this work. I applied all patches to the alsa-lib repository. I
dislike the last one - dynamic allocation for each printf(), but I applied it
until we found a better solution.

We may use the dynamic allocation only when the printf is bigger than the 1024
bytes threshold (and keep the small buffer on stack otherwise) or create 'dst'
structure which will carry the output buffer point and the temporary buffer
pointer which will be freed when the output is finished.

				Thank you,
					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

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

* Re: [PATCH v4 00/10] topology: decode: Various fixes
  2020-08-31 11:11 ` [PATCH v4 00/10] topology: decode: Various fixes Jaroslav Kysela
@ 2020-08-31 17:57   ` Jaroslav Kysela
  2020-09-03  8:10     ` Piotr Maziarz
  0 siblings, 1 reply; 14+ messages in thread
From: Jaroslav Kysela @ 2020-08-31 17:57 UTC (permalink / raw)
  To: Piotr Maziarz, alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, tiwai, amadeuszx.slawinski

Dne 31. 08. 20 v 13:11 Jaroslav Kysela napsal(a):
> Dne 31. 08. 20 v 11:08 Piotr Maziarz napsal(a):
>> This series fixes various problems with topology decoding mechanism.
>> Some of the problems were critical like improper memory management or
>> infinite loops that were causing undefined behaviour or program crashes,
>> while other resulted in losing some data during conversion.
>>
>> Bugs found while testing with Intel SST topologies.
> 
> Thank you for this work. I applied all patches to the alsa-lib repository. I
> dislike the last one - dynamic allocation for each printf(), but I applied it
> until we found a better solution.
> 
> We may use the dynamic allocation only when the printf is bigger than the 1024
> bytes threshold (and keep the small buffer on stack otherwise) or create 'dst'
> structure which will carry the output buffer point and the temporary buffer
> pointer which will be freed when the output is finished.

I implemented the later suggestion in:

https://github.com/alsa-project/alsa-lib/commit/472ab5db67a0ed04de634214773e7b17d10b5415

There are also other fixes in the topology library. It would be nice, if you
can give a test.

			Thank you,
				Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

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

* Re: [PATCH v4 00/10] topology: decode: Various fixes
  2020-08-31 17:57   ` Jaroslav Kysela
@ 2020-09-03  8:10     ` Piotr Maziarz
  0 siblings, 0 replies; 14+ messages in thread
From: Piotr Maziarz @ 2020-09-03  8:10 UTC (permalink / raw)
  To: Jaroslav Kysela, alsa-devel
  Cc: pierre-louis.bossart, cezary.rojewski, tiwai, amadeuszx.slawinski

On 2020-08-31 19:57, Jaroslav Kysela wrote:
> Dne 31. 08. 20 v 13:11 Jaroslav Kysela napsal(a):
>> Dne 31. 08. 20 v 11:08 Piotr Maziarz napsal(a):
>>> This series fixes various problems with topology decoding mechanism.
>>> Some of the problems were critical like improper memory management or
>>> infinite loops that were causing undefined behaviour or program crashes,
>>> while other resulted in losing some data during conversion.
>>>
>>> Bugs found while testing with Intel SST topologies.
>>
>> Thank you for this work. I applied all patches to the alsa-lib repository. I
>> dislike the last one - dynamic allocation for each printf(), but I applied it
>> until we found a better solution.
>>
>> We may use the dynamic allocation only when the printf is bigger than the 1024
>> bytes threshold (and keep the small buffer on stack otherwise) or create 'dst'
>> structure which will carry the output buffer point and the temporary buffer
>> pointer which will be freed when the output is finished.
> 
> I implemented the later suggestion in:
> 
> https://github.com/alsa-project/alsa-lib/commit/472ab5db67a0ed04de634214773e7b17d10b5415
> 
> There are also other fixes in the topology library. It would be nice, if you
> can give a test.
> 
> 			Thank you,
> 				Jaroslav
> 
I've tested your changes on a bunch of our topologies, everything works 
fine.
Thanks for merge and update.

Best Regards,
Piotr

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

end of thread, other threads:[~2020-09-03  8:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-31  9:08 [PATCH v4 00/10] topology: decode: Various fixes Piotr Maziarz
2020-08-31  9:08 ` [PATCH v4 01/10] topology: decode: Fix channel map memory allocation Piotr Maziarz
2020-08-31  9:08 ` [PATCH v4 02/10] topology: decode: Fix infinite loop in decoding enum control Piotr Maziarz
2020-08-31  9:08 ` [PATCH v4 03/10] topology: decode: Remove decoding values for " Piotr Maziarz
2020-08-31  9:08 ` [PATCH v4 04/10] topology: decode: Add enum control texts as separate element Piotr Maziarz
2020-08-31  9:08 ` [PATCH v4 05/10] topology: decode: Fix printing texts section Piotr Maziarz
2020-08-31  9:08 ` [PATCH v4 06/10] topology: decode: Change declaration of enum decoding function Piotr Maziarz
2020-08-31  9:09 ` [PATCH v4 07/10] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
2020-08-31  9:09 ` [PATCH v4 08/10] topology: decode: Print sig_bits field in PCM capabilities section Piotr Maziarz
2020-08-31  9:09 ` [PATCH v4 09/10] topology: decode: Add DAI name printing Piotr Maziarz
2020-08-31  9:09 ` [PATCH v4 10/10] topology: Make buffer for saving dynamic size Piotr Maziarz
2020-08-31 11:11 ` [PATCH v4 00/10] topology: decode: Various fixes Jaroslav Kysela
2020-08-31 17:57   ` Jaroslav Kysela
2020-09-03  8:10     ` Piotr Maziarz

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.