[PATCH] virsh-snapshot: Avoid passing NULL to qsort() in virshSnapshotListCollect()

Michal Privoznik posted 1 patch 8 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/85e9f4fa5375fb8acc42e46d173b20ec77c6f08a.1694077362.git.mprivozn@redhat.com
tools/virsh-snapshot.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH] virsh-snapshot: Avoid passing NULL to qsort() in virshSnapshotListCollect()
Posted by Michal Privoznik 8 months, 2 weeks ago
If a domain has no snapshots and 'virsh snapshot-list' is called,
this gets all the way down to virshSnapshotListCollect() which
then collects all snapshots (none), and passes them to qsort()
which doesn't like being called with NULL:

  extern void qsort (void *__base, size_t __nmemb, size_t __size,
                     __compar_fn_t __compar) __nonnull ((1, 4));

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/533
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tools/virsh-snapshot.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index d7889a38e4..ecb935b2b4 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -1310,9 +1310,11 @@ virshSnapshotListCollect(vshControl *ctl, virDomainPtr dom,
             }
         }
     }
-    if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL))
+    if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL) &&
+        snaplist->snaps) {
         qsort(snaplist->snaps, snaplist->nsnaps, sizeof(*snaplist->snaps),
               virshSnapSorter);
+    }
     snaplist->nsnaps -= deleted;
 
     ret = g_steal_pointer(&snaplist);
-- 
2.41.0
Re: [PATCH] virsh-snapshot: Avoid passing NULL to qsort() in virshSnapshotListCollect()
Posted by Peter Krempa 8 months, 2 weeks ago
On Thu, Sep 07, 2023 at 11:02:42 +0200, Michal Privoznik wrote:
> If a domain has no snapshots and 'virsh snapshot-list' is called,
> this gets all the way down to virshSnapshotListCollect() which
> then collects all snapshots (none), and passes them to qsort()
> which doesn't like being called with NULL:
> 
>   extern void qsort (void *__base, size_t __nmemb, size_t __size,
>                      __compar_fn_t __compar) __nonnull ((1, 4));
> 
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/533
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tools/virsh-snapshot.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index d7889a38e4..ecb935b2b4 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -1310,9 +1310,11 @@ virshSnapshotListCollect(vshControl *ctl, virDomainPtr dom,
>              }
>          }
>      }
> -    if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL))
> +    if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL) &&
> +        snaplist->snaps) {

In most other places in virsh we do the check as 'snaplist->snaps && snaplist->nsnaps'.

>          qsort(snaplist->snaps, snaplist->nsnaps, sizeof(*snaplist->snaps),
>                virshSnapSorter);
> +    }
>      snaplist->nsnaps -= deleted;
>  
>      ret = g_steal_pointer(&snaplist);

Regardless of the above comment:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>