[systemd-devel] [PATCH 2/2] detect-virt: dmi: fix mem leak

[Andrew Jones]

On Wed, Nov 04, 2015 at 08:26:00AM -0600, Andrew Jones wrote:
> On Wed, Nov 04, 2015 at 11:46:22AM +0100, Lennart Poettering wrote:
> > On Tue, 03.11.15 15:04, Andrew Jones (drjones at redhat.com) wrote:
> > 
> > > The variable 's' is still in scope until we exit the function. We
> > > can't call read_one_line_file on it multiple times without calling
> > > free in between.
> > > ---
> > >  src/basic/virt.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/basic/virt.c b/src/basic/virt.c
> > > index 1e10fc755f201..f88bd04bd72ed 100644
> > > --- a/src/basic/virt.c
> > > +++ b/src/basic/virt.c
> > > @@ -176,9 +176,10 @@ static int detect_vm_dmi(void) {
> > >  
> > >                  r = read_one_line_file(dmi_vendors[i], &s);
> > >                  if (r < 0) {
> > > -                        if (r == -ENOENT)
> > > +                        if (r == -ENOENT) {
> > > +                                free(s);
> > >                                  continue;
> > 
> > Note that we use the _cleanup_free_ macro on "s", which uses gcc's
> > "cleanup" attribute to automatically free the variable when it goes
> > out of scope. An explicit free() is hence unnecessary. In fact, it
> > would result in a double-free.
> > 
> > We use the "cleanup" logic pretty much everywhere in systemd, in order
> > to keep the error paths manageable.
> 
> As I wrote in the commit message 's' is still in scope until we exit
> the function, detect_vm_dmi(). Calling read_one_line_file multiple times
> will reassign 's' with newly allocated buffers each time, losing
> references to the previously allocated buffers. _cleanup_free_ will only
> ensure the last buffer is freed (which is why I didn't put the free() at
> the function exit, but rather above the continue.)
> 
> Thanks,
> drew

Crap. Never mind. I just looked again and see that 's' is local to the
loop, not the function... Sorry for the noise.

drew

> 
> > 
> > Lennart
> > 
> > -- 
> > Lennart Poettering, Red Hat