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

Julio Faracco posted 1 patch 6 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180228011754.25081-1-jcfaracco@gmail.com
Test syntax-check passed
tools/virsh-pool.c | 3 +++
1 file changed, 3 insertions(+)
[libvirt] [PATCH] virsh: fixing segfault by pool autocompleter function.
Posted by Julio Faracco 6 years ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fixing segfault by pool autocompleter function.
Posted by Michal Privoznik 6 years ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list