[patch] null after free

John (J5) Palmieri johnp at redhat.com
Mon Oct 4 20:28:46 UTC 2004

On Mon, 2004-10-04 at 10:27 -0400, John (J5) Palmieri wrote:
> On Sat, 2004-10-02 at 18:59 -0400, Havoc Pennington wrote:
> > On Fri, 2004-10-01 at 16:42 -0400, John (J5) Palmieri wrote:
> > > An unfortunate breakage in glibc's getgrouplist function led me to this
> > > bug in a rarely used error code path.  Basically on errors in the
> > > fill_user_info function there is a double free of the info structure.  I
> > > have a quick fix which nulls out the structure after the free.  This
> > > ensures that double frees don't crash dbus.  This however is a bandaid
> > > and we need to look at why the double free is happening and to determine
> > > which of the frees is the correct one. 
> > > 
> > > The glibc guys took care of the glib bug so no worries there.
> > > 
> > 
> > OK, we should really figure out the real bug instead of applying the
> > bandaid - valgrind should spell it out for you, I would think, if you
> > can reproduce.
> > 
> > Havoc
> I know where it is happening.  I just wasn't sure if it was happening in
> other places and didn't have time to fix it properly.  I thought I would
> get the bandaid out first and report the error while I was tracking down
> the glibc bug.  What I will do, now that I have time, is put asserts in
> the user_info_free function and fix the double frees properly where I
> can find them.  

What is actually happening is that on an error in the fill_user_info
function in dbus-sysdeps.c we are jumping to the failed section which
frees the DBusUserInfo structure:

  _dbus_user_info_free (info);
  return FALSE;

However this returns FALSE and an error to the caller which then calls
free_user_info (which calls _dbus_user_info_free somewhere down the
line).  Here is an example from dbus-userdb.c

if (!_dbus_user_info_fill_uid (info, uid, error))
    free_user_info (info);
    return NULL;

This is just one example but it happens a couple of times.  The question
is where is it best to free the structure?  The DBusUserInfo field is
passed into fill_user_info so I feel this is not the place it should be
freed.  However if it is not freed there we must make sure the error is
handled everywhere else.  Any opinions?

John (J5) Palmieri
Associate Software Engineer
Desktop Group
Red Hat, Inc.
Blog: http://martianrock.com

More information about the dbus mailing list