[PATCH] qemu: Fix crash in virQEMUCapsProbeQMPTPM

[Peter Krempa]

On Mon, Jul 12, 2021 at 16:34:47 +0200, Erik Skultety wrote:
> On Mon, Jul 12, 2021 at 04:25:59PM +0200, Peter Krempa wrote:
> > If the queried QMP command doesn't exist qemuMonitorGetTPMModels returns
> > 0 but sets the string list to NULL which isn't accepted by
> > g_strv_contains.
> > 
> > Fixes: a5bc5f0ecf8
> > Reported-by: Olaf Hering <olaf at aepfle.de>
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >  src/qemu/qemu_capabilities.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index d1cd8f11ac..5b359258a1 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -3168,6 +3168,9 @@ virQEMUCapsProbeQMPTPM(virQEMUCaps *qemuCaps,
> >      if (qemuMonitorGetTPMModels(mon, &models) < 0)
> >          return -1;
> > 
> > +    if (!models)
> > +        return 0;
> > +
> >      for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMModelsToCaps); i++) {
> >          const char *needle = virDomainTPMModelTypeToString(virQEMUCapsTPMModelsToCaps[i].type);
> >          if (g_strv_contains((const char **)models, needle))
> > @@ -3177,10 +3180,12 @@ virQEMUCapsProbeQMPTPM(virQEMUCaps *qemuCaps,
> >      if (qemuMonitorGetTPMTypes(mon, &types) < 0)
> >          return -1;
> > 
> > -    for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMTypesToCaps); i++) {
> > -        const char *needle = virDomainTPMBackendTypeToString(virQEMUCapsTPMTypesToCaps[i].type);
> > -        if (g_strv_contains((const char **)types, needle))
> > -            virQEMUCapsSet(qemuCaps, virQEMUCapsTPMTypesToCaps[i].caps);
> > +    if (types) {
> > +        for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMTypesToCaps); i++) {
> > +            const char *needle = virDomainTPMBackendTypeToString(virQEMUCapsTPMTypesToCaps[i].type);
> > +            if (g_strv_contains((const char **)types, needle))
> > +                virQEMUCapsSet(qemuCaps, virQEMUCapsTPMTypesToCaps[i].caps);
> > +        }
> 
> Why not:
> 
>     if (!types)
>         return 0;
> 
> to remain consistent with the essentially same fix in the first hunk? :)

Hmm, yeah in this instance we can and should do that. It's unlikely that
new code will be added after and if it will it probably will depend on
newer features.