[PATCH iproute2] lnstat: fix strdup leak in -w argument parsing

[PATCH iproute2] lnstat: fix strdup leak in -w argument parsing

[Maxim Petrov]

'tmp' string is used for safe tokenizing, but it is not required after
getting all the widths in -w option. As 'tmp' string is obtained by strdup
call, the caller has to deallocate it to avoid memory leak.

Signed-off-by: Maxim Petrov <mmrmaximuzz@gmail.com>
---
 misc/lnstat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/misc/lnstat.c b/misc/lnstat.c
index 98904d45..c3293a8e 100644
--- a/misc/lnstat.c
+++ b/misc/lnstat.c
@@ -331,6 +331,7 @@ int main(int argc, char **argv)
 				for (i = 0; i < MAX_FIELDS; i++)
 					fp.params[i].print.width = len;
 			}
+			free(tmp);
 			break;
 		default:
 			usage(argv[0], 1);
-- 
2.25.1

Re: [PATCH iproute2] lnstat: fix strdup leak in -w argument parsing

[Stephen Hemminger]

On Tue, 15 Feb 2022 23:53:47 +0300
Maxim Petrov <mmrmaximuzz@gmail.com> wrote:

> 'tmp' string is used for safe tokenizing, but it is not required after
> getting all the widths in -w option. As 'tmp' string is obtained by strdup
> call, the caller has to deallocate it to avoid memory leak.
> 
> Signed-off-by: Maxim Petrov <mmrmaximuzz@gmail.com>

Would strdupa() be cleaner/simpler. I have no strong preference.

Re: [PATCH iproute2] lnstat: fix strdup leak in -w argument parsing

[Maxim Petrov]

On 2022-02-15 22:49 UTC, Stephen Hemminger wrote:
> Would strdupa() be cleaner/simpler.

strdupa will not free the allocated memory until the caller returns. In our
case the caller is 'main', so despite valgrind will be happy, the memory
will be wasted anyway.

However, I guess that the option is mostly used just like '-w 20', so the
_wasted anyway_ strdupa memory is only about a few bytes in these cases,
and from this perspective I also have no strong preference. But I don't
know how this option is used by others.

Re: [PATCH iproute2] lnstat: fix strdup leak in -w argument parsing

[Stephen Hemminger]

On Wed, 16 Feb 2022 22:40:58 +0300
Maxim Petrov <mmrmaximuzz@gmail.com> wrote:

> On 2022-02-15 22:49 UTC, Stephen Hemminger wrote:
> > Would strdupa() be cleaner/simpler.  
> 
> strdupa will not free the allocated memory until the caller returns. In our
> case the caller is 'main', so despite valgrind will be happy, the memory
> will be wasted anyway.
> 
> However, I guess that the option is mostly used just like '-w 20', so the
> _wasted anyway_ strdupa memory is only about a few bytes in these cases,
> and from this perspective I also have no strong preference. But I don't
> know how this option is used by others.

There are not a lot of paths through the code in main, so not a bit worry.