diff mbox

[ovs-dev,1/2] lib/dpdk: fix double free on exit

Message ID 1481300548-4522-2-git-send-email-aconole@redhat.com
State Accepted
Delegated to: Daniele Di Proietto
Headers show

Commit Message

Aaron Conole Dec. 9, 2016, 4:22 p.m. UTC
The DPDK EAL library intents that all argc/argv arguments passed on the
command line will be in the form:

    progname dpdk arguments program arguments

This means the argv array will look something like:
   argv[0] = progname
   argv[1..x] = dpdk arguments
   argv[x..y] = program arguments

When the eal initialization routine completes, it will modify the argv array
to set argv[ret] = progname, such that the arguments can then be passed to
something like getopts for further processing.

When the dpdk arguments rework was initially added, the assignment mentioned
above was not considered.  This means two errors were introduced:
1. Leak of the element at argv[ret]
2. Double-free of the element at argv[0]

Reported-by: Ilya Maximets <i.maximets@samsung.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325442.html
Fixes: bab694097133 ("netdev-dpdk: Convert initialization from cmdline to db")
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 lib/dpdk.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Daniele Di Proietto Dec. 12, 2016, 8:31 p.m. UTC | #1
2016-12-09 8:22 GMT-08:00 Aaron Conole <aconole@redhat.com>:
> The DPDK EAL library intents that all argc/argv arguments passed on the
> command line will be in the form:
>
>     progname dpdk arguments program arguments
>
> This means the argv array will look something like:
>    argv[0] = progname
>    argv[1..x] = dpdk arguments
>    argv[x..y] = program arguments
>
> When the eal initialization routine completes, it will modify the argv array
> to set argv[ret] = progname, such that the arguments can then be passed to
> something like getopts for further processing.
>
> When the dpdk arguments rework was initially added, the assignment mentioned
> above was not considered.  This means two errors were introduced:
> 1. Leak of the element at argv[ret]
> 2. Double-free of the element at argv[0]
>
> Reported-by: Ilya Maximets <i.maximets@samsung.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325442.html
> Fixes: bab694097133 ("netdev-dpdk: Convert initialization from cmdline to db")
> Signed-off-by: Aaron Conole <aconole@redhat.com>

Thanks, I pushed this to master and branch-2.6



> ---
>  lib/dpdk.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index 49a589a..28f3485 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -250,7 +250,7 @@ get_dpdk_args(const struct smap *ovs_other_config, char ***argv,
>      return i + extra_argc;
>  }
>
> -static char **dpdk_argv;
> +static char **dpdk_argv, **dpdk_argv_release;
>  static int dpdk_argc;
>
>  static void
> @@ -258,9 +258,10 @@ deferred_argv_release(void)
>  {
>      int result;
>      for (result = 0; result < dpdk_argc; ++result) {
> -        free(dpdk_argv[result]);
> +        free(dpdk_argv_release[result]);
>      }
>
> +    free(dpdk_argv_release);
>      free(dpdk_argv);
>  }
>
> @@ -366,6 +367,11 @@ dpdk_init__(const struct smap *ovs_other_config)
>          ds_destroy(&eal_args);
>      }
>
> +    dpdk_argv_release = grow_argv(&dpdk_argv_release, 0, argc);
> +    for (argc_tmp = 0; argc_tmp < argc; ++argc_tmp) {
> +        dpdk_argv_release[argc_tmp] = argv[argc_tmp];
> +    }
> +
>      /* Make sure things are initialized ... */
>      result = rte_eal_init(argc, argv);
>      if (result < 0) {
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox

Patch

diff --git a/lib/dpdk.c b/lib/dpdk.c
index 49a589a..28f3485 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -250,7 +250,7 @@  get_dpdk_args(const struct smap *ovs_other_config, char ***argv,
     return i + extra_argc;
 }
 
-static char **dpdk_argv;
+static char **dpdk_argv, **dpdk_argv_release;
 static int dpdk_argc;
 
 static void
@@ -258,9 +258,10 @@  deferred_argv_release(void)
 {
     int result;
     for (result = 0; result < dpdk_argc; ++result) {
-        free(dpdk_argv[result]);
+        free(dpdk_argv_release[result]);
     }
 
+    free(dpdk_argv_release);
     free(dpdk_argv);
 }
 
@@ -366,6 +367,11 @@  dpdk_init__(const struct smap *ovs_other_config)
         ds_destroy(&eal_args);
     }
 
+    dpdk_argv_release = grow_argv(&dpdk_argv_release, 0, argc);
+    for (argc_tmp = 0; argc_tmp < argc; ++argc_tmp) {
+        dpdk_argv_release[argc_tmp] = argv[argc_tmp];
+    }
+
     /* Make sure things are initialized ... */
     result = rte_eal_init(argc, argv);
     if (result < 0) {