[libvirt] [PATCH] Fix crash in virsh after bogus command.

If you ran virsh in interactive mode and ran a command that virsh could not parse, it would then SEGV on subsequent commands. The problem is that we are freeing the vshCmd structure in the syntaxError label at the end of vshCommandParse, but forgetting to set ctl->cmd to NULL. This means that on the next command, we would try to free the same structure again, leading to badness. Make sure to set ctl->cmd to NULL after freeing it. Signed-off-by: Chris Lalancette <clalance@redhat.com> --- tools/virsh.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c6e3f2a..eeaddbc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8643,8 +8643,10 @@ vshCommandParse(vshControl *ctl, char *cmdstr) return TRUE; syntaxError: - if (ctl->cmd) + if (ctl->cmd) { vshCommandFree(ctl->cmd); + ctl->cmd = NULL; + } if (first) vshCommandOptFree(first); VIR_FREE(tkdata); -- 1.6.6.1

On 03/11/2010 06:00 PM, Chris Lalancette wrote:
If you ran virsh in interactive mode and ran a command that virsh could not parse, it would then SEGV on subsequent commands. The problem is that we are freeing the vshCmd structure in the syntaxError label at the end of vshCommandParse, but forgetting to set ctl->cmd to NULL. This means that on the next command, we would try to free the same structure again, leading to badness. Make sure to set ctl->cmd to NULL after freeing it.
Signed-off-by: Chris Lalancette<clalance@redhat.com> --- tools/virsh.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index c6e3f2a..eeaddbc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8643,8 +8643,10 @@ vshCommandParse(vshControl *ctl, char *cmdstr) return TRUE;
syntaxError: - if (ctl->cmd) + if (ctl->cmd) { vshCommandFree(ctl->cmd); + ctl->cmd = NULL; + } if (first) vshCommandOptFree(first); VIR_FREE(tkdata);
ACK. Tricky! This only reveals itself if you put multiple commands on a single line (separated by ";"), the first command is valid, and one of the subsequent commands on the line is bogus. (Yes, I verified presence of the crash before applying, and absence afterwards).

On Thu, Mar 11, 2010 at 06:00:54PM -0500, Chris Lalancette wrote:
If you ran virsh in interactive mode and ran a command that virsh could not parse, it would then SEGV on subsequent commands. The problem is that we are freeing the vshCmd structure in the syntaxError label at the end of vshCommandParse, but forgetting to set ctl->cmd to NULL. This means that on the next command, we would try to free the same structure again, leading to badness. Make sure to set ctl->cmd to NULL after freeing it.
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- tools/virsh.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index c6e3f2a..eeaddbc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8643,8 +8643,10 @@ vshCommandParse(vshControl *ctl, char *cmdstr) return TRUE;
syntaxError: - if (ctl->cmd) + if (ctl->cmd) { vshCommandFree(ctl->cmd); + ctl->cmd = NULL; + } if (first) vshCommandOptFree(first); VIR_FREE(tkdata);
ACK, serious one, pushed ! Thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Chris Lalancette
-
Daniel Veillard
-
Laine Stump