* [PATCH v2 00/10] topology: decode: Various fixes
@ 2020-07-06 9:05 Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 01/10] topology: decode: Fix channel map memory allocation Piotr Maziarz
` (10 more replies)
0 siblings, 11 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:05 UTC (permalink / raw)
To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
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.
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
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] 15+ messages in thread
* [PATCH v2 01/10] topology: decode: Fix channel map memory allocation
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
@ 2020-07-06 9:05 ` Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 02/10] topology: decode: Fix infinite loop in decoding enum control Piotr Maziarz
` (9 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:05 UTC (permalink / raw)
To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
Memory allocated on the stack was referenced outside of the function scope
caused undefined behaviour.
Change-Id: Iac0bfe7dabfd59494b78afba17cf2d3e0b429fef
Signed-off-by: Piotr Maziarz <piotrx.maziarz@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] 15+ messages in thread
* [PATCH v2 02/10] topology: decode: Fix infinite loop in decoding enum control
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 01/10] topology: decode: Fix channel map memory allocation Piotr Maziarz
@ 2020-07-06 9:05 ` Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 03/10] topology: decode: Remove decoding values for " Piotr Maziarz
` (8 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:05 UTC (permalink / raw)
To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
Accessing memory outside of allocated boundaries caused segmentation fault.
Change-Id: Ide6b4d0e6ee7801bb7185e286be5d2fbf29695a6
---
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] 15+ messages in thread
* [PATCH v2 03/10] topology: decode: Remove decoding values for enum control
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 01/10] topology: decode: Fix channel map memory allocation Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 02/10] topology: decode: Fix infinite loop in decoding enum control Piotr Maziarz
@ 2020-07-06 9:05 ` Piotr Maziarz
2020-07-06 19:54 ` Pierre-Louis Bossart
2020-07-06 9:05 ` [PATCH v2 04/10] topology: decode: Add enum control texts as separate element Piotr Maziarz
` (7 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:05 UTC (permalink / raw)
To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
Values have no representation in UCM file, therefore there is no need to
populate them. Also memory for values wasn't allocated which was causing
undefined behaviour.
Change-Id: I86b990d20808091eb61fb6710b35a0c1f1cbce10
---
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] 15+ messages in thread
* [PATCH v2 04/10] topology: decode: Add enum control texts as separate element
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
` (2 preceding siblings ...)
2020-07-06 9:05 ` [PATCH v2 03/10] topology: decode: Remove decoding values for " Piotr Maziarz
@ 2020-07-06 9:05 ` Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 05/10] topology: decode: Fix printing texts section Piotr Maziarz
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:05 UTC (permalink / raw)
To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
Texts are separate sections that should referenced by enum control.
Change-Id: I7b97803da13478c642d003b78ce12be1eedbf802
---
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] 15+ messages in thread
* [PATCH v2 05/10] topology: decode: Fix printing texts section
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
` (3 preceding siblings ...)
2020-07-06 9:05 ` [PATCH v2 04/10] topology: decode: Add enum control texts as separate element Piotr Maziarz
@ 2020-07-06 9:05 ` Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 06/10] topology: decode: Change declaration of enum decoding function Piotr Maziarz
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:05 UTC (permalink / raw)
To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
Change-Id: Ie00ca47b18f527140b63b8c31e948f8850dfe11f
---
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] 15+ messages in thread
* [PATCH v2 06/10] topology: decode: Change declaration of enum decoding function
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
` (4 preceding siblings ...)
2020-07-06 9:05 ` [PATCH v2 05/10] topology: decode: Fix printing texts section Piotr Maziarz
@ 2020-07-06 9:05 ` Piotr Maziarz
2020-07-06 9:06 ` [PATCH v2 07/10] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:05 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.
Change-Id: I8bdd93cb1534ae9217aee87cb2b267e175aa483f
---
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] 15+ messages in thread
* [PATCH v2 07/10] topology: decode: Fix decoding PCM formats and rates
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
` (5 preceding siblings ...)
2020-07-06 9:05 ` [PATCH v2 06/10] topology: decode: Change declaration of enum decoding function Piotr Maziarz
@ 2020-07-06 9:06 ` Piotr Maziarz
2020-07-06 19:58 ` Pierre-Louis Bossart
2020-07-06 9:06 ` [PATCH v2 08/10] topology: decode: Print sig_bits field in PCM capabilities section Piotr Maziarz
` (3 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:06 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 UCM.
Change-Id: I71ac42b65e8b1d79f2a061f805c62e29a758ad74
---
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] 15+ messages in thread
* [PATCH v2 08/10] topology: decode: Print sig_bits field in PCM capabilities section
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
` (6 preceding siblings ...)
2020-07-06 9:06 ` [PATCH v2 07/10] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
@ 2020-07-06 9:06 ` Piotr Maziarz
2020-07-06 9:06 ` [PATCH v2 09/10] topology: decode: Add DAI name printing Piotr Maziarz
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:06 UTC (permalink / raw)
To: alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
Not printing this field makes data loss while converting from binary
to UCM.
Change-Id: Idba0c8bb537e7fc8af78f697781032005fa78d63
---
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] 15+ messages in thread
* [PATCH v2 09/10] topology: decode: Add DAI name printing
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
` (7 preceding siblings ...)
2020-07-06 9:06 ` [PATCH v2 08/10] topology: decode: Print sig_bits field in PCM capabilities section Piotr Maziarz
@ 2020-07-06 9:06 ` Piotr Maziarz
2020-07-06 9:06 ` [PATCH v2 10/10] topology: Make buffer for saving dynamic size Piotr Maziarz
2020-07-06 20:01 ` [PATCH v2 00/10] topology: decode: Various fixes Pierre-Louis Bossart
10 siblings, 0 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:06 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 UCM.
Change-Id: I57307108b5e922d18db2f155b9237db3fe175d7f
---
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] 15+ messages in thread
* [PATCH v2 10/10] topology: Make buffer for saving dynamic size
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
` (8 preceding siblings ...)
2020-07-06 9:06 ` [PATCH v2 09/10] topology: decode: Add DAI name printing Piotr Maziarz
@ 2020-07-06 9:06 ` Piotr Maziarz
2020-07-06 20:01 ` [PATCH v2 00/10] topology: decode: Various fixes Pierre-Louis Bossart
10 siblings, 0 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-06 9:06 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.
Change-Id: Ie274fd304eba5d600e198f4febbc6216f01b57e1
---
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] 15+ messages in thread
* Re: [PATCH v2 03/10] topology: decode: Remove decoding values for enum control
2020-07-06 9:05 ` [PATCH v2 03/10] topology: decode: Remove decoding values for " Piotr Maziarz
@ 2020-07-06 19:54 ` Pierre-Louis Bossart
0 siblings, 0 replies; 15+ messages in thread
From: Pierre-Louis Bossart @ 2020-07-06 19:54 UTC (permalink / raw)
To: Piotr Maziarz, alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
On 7/6/20 4:05 AM, Piotr Maziarz wrote:
> Values have no representation in UCM file, therefore there is no need to
Sorry, I don't get the logical assertion here. What is the link or
dependency between UCM and topology?
> populate them. Also memory for values wasn't allocated which was causing
> undefined behaviour.
>
> Change-Id: I86b990d20808091eb61fb6710b35a0c1f1cbce10
> ---
> 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));
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 07/10] topology: decode: Fix decoding PCM formats and rates
2020-07-06 9:06 ` [PATCH v2 07/10] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
@ 2020-07-06 19:58 ` Pierre-Louis Bossart
0 siblings, 0 replies; 15+ messages in thread
From: Pierre-Louis Bossart @ 2020-07-06 19:58 UTC (permalink / raw)
To: Piotr Maziarz, alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
On 7/6/20 4:06 AM, Piotr Maziarz wrote:
> Not checking _LAST format and rate, which are valid indexes in arrays,
> makes data loss while converting binary to UCM.
In the previous version you were decoding to UML. I presume this was
typo, but now what does decoding a topology binary to UCM means? Did you
really mean UCM as in Use Case Management
(https://www.alsa-project.org/alsa-doc/alsa-lib/group__ucm.html)
> Change-Id: I71ac42b65e8b1d79f2a061f805c62e29a758ad74
> ---
> 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",
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 00/10] topology: decode: Various fixes
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
` (9 preceding siblings ...)
2020-07-06 9:06 ` [PATCH v2 10/10] topology: Make buffer for saving dynamic size Piotr Maziarz
@ 2020-07-06 20:01 ` Pierre-Louis Bossart
2020-07-14 12:13 ` Piotr Maziarz
10 siblings, 1 reply; 15+ messages in thread
From: Pierre-Louis Bossart @ 2020-07-06 20:01 UTC (permalink / raw)
To: Piotr Maziarz, alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
On 7/6/20 4:05 AM, Piotr Maziarz wrote:
> 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.
>
> Changelog:
> v2:
> -Divide into more patches, critical fixes are in separate patches now
> -More specific descriptions
> -Fix a typo UML to UCM
That fix makes it even more confusing, I get that a UCM file can set
values for controls defined in a topology file, but 'decoding to UCM'
leaves me wondering what you are referring to.
Also you may want to remove all the Gerrit ChangeId before sending to
the mailing list.
> -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
>
> 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(-)
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 00/10] topology: decode: Various fixes
2020-07-06 20:01 ` [PATCH v2 00/10] topology: decode: Various fixes Pierre-Louis Bossart
@ 2020-07-14 12:13 ` Piotr Maziarz
0 siblings, 0 replies; 15+ messages in thread
From: Piotr Maziarz @ 2020-07-14 12:13 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel; +Cc: cezary.rojewski, amadeuszx.slawinski
On 2020-07-06 22:01, Pierre-Louis Bossart wrote:
>
>
> On 7/6/20 4:05 AM, Piotr Maziarz wrote:
>> 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.
>>
>> Changelog:
>> v2:
>> -Divide into more patches, critical fixes are in separate patches now
>> -More specific descriptions
>> -Fix a typo UML to UCM
>
> That fix makes it even more confusing, I get that a UCM file can set
> values for controls defined in a topology file, but 'decoding to UCM'
> leaves me wondering what you are referring to.
>
I meant standard ALSA configuration file format that is used also by UCM
files. I'll change it for more clarity.
> Also you may want to remove all the Gerrit ChangeId before sending to
> the mailing list.
>
>> -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
>>
>> 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(-)
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-07-14 12:14 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-06 9:05 [PATCH v2 00/10] topology: decode: Various fixes Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 01/10] topology: decode: Fix channel map memory allocation Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 02/10] topology: decode: Fix infinite loop in decoding enum control Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 03/10] topology: decode: Remove decoding values for " Piotr Maziarz
2020-07-06 19:54 ` Pierre-Louis Bossart
2020-07-06 9:05 ` [PATCH v2 04/10] topology: decode: Add enum control texts as separate element Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 05/10] topology: decode: Fix printing texts section Piotr Maziarz
2020-07-06 9:05 ` [PATCH v2 06/10] topology: decode: Change declaration of enum decoding function Piotr Maziarz
2020-07-06 9:06 ` [PATCH v2 07/10] topology: decode: Fix decoding PCM formats and rates Piotr Maziarz
2020-07-06 19:58 ` Pierre-Louis Bossart
2020-07-06 9:06 ` [PATCH v2 08/10] topology: decode: Print sig_bits field in PCM capabilities section Piotr Maziarz
2020-07-06 9:06 ` [PATCH v2 09/10] topology: decode: Add DAI name printing Piotr Maziarz
2020-07-06 9:06 ` [PATCH v2 10/10] topology: Make buffer for saving dynamic size Piotr Maziarz
2020-07-06 20:01 ` [PATCH v2 00/10] topology: decode: Various fixes Pierre-Louis Bossart
2020-07-14 12:13 ` Piotr Maziarz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).