[alsa-lib,8/8] topology: Make buffer for saving dynamic size
diff mbox series

Message ID 1593083026-7501-8-git-send-email-piotrx.maziarz@linux.intel.com
State New
Headers show
Series
  • [alsa-lib,1/8] topology: decode: fix channel map memory allocation
Related show

Commit Message

Piotr Maziarz June 25, 2020, 11:03 a.m. UTC
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(-)

Comments

Pierre-Louis Bossart June 25, 2020, 2:31 p.m. UTC | #1
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;
>   }
>   
>
Piotr Maziarz July 2, 2020, 3:04 p.m. UTC | #2
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?

Patch
diff mbox series

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;
 }