Patchwork P2P: Disconnect P2P group on supplicant deinit

login
register
mail settings
Submitter nirav shah
Date March 22, 2012, 4:17 p.m.
Message ID <1332433056-9074-1-git-send-email-nirav.j2.shah@intel.com>
Download mbox | patch
Permalink /patch/148280/
State Superseded
Headers show

Comments

nirav shah - March 22, 2012, 4:17 p.m.
When a supplicant is deinited and shutting, adding code to disconnect
from p2p_group. Found root causing the memory leak on variable
dbus_groupobj_path on exiting supplicant.

Signed-hostap: Nirav Shah <nirav.j2.shah@intel.com>
Signed-hostap: Angie Chinchilla <angie.v.chinchilla@intel.com>

---
 wpa_supplicant/wpa_supplicant.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
Jouni Malinen - March 31, 2012, 6:29 p.m.
On Thu, Mar 22, 2012 at 09:17:36AM -0700, nirav shah wrote:
> When a supplicant is deinited and shutting, adding code to disconnect
> from p2p_group. Found root causing the memory leak on variable
> dbus_groupobj_path on exiting supplicant.

> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> @@ -2570,6 +2570,18 @@ next_driver:
>  static void wpa_supplicant_deinit_iface(struct wpa_supplicant *wpa_s,
>  					int notify, int terminate)
>  {
> +
> +#ifdef CONFIG_P2P
> +	if (wpa_s && wpa_s->p2p_group_interface != NOT_P2P_GROUP_INTERFACE) {
> +		/* This is a p2p interface */
> +		wpa_printf(MSG_DEBUG, "P2P: Disconnecting the P2P group %s",
> +			 wpa_s->dbus_groupobj_path);

This breaks non-D-Bus builds..

> +		/* Disconnect from the p2p group if any */
> +		wpas_p2p_disconnect(wpa_s);

And this could potentially free the wpa_s structure which looks
suspicious when the following statement here dereferences the pointer:

>  	if (wpa_s->drv_priv) {


This will need to be done differently. What parts of
wpas_p2p_group_delete() are needed to avoid the memory leak? Just the
call to wpas_notify_p2p_group_removed() to get
wpas_dbus_unregister_p2p_group() called to free
wpa_s->dbus_groupobj_path?
nirav shah - April 4, 2012, 12:55 a.m.
Hey Jouni, 

You are right it does break the non dbus build because of wpa_printf message in the patch which references dbus_groupobj_path. Also I have realized that wpa_supplicant_deinit_iface() may not be a good place to do p2p specific operation. To answer your question about the leak, although wpas_notify_p2p_group_removed is enough to solve the immediate problem, I am calling wpas_p2p_group_delete to avoid similar cleanup issues in the future. And I do need ssid portion of code in wpas_p2p_group_delete as well. 

I have created an updated patch (I will send that to the mailing list in a few mins).Here I am calling the same function inside wpas_p2p_deinit_global(). If you know of a better way to do this let me know and I can redo the patch. 

Thanks,
Nirav.

-----Original Message-----
From: hostap-bounces@lists.shmoo.com [mailto:hostap-bounces@lists.shmoo.com] On Behalf Of Jouni Malinen
Sent: Saturday, March 31, 2012 11:30 AM
To: hostap@lists.shmoo.com
Subject: Re: [PATCH] P2P: Disconnect P2P group on supplicant deinit

On Thu, Mar 22, 2012 at 09:17:36AM -0700, nirav shah wrote:
> When a supplicant is deinited and shutting, adding code to disconnect
> from p2p_group. Found root causing the memory leak on variable
> dbus_groupobj_path on exiting supplicant.

> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> @@ -2570,6 +2570,18 @@ next_driver:
>  static void wpa_supplicant_deinit_iface(struct wpa_supplicant *wpa_s,
>  					int notify, int terminate)
>  {
> +
> +#ifdef CONFIG_P2P
> +	if (wpa_s && wpa_s->p2p_group_interface != NOT_P2P_GROUP_INTERFACE) {
> +		/* This is a p2p interface */
> +		wpa_printf(MSG_DEBUG, "P2P: Disconnecting the P2P group %s",
> +			 wpa_s->dbus_groupobj_path);

This breaks non-D-Bus builds..

> +		/* Disconnect from the p2p group if any */
> +		wpas_p2p_disconnect(wpa_s);

And this could potentially free the wpa_s structure which looks
suspicious when the following statement here dereferences the pointer:

>  	if (wpa_s->drv_priv) {


This will need to be done differently. What parts of
wpas_p2p_group_delete() are needed to avoid the memory leak? Just the
call to wpas_notify_p2p_group_removed() to get
wpas_dbus_unregister_p2p_group() called to free
wpa_s->dbus_groupobj_path?

Patch

diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 4c365dd..2b82a46 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -2570,6 +2570,18 @@  next_driver:
 static void wpa_supplicant_deinit_iface(struct wpa_supplicant *wpa_s,
 					int notify, int terminate)
 {
+
+#ifdef CONFIG_P2P
+	if (wpa_s && wpa_s->p2p_group_interface != NOT_P2P_GROUP_INTERFACE) {
+		/* This is a p2p interface */
+		wpa_printf(MSG_DEBUG, "P2P: Disconnecting the P2P group %s",
+			 wpa_s->dbus_groupobj_path);
+		/* Disconnect from the p2p group if any */
+		wpas_p2p_disconnect(wpa_s);
+	}
+#endif /* CONFIG_P2P */
+
+
 	if (wpa_s->drv_priv) {
 		wpa_supplicant_deauthenticate(wpa_s,
 					      WLAN_REASON_DEAUTH_LEAVING);