[libvirt] [PATCH] daemon: Fix crash in virTypedParameterArrayClear
Jiri Denemark
jdenemar at redhat.com
Mon Jul 30 14:29:30 UTC 2012
On Mon, Jul 30, 2012 at 21:08:22 +0800, Osier Yang wrote:
> On 2012年07月30日 19:55, Jiri Denemark wrote:
> > Daemon uses the following pattern when dispatching APIs with typed
> > parameters:
> >
> > VIR_ALLOC_N(params, nparams);
> > virDomain*(dom, params,&nparams, flags);
> > virTypedParameterArrayClear(params, nparams);
> >
> > In case nparams was originally set to 0, virDomain* API would fill it
> > with the number of typed parameters it can provide and we would use this
> > number (rather than zero) to clear params. Because VIR_ALLOC* returns
> > non-NULL pointer even if size is 0, the code would end up walking
> > through random memory. If we were lucky enough and the memory contained
> > 7 (VIR_TYPED_PARAM_STRING) at the right place, we would try to free a
> > random pointer and crash.
> >
> > Let's make sure params stays NULL when nparams is 0.
> > ---
> > daemon/remote.c | 16 ++++++++--------
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/daemon/remote.c b/daemon/remote.c
> > index 80626a2..d25717c 100644
> > --- a/daemon/remote.c
> > +++ b/daemon/remote.c
> > @@ -989,7 +989,7 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUS
> > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
> > goto cleanup;
> > }
> > - if (VIR_ALLOC_N(params, nparams)< 0)
> > + if (nparams&& VIR_ALLOC_N(params, nparams)< 0)
> > goto no_memory;
> >
> > if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
> > @@ -1098,7 +1098,7 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server ATTRIBUTE
> > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
> > goto cleanup;
> > }
> > - if (VIR_ALLOC_N(params, nparams)< 0)
> > + if (nparams&& VIR_ALLOC_N(params, nparams)< 0)
>
> Isn't nparams for these 2 APIs are guarantee positive in the APIs?
>
> virCheckPositiveArgGoto(*nparams, error);
Yes but this function is an entry point from RPC and the daemon should not
rely on client-side checking.
Jirka
More information about the libvir-list
mailing list