Re: [dpdk-dev] [PATCH] test: fix sprintf with snprintf

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Feb 08, 2019 at 01:27:37PM +0000, Pallantla Poornima wrote:
> sprintf function is not secure as it doesn't check the length of string.
> More secure function snprintf is used.
> 
> Fixes: 727909c592 ("app/test: introduce dynamic commands list")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Pallantla Poornima <pallantlax.poornima@intel.com>
> ---
>  test/test/commands.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/test/test/commands.c b/test/test/commands.c
> index 94fbc310e..5aeb35498 100644
> --- a/test/test/commands.c
> +++ b/test/test/commands.c
> @@ -367,6 +367,8 @@ int commands_init(void)
>  	struct test_command *t;
>  	char *commands, *ptr;
>  	int commands_len = 0;
> +	int total_written = 0;
> +	int count = 0;
>  
>  	TAILQ_FOREACH(t, &commands_list, next) {
>  		commands_len += strlen(t->command) + 1;
> @@ -378,7 +380,10 @@ int commands_init(void)
>  
>  	ptr = commands;
>  	TAILQ_FOREACH(t, &commands_list, next) {
> -		ptr += sprintf(ptr, "%s#", t->command);
> +		count = snprintf(ptr, commands_len - total_written - 1, "%s#",
> +				t->command);
> +		ptr += count;
> +		total_written += count;
>  	}

I don't think the "-1" should be necessary here. Also, I think you should
check the return value of snprintf to check for truncation, and abort the
loop if so.

/Bruce