observed memory leak in wpa_supplicant

[Dan Williams]

On Tue, 2016-02-09 at 17:33 -0800, Naveen Singh wrote:
> On Tue, Feb 9, 2016 at 5:28 PM, Naveen Singh <naveensingh0977 at gmail.c
> om> wrote:
> > On Wed, Feb 3, 2016 at 10:13 PM, Naveen Singh <naveensingh0977 at gmai
> > l.com> wrote:
> > > On Wed, Feb 3, 2016 at 4:20 AM, Jouni Malinen <j at w1.fi> wrote:
> > > > On Wed, Jan 27, 2016 at 10:06:32PM -0800, Naveen Singh wrote:
> > > > > We have been running some stress test of sending a deauth
> > > > > with reason
> > > > > code 6 and 7 from an AP to an associated client and
> > > > > monitoring the
> > > > > memory usage of wpa_supplicant. In our setup wpa_supplicant
> > > > > talks to
> > > > > connman over dbus. Running this stress test reveals memory
> > > > > leak in wpa
> > > > > supplicant (memory usage of wpa supplicant keeps on growing).
> > > > 
> > > > Have you checked that this is actually a real memory leak? For
> > > > example,
> > > > run wpa_supplicant under valgrind and see whether such memory
> > > > gets freed
> > > > when the process exits.
> > > 
> > > Yes I ran valgrind. Valgrind reported this memory as still
> > > reachable
> > > as it is lying in list. It is ideally not a leak because all the
> > > pointers point to same network,
> > > > 
> > > > > For connection after getting disconnected connman does
> > > > > "AddNetwork"
> > > > > DBUS call to the previously connected SSID. This ends up
> > > > > causing
> > > > > memory to be allocated for struct wpa_ssid (os_zalloc
> > > > > called). At this
> > > > > time there is already a structure allocated for same SSID as
> > > > > a result
> > > > > of previous connection. Every connection ends up creating a
> > > > > memory
> > > > > which is never freed.
> > > > 
> > > > Are you sure it is not freed when wpa_supplicant process
> > > > terminates? All
> > > > the added network blocks should be freed at that point in
> > > > time..
> > > When supplicant terminates or is restarted it reclaims all the
> > > memory.
> > > > 
> > > > > This surely causes memory usage of wpa_supplicant to go up.
> > > > > Should not
> > > > > we be allocating memory only if that profile (for that SSID
> > > > > does not
> > > > > exist?)
> > > > 
> > > > Memory is allocated only if a new profile is added. There can
> > > > be
> > > > multiple profiles for a single SSID.
> > > I guess that is intent but surely that is not happening. We are
> > > allocating memory for same profile (same SSID and security)
> > > > 
> > > > > I am talking about this function:
> > > > > struct wpa_ssid * wpa_config_add_network(struct wpa_config
> > > > > *config)
> > > > 
> > > > This needs to add a new network profile and allocate memory for
> > > > it.
> > > > 
> > > > If you do not want more memory to be used, it is responsibility
> > > > of the
> > > > program managing wpa_supplicant (either through the control
> > > > interface or
> > > > through D-Bus) to re-use existing network profiles or remove
> > > > profiles
> > > > they have added.
> > > I guess that is the solution. I am working on a patch to remove
> > > the
> > > network once it is not needed. It is slightly tricky to do so
> > > because
> > > we just do not want to remove the network when wpa_supplicant is
> > > trying to connect to same network.
> > > > 
> > > > --
> > > > Jouni Malinen                                            PGP id
> > > > EFC895FA
> 
> I have a patch out to connman for this. In this particular case we
> are
> relying on a separate process to do the right stuff. What if that
> process dies or crashes before it could do the remove network. That
> memory is lost for ever till we restart supplicant. Isn't a good idea
> to ensure that we allocate memory only if that network does not
> exist.

Or, that process can certainly read back the network blocks from the
supplicant, and if one matches the configuration that it would have
sent, it can just use the existing network block instead of creating a
new one.  Also, removing the interface from the supplicant and re-
adding it will release the memory, which the process could do when it
starts up too.  Or the process could just remove all network blocks on
startup.  There's a number of ways to do it, some with more code and
some with less, and some that interact better with other processes and
some that don't.  But in the end, it's highly likely that only one
process will be controlling a single wifi device at a time, so that
process can probably just remove all network blocks when it starts up.

Dan