libxl memory leaks when Xen is compiled with XSM

libxl memory leaks when Xen is compiled with XSM

[Andrew Cooper]

Correctly CC'ing xen-devel as well this time

~Andrew

On 19/12/14 14:31, Andrew Cooper wrote:
> Hello,
>
> Coverity has identified a large number of memory leaks in libxl, because
> of the use of libxl_domain_info() without an init/dispose for the
> libxl_dominfo object.
>
> If XSM is active, this leaks the ssid_label string, and in almost all
> cases, from the successful completion of the library function in question.
>
> Most of the issues can be fixed with the correct use of
> init()/dispose(), but libxl_wait_for_memory_target() is a little more
> interesting.
>
> Strictly speaking, I think I would need to fix it with a
> dispose();init() pair immediately before the call to
> libxl_domain_info().  However, this feels like overkill.  As only the
> memory information is needed, would it be appropriate to downgrade to an
> xc_domain_getinfo() instead, forgoing the memory allocation and
> extraneous hypercalls?
>
> ~Andrew
>

Re: libxl memory leaks when Xen is compiled with XSM

[Wei Liu]

On Fri, Dec 19, 2014 at 02:51:33PM +0000, Andrew Cooper wrote:
> Correctly CC'ing xen-devel as well this time
> 
> ~Andrew
> 
> On 19/12/14 14:31, Andrew Cooper wrote:
> > Hello,
> >
> > Coverity has identified a large number of memory leaks in libxl, because
> > of the use of libxl_domain_info() without an init/dispose for the
> > libxl_dominfo object.
> >
> > If XSM is active, this leaks the ssid_label string, and in almost all
> > cases, from the successful completion of the library function in question.
> >
> > Most of the issues can be fixed with the correct use of
> > init()/dispose(), but libxl_wait_for_memory_target() is a little more
> > interesting.
> >
> > Strictly speaking, I think I would need to fix it with a
> > dispose();init() pair immediately before the call to
> > libxl_domain_info().  However, this feels like overkill.  As only the
> > memory information is needed, would it be appropriate to downgrade to an
> > xc_domain_getinfo() instead, forgoing the memory allocation and
> > extraneous hypercalls?
> >

As long as caller visible behaviour is not changed I think it's OK to do
so.

Wei.

> > ~Andrew
> >

Re: libxl memory leaks when Xen is compiled with XSM [and 1 more messages]

[Ian Jackson]

(Resending my reply to the list too.)

Andrew Cooper writes ("libxl memory leaks when Xen is compiled with XSM"):
> Coverity has identified a large number of memory leaks in libxl, because
> of the use of libxl_domain_info() without an init/dispose for the
> libxl_dominfo object.

Hrm, how annoying.  (Why is this not on -devel BTW?)

> If XSM is active, this leaks the ssid_label string, and in almost all
> cases, from the successful completion of the library function in question.

You seem to be right.

> Most of the issues can be fixed with the correct use of
> init()/dispose(), but libxl_wait_for_memory_target() is a little more
> interesting.

Bear in mind the libxl memory handling convention.  The init should be
at the top of the function, and the dispose at the bottom.

> Strictly speaking, I think I would need to fix it with a
> dispose();init() pair immediately before the call to
> libxl_domain_info().  However, this feels like overkill.  As only the
> memory information is needed, would it be appropriate to downgrade to an
> xc_domain_getinfo() instead, forgoing the memory allocation and
> extraneous hypercalls?

I think that would be OK in general although for the 4.5 release
(for which this ought to be a candidate) it would be better to simply
fix the memory management.

Note that I think we intend to make libxl_*_dispose idempotent and
when we do the memory management becomes a bit less annoying.

Ian.