[dpdk-dev] [PATCH] app/test: fix +/-1 error in allocation

[dpdk-dev] [PATCH] app/test: fix +/-1 error in allocation

[Jan Viktorin]

A bug has been detected by valgrind:

==14406== Invalid write of size 1
==14406==    by 0x86ECC76: sprintf (in /usr/lib/libc-2.23.so)
==14406==    by 0x430B0A: commands_init (in /home/jviki/Projects/dpdk/dpdk-soc/build/app/test)
==14406==    by 0x42F215: main (in /home/jviki/Projects/dpdk/dpdk-soc/build/app/test)
==14406==  Address 0x9d72ff2 is 0 bytes after a block of size 1,346 alloc'd
==14406==    at 0x78C1BD0: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14406==    by 0x430AE4: commands_init (in /home/jviki/Projects/dpdk/dpdk-soc/build/app/test)
==14406==    by 0x42F215: main (in /home/jviki/Projects/dpdk/dpdk-soc/build/app/test)
==14406==

The commands buffer is exactly 1346 B long so there is an access just after
the buffer. The sprintf always writes '\0' at the end of the string. The '#'
separator adds 1 B more to each string. This is correct until the last string
is sprinted there. The last one is 1 B longer then expected, i.e.:

 strlen(t->command) + strlen("#") + ONE_FOR_ZERO

Fixes: 727909c59231 ("app/test: introduce dynamic commands list")

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
---
 app/test/commands.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/commands.c b/app/test/commands.c
index 9cb9606..e0af8e4 100644
--- a/app/test/commands.c
+++ b/app/test/commands.c
@@ -439,7 +439,7 @@ int commands_init(void)
 		commands_len += strlen(t->command) + 1;
 	}
 
-	commands = malloc(commands_len);
+	commands = malloc(commands_len + 1);
 	if (!commands)
 		return -1;
 
-- 
2.8.0

Re: [dpdk-dev] [PATCH] app/test: fix +/-1 error in allocation

[David Marchand]

On Tue, May 3, 2016 at 9:15 PM, Jan Viktorin <viktorin@rehivetech.com> wrote:
> A bug has been detected by valgrind:
>
> ==14406== Invalid write of size 1
> ==14406==    by 0x86ECC76: sprintf (in /usr/lib/libc-2.23.so)
> ==14406==    by 0x430B0A: commands_init (in /home/jviki/Projects/dpdk/dpdk-soc/build/app/test)
> ==14406==    by 0x42F215: main (in /home/jviki/Projects/dpdk/dpdk-soc/build/app/test)
> ==14406==  Address 0x9d72ff2 is 0 bytes after a block of size 1,346 alloc'd
> ==14406==    at 0x78C1BD0: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==14406==    by 0x430AE4: commands_init (in /home/jviki/Projects/dpdk/dpdk-soc/build/app/test)
> ==14406==    by 0x42F215: main (in /home/jviki/Projects/dpdk/dpdk-soc/build/app/test)
> ==14406==
>
> The commands buffer is exactly 1346 B long so there is an access just after
> the buffer. The sprintf always writes '\0' at the end of the string. The '#'
> separator adds 1 B more to each string. This is correct until the last string
> is sprinted there. The last one is 1 B longer then expected, i.e.:
>
>  strlen(t->command) + strlen("#") + ONE_FOR_ZERO
>
> Fixes: 727909c59231 ("app/test: introduce dynamic commands list")
>
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>

Good catch.
Acked-by: David Marchand <david.marchand@6wind.com>

-- 
David Marchand

Re: [dpdk-dev] [PATCH] app/test: fix +/-1 error in allocation

[Thomas Monjalon]

2016-05-04 10:15, David Marchand:
> On Tue, May 3, 2016 at 9:15 PM, Jan Viktorin <viktorin@rehivetech.com> wrote:
> > A bug has been detected by valgrind:
[...]
> >  strlen(t->command) + strlen("#") + ONE_FOR_ZERO
> >
> > Fixes: 727909c59231 ("app/test: introduce dynamic commands list")
> >
> > Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> 
> Good catch.
> Acked-by: David Marchand <david.marchand@6wind.com>

Applied, thanks