ligang/vpp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

VPP-635: CLI Memory leak with invalid parameter

Browse Source 
In the CLI parsing, below is a common pattern:
  /* Get a line of input. */
  if (!unformat_user (input, unformat_line_input, line_input))
    return 0;

  while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
    {
      if (unformat (line_input, "x"))
	x = 1;
      :
      else
	return clib_error_return (0, "unknown input `%U'",
				  format_unformat_error, line_input);
    }
  unformat_free (line_input);

The 'else' returns if an unknown string is encountered. There a memory
leak because the 'unformat_free(line_input)' is not called. There is a
large number of instances of this pattern.

Replaced the previous pattern with:
  /* Get a line of input. */
  if (!unformat_user (input, unformat_line_input, line_input))
    return 0;

  while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
    {
      if (unformat (line_input, "x"))
	x = 1;
      :
      else
        {
	  error = clib_error_return (0, "unknown input `%U'",
				     format_unformat_error, line_input);
	  goto done:
        }
    }

  /* ...Remaining code... */

done:
  unformat_free (line_input);
  return error;
}

In multiple files, 'unformat_free (line_input);' was never called, so
there was a memory leak whether an invalid string was entered or not.

Also, there were multiple instance where:
	  error = clib_error_return (0, "unknown input `%U'",
				     format_unformat_error, line_input);
used 'input' as the last parameter instead of 'line_input'. The result
is that output did not contain the substring in error, instead just an
empty string. Fixed all of those as well.

There are a lot of file, and very mind numbing work, so tried to keep
it to a pattern to avoid mistakes.

Change-Id: I8902f0c32a47dd7fb3bb3471a89818571702f1d2
Signed-off-by: Billy McFall <bmcfall@redhat.com>
Signed-off-by: Dave Barach <dave@barachs.net>
This commit is contained in:
Billy McFall
2017-02-15 11:39:12 -05:00
committed by Dave Barach
parent 2291a36008
commit a9a20e7f69
37 changed files with 1490 additions and 579 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
build-root/emacs-lisp/tunnel-c-skel.el
View File

@ -288,6 +288,7 @@ static clib_error_t *
vlib_cli_command_t * cmd)
{
unformat_input_t _line_input, * line_input = &_line_input;
clib_error_t *error = 0;
ip4_address_t src, dst;
u8 is_add = 1;
u8 src_set = 0;
@ -322,13 +323,19 @@ static clib_error_t *
{
encap_fib_index = fib_index_from_fib_id (tmp);
if (encap_fib_index == ~0)
return clib_error_return (0, \"nonexistent encap fib id %d\", tmp);
{
unformat_free (line_input);
return clib_error_return (0, \"nonexistent encap fib id %d\", tmp);
}
}
else if (unformat (line_input, \"decap-vrf-id %d\", &tmp))
{
decap_fib_index = fib_index_from_fib_id (tmp);
if (decap_fib_index == ~0)
return clib_error_return (0, \"nonexistent decap fib id %d\", tmp);
{
unformat_free (line_input);
return clib_error_return (0, \"nonexistent decap fib id %d\", tmp);
}
}
else if (unformat (line_input, \"decap-next %U\", unformat_decap_next,
&decap_next_index))
@ -346,8 +353,12 @@ static clib_error_t *
* in the " ENCAP_STACK " header
*/
else
return clib_error_return (0, \"parse error: '%U'\",
format_unformat_error, line_input);
{
error = clib_error_return (0, \"parse error: '%U'\",
format_unformat_error, line_input);
unformat_free (line_input);
return error;
}
}
unformat_free (line_input);