[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] cpuCompare: Fix crash on unexpected CPU XML



> > When comparing a CPU without <model> element, such as
> > 
> >     <cpu>
> >         <topology sockets='1' cores='1' threads='1'/>
> >     </cpu>
> > 
> > libvirt would happily crash without warning.
> > ---
> >  src/cpu/cpu.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
> > index 279eee7..def6974 100644
> > --- a/src/cpu/cpu.c
> > +++ b/src/cpu/cpu.c
> > @@ -90,6 +90,12 @@ cpuCompareXML(virCPUDefPtr host,
> >      if (cpu == NULL)
> >          goto cleanup;
> >  
> > +    if (!cpu->model) {
> > +        virCPUReportError(VIR_ERR_OPERATION_INVALID,
> > +                "%s", _("no CPU model specified"));
> > +        goto cleanup;
> > +    }
> > +
> >      ret = cpuCompare(host, cpu);
> >  
> >  cleanup:
> 
>   Argh, ACK
> 
> though that could have been checked one level down in cpuCompare() which
> could also make some checking about host and host->arch before
> dereferencing.
> Either it's an internal API where we trust the args or it's not
> but it's important to fix the crash either there or in cpuCompare

Well, both of them are internal.

cpuCompare is used with virCPUDef structure created internally in libvirt so
we trust the caller that it knows what it's doing. However, we should probably
add some check to cpu* functions used internally to avoid troubles when we
change something in the future.

But that is a separate issue with lower priority, so I pushed this fix.

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]