| Message ID | 1445117041-16814-1-git-send-email-blp@nicira.com |
|---|---|
| State | Accepted |
| Headers | show |
On 17/10/2015 14:24, "Ben Pfaff" <blp@nicira.com> wrote: >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@gmail.com> >Reported-at: >https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_piperm >ail_discuss_2015-2DOctober_019135.html&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJA >XVeAw-YihVMNtXt-uEs&r=SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=OrWGS3 >tQ3PzCvpaif3pjUIuOxAY8nQCTY_8Q_Qs_Wj8&s=TeEaw1_lnbTmgin2fd7Sjw0dEf__XAROJU >z7KORhS1s&e= >Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Daniele Di Proietto <diproiettod@vmware.com>
On Tue, Oct 20, 2015 at 10:55:37PM +0000, Daniele Di Proietto wrote: > > On 17/10/2015 14:24, "Ben Pfaff" <blp@nicira.com> wrote: > > >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@gmail.com> > >Reported-at: > >https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_piperm > >ail_discuss_2015-2DOctober_019135.html&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJA > >XVeAw-YihVMNtXt-uEs&r=SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=OrWGS3 > >tQ3PzCvpaif3pjUIuOxAY8nQCTY_8Q_Qs_Wj8&s=TeEaw1_lnbTmgin2fd7Sjw0dEf__XAROJU > >z7KORhS1s&e= > >Signed-off-by: Ben Pfaff <blp@nicira.com> > > Acked-by: Daniele Di Proietto <diproiettod@vmware.com> Thanks Daniele, I applied this to master and branch-2.4.
diff --git a/AUTHORS b/AUTHORS index f4e1ca9..41264ec 100644 --- a/AUTHORS +++ b/AUTHORS @@ -328,6 +328,7 @@ Mike Kruze mkruze@nicira.com Min Chen ustcer.tonychan@gmail.com Mikael Doverhag mdoverhag@nicira.com Mrinmoy Das mrdas@ixiacom.com +Murali R muralirdev@gmail.com Nagi Reddy Jonnala njonnala@Brocade.com Niels van Adrichem N.L.M.vanAdrichem@tudelft.nl Niklas Andersson nandersson@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); + } } }
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@gmail.com> Reported-at: http://openvswitch.org/pipermail/discuss/2015-October/019135.html Signed-off-by: Ben Pfaff <blp@nicira.com> --- AUTHORS | 1 + lib/dpctl.c | 27 ++++++++++++--------------- 2 files changed, 13 insertions(+), 15 deletions(-)