From a9a20e7f69f4a91a4d5267ab5ce14125bdc7d6c6 Mon Sep 17 00:00:00 2001
From: Billy McFall <bmcfall@redhat.com>
Date: Wed, 15 Feb 2017 11:39:12 -0500
Subject: VPP-635: CLI Memory leak with invalid parameter

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>
---
 build-root/emacs-lisp/tunnel-c-skel.el | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

(limited to 'build-root')

diff --git a/build-root/emacs-lisp/tunnel-c-skel.el b/build-root/emacs-lisp/tunnel-c-skel.el
index aa260e53333..a1b1757dab7 100644
--- a/build-root/emacs-lisp/tunnel-c-skel.el
+++ b/build-root/emacs-lisp/tunnel-c-skel.el
@@ -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);
-- 
cgit 1.2.3-korg