[libvirt] [PATCH] virsh: fixing segfault by pool autocompleter function.

The commands which requires a pool to perform any action for a volume is throwing a segfault when you pass the volume name before a pool name or without the argument '--pool'. An example that works: virsh # vol-list loops-pool Name Path ------------------------------------------------------------------- loop0 /mnt/loop0 virsh # vol-info --pool loops-pool lo<TAB> An example that does not work: virsh # vol-list loops-pool Name Path ------------------------------------------------------------------- loop0 /mnt/loop0 virsh # vol-info lo<TAB> Segmentation Fault The example 'vol-info' can be executed as 'vol-info loop0 --pool loops-pool'. So, this commit fixes this problem when the arguments are inverted and avoids the segfault. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tools/virsh-pool.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 56b6cfc73..24a4d1ee7 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -143,6 +143,9 @@ virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) return NULL; + if (!n) + return NULL; + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", cmd->def->name, optname, n); -- 2.14.1

On 02/28/2018 02:17 AM, Julio Faracco wrote:
The commands which requires a pool to perform any action for a volume is throwing a segfault when you pass the volume name before a pool name or without the argument '--pool'.
An example that works: virsh # vol-list loops-pool Name Path ------------------------------------------------------------------- loop0 /mnt/loop0
virsh # vol-info --pool loops-pool lo<TAB>
An example that does not work: virsh # vol-list loops-pool Name Path ------------------------------------------------------------------- loop0 /mnt/loop0
virsh # vol-info lo<TAB> Segmentation Fault
The example 'vol-info' can be executed as 'vol-info loop0 --pool loops-pool'. So, this commit fixes this problem when the arguments are inverted and avoids the segfault.
Ah, snap. Yes, this crashes.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tools/virsh-pool.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 56b6cfc73..24a4d1ee7 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -143,6 +143,9 @@ virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) return NULL;
+ if (!n) + return NULL;
This works, but I think we should make it explicit that this can happen only from completeres. I mean, if cmd->skipChecks is set. Otherwise the control doesn't even get here - the vshCommandCheckOpts() prints out error and prevents any cmd*() call. Having said that, I'm changing this condition to: if (cmd->skipChecks && !n) return NULL; But this got me looking at other completers and we have similar issue in virshDomainInterfaceCompleter. For instance: # domif-getling --interface <TAB> causes instant crash. Again, result of a26ff63ae4030. I have a patch ready and will send it ASAP. Anyway, ACKed your patch and pushed. Michal
participants (2)
-
Julio Faracco
-
Michal Privoznik