[ovs-dev] [PATCH] dpctl: Fix jump through wild pointer in "dpctl/help".

Ben Pfaff blp at nicira.com
Sat Oct 17 21:24:01 UTC 2015


dpctl_unixctl_handler() didn't fully initialize the dpctl_params structure
it passed to the handler, which meant that dpctl_help() could see a nonnull
(indeterminate) 'usage' pointer and jump through it, causes a crash.
This commit fixes the crash by fully initializing the structure.

The dpctl/help command wasn't going to do anything useful anyway, so this
commit also stops registering it.

Reported-by: Murali R <muralirdev at gmail.com>
Reported-at: http://openvswitch.org/pipermail/discuss/2015-October/019135.html
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 AUTHORS     |  1 +
 lib/dpctl.c | 27 ++++++++++++---------------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index f4e1ca9..41264ec 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -328,6 +328,7 @@ Mike Kruze              mkruze at nicira.com
 Min Chen                ustcer.tonychan at gmail.com
 Mikael Doverhag         mdoverhag at nicira.com
 Mrinmoy Das             mrdas at ixiacom.com
+Murali R                muralirdev at gmail.com
 Nagi Reddy Jonnala      njonnala at Brocade.com
 Niels van Adrichem      N.L.M.vanAdrichem at tudelft.nl
 Niklas Andersson        nandersson at nicira.com
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 48bf6bc..438bfd3 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1583,15 +1583,13 @@ dpctl_unixctl_handler(struct unixctl_conn *conn, int argc, const char *argv[],
                       void *aux)
 {
     struct ds ds = DS_EMPTY_INITIALIZER;
-    struct dpctl_params dpctl_p;
     bool error = false;
 
-    dpctl_command_handler *handler = (dpctl_command_handler *) aux;
-
-    dpctl_p.print_statistics = false;
-    dpctl_p.zero_statistics = false;
-    dpctl_p.may_create = false;
-    dpctl_p.verbosity = 0;
+    struct dpctl_params dpctl_p = {
+        .is_appctl = true,
+        .output = dpctl_unixctl_print,
+        .aux = &ds,
+    };
 
     /* Parse options (like getopt). Unfortunately it does
      * not seem a good idea to call getopt_long() here, since it uses global
@@ -1644,10 +1642,7 @@ dpctl_unixctl_handler(struct unixctl_conn *conn, int argc, const char *argv[],
     }
 
     if (!error) {
-        dpctl_p.is_appctl = true;
-        dpctl_p.output = dpctl_unixctl_print;
-        dpctl_p.aux = &ds;
-
+        dpctl_command_handler *handler = (dpctl_command_handler *) aux;
         error = handler(argc, argv, &dpctl_p) != 0;
     }
 
@@ -1666,9 +1661,11 @@ dpctl_unixctl_register(void)
     const struct dpctl_command *p;
 
     for (p = all_commands; p->name != NULL; p++) {
-        char *cmd_name = xasprintf("dpctl/%s", p->name);
-        unixctl_command_register(cmd_name, "", p->min_args, p->max_args,
-                                 dpctl_unixctl_handler, p->handler);
-        free(cmd_name);
+        if (strcmp(p->name, "help")) {
+            char *cmd_name = xasprintf("dpctl/%s", p->name);
+            unixctl_command_register(cmd_name, "", p->min_args, p->max_args,
+                                     dpctl_unixctl_handler, p->handler);
+            free(cmd_name);
+        }
     }
 }
-- 
2.1.3




More information about the dev mailing list