diff mbox

p2p: Fix DBus crash and return additional information for P2P group properties

Message ID 1328316362-5956-1-git-send-email-angie.v.chinchilla@intel.com
State Accepted
Commit aa89df56996c0da499f1cdb132893c1863b6b2fb
Headers show

Commit Message

Chinchilla, Angie V Feb. 4, 2012, 12:46 a.m. UTC 
From: Todd Previte <toddx.a.previte@intel.com>

When using DBus to get group properties, a segmentation fault is generated on P2P
clients due to a NULL pointer for the ap_iface struct. The current implementation
only returns vendor extensions when called on a P2P group owner.

The code now checks the P2P role which allows for role-specific information to be
provided. This also fixes the crash issue by only looking for the correct structures
based on the current P2P role.

Signed-hostap: Todd Previte <toddx.a.previte@intel.com>
Signed-hostap: Angie Chinchilla <angie.v.chinchilla@intel.com>
---
 wpa_supplicant/dbus/dbus_new_handlers_p2p.c |   80 ++++++++++++++++++++++-----
 1 files changed, 66 insertions(+), 14 deletions(-)

Comments

Chinchilla, Angie V Feb. 4, 2012, 1:05 a.m. UTC | #1 
> -----Original Message-----
> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
> bounces@lists.shmoo.com] On Behalf Of Angie Chinchilla
> Sent: Friday, February 03, 2012 4:46 PM
> To: hostap@lists.shmoo.com
> Cc: Previte, ToddX A
> Subject: [PATCH] p2p: Fix DBus crash and return additional information
> for P2P group properties
> 
> From: Todd Previte <toddx.a.previte@intel.com>
> 
> When using DBus to get group properties, a segmentation fault is
> generated on P2P
> clients due to a NULL pointer for the ap_iface struct. The current
> implementation
> only returns vendor extensions when called on a P2P group owner.
> 
> The code now checks the P2P role which allows for role-specific
> information to be
> provided. This also fixes the crash issue by only looking for the
> correct structures
> based on the current P2P role.
> 
> Signed-hostap: Todd Previte <toddx.a.previte@intel.com>
> Signed-hostap: Angie Chinchilla <angie.v.chinchilla@intel.com>
> ---
>  wpa_supplicant/dbus/dbus_new_handlers_p2p.c |   80
> ++++++++++++++++++++++-----
>  1 files changed, 66 insertions(+), 14 deletions(-)

This patch doesn't currently follow the P2P DBus interface changes 
proposed by Flávio. If we do decide to change the interface we'll 
write a second follow up patch that brings these changes in line 
with the new scheme.

This fixes a supplicant crash, so I would like to get some version of
it into hostap-1.0.

Also, I forgot the following: (oops!) :)

Intended-for: hostap-1

Thanks,
Angie
Jouni Malinen Feb. 4, 2012, 11:12 a.m. UTC | #2 
On Fri, Feb 03, 2012 at 04:46:01PM -0800, Angie Chinchilla wrote:
> From: Todd Previte <toddx.a.previte@intel.com>
> When using DBus to get group properties, a segmentation fault is generated on P2P
> clients due to a NULL pointer for the ap_iface struct. The current implementation
> only returns vendor extensions when called on a P2P group owner.
> 
> The code now checks the P2P role which allows for role-specific information to be
> provided. This also fixes the crash issue by only looking for the correct structures
> based on the current P2P role.

Thanks! Applied. Please note that there were number of pointers that
could still be left to NULL in some corner cases and then get
dereferences. I don't know whether these would really happen in
practice, but anyway, static code analyzers would complain and it is
safer to validate the cases, so I added some more checks to the
function.
diff mbox

Patch

diff --git a/wpa_supplicant/dbus/dbus_new_handlers_p2p.c b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
index 0f6914c..d621fb2 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
+++ b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
@@ -1664,14 +1664,38 @@  dbus_bool_t wpas_dbus_getter_p2p_group_properties(DBusMessageIter *iter,
 {
 	struct wpa_supplicant *wpa_s = user_data;
 	DBusMessageIter variant_iter, dict_iter;
-	struct hostapd_data *hapd = wpa_s->ap_iface->bss[0];
+	struct hostapd_data *hapd = NULL;
 	const struct wpabuf *vendor_ext[MAX_WPS_VENDOR_EXTENSIONS];
 	int num_vendor_ext = 0;
 	int i;
+	u8 role = wpas_get_p2p_role(wpa_s);
+	u16 op_freq = 0;
+	u8 *p_bssid = NULL;
+	char *role_name = NULL;
 
-	if (!hapd) {
-		dbus_set_error_const(error, DBUS_ERROR_FAILED,
-				     "internal error");
+	/* Check current role and adjust information accordingly */
+	switch (role) {
+	case WPAS_P2P_ROLE_CLIENT:
+		/* go_params is only valid for a client */
+		if (wpa_s->go_params) {
+			op_freq = wpa_s->go_params->freq;
+			p_bssid = wpa_s->current_ssid->bssid;
+			role_name = "client";
+		} else
+			return FALSE;
+		break;
+	case WPAS_P2P_ROLE_GO:
+		/* ap_iface is only valid for a GO */
+		if (wpa_s->ap_iface) {
+			hapd = wpa_s->ap_iface->bss[0];
+			p_bssid = hapd->own_addr;
+			op_freq = wpa_s->ap_iface->freq;
+			role_name = "GO";
+		} else
+			return FALSE;
+		break;
+	default:
+		/* Error condition; this should NEVER occur */
 		return FALSE;
 	}
 
@@ -1679,19 +1703,47 @@  dbus_bool_t wpas_dbus_getter_p2p_group_properties(DBusMessageIter *iter,
 					      "a{sv}", &variant_iter) ||
 	    !wpa_dbus_dict_open_write(&variant_iter, &dict_iter))
 		goto err_no_mem;
+	/* Provide the SSID */
+	if (!wpa_dbus_dict_append_byte_array(&dict_iter, "SSID",
+					(const char *)wpa_s->current_ssid->ssid,
+					wpa_s->current_ssid->ssid_len))
+		goto err_no_mem;
+	/* Provide the BSSID */
+	if (!wpa_dbus_dict_append_byte_array(&dict_iter, "BSSID",
+					(const char *)p_bssid, ETH_ALEN))
+		goto err_no_mem;
+	/* Provide the role within the group */
+	if (!wpa_dbus_dict_append_string(&dict_iter, "Role", role_name))
+		goto err_no_mem;
+	/* Provide the operational frequency */
+	if (!wpa_dbus_dict_append_uint16(&dict_iter, "Frequency", op_freq))
+		goto err_no_mem;
 
-	/* Parse WPS Vendor Extensions sent in Beacon/Probe Response */
-	for (i = 0; i < MAX_WPS_VENDOR_EXTENSIONS; i++) {
-		if (hapd->conf->wps_vendor_ext[i] == NULL)
-			continue;
-		vendor_ext[num_vendor_ext++] = hapd->conf->wps_vendor_ext[i];
+	/* Additional information for group owners */
+	if (role == WPAS_P2P_ROLE_GO) {
+		/* Provide the passphrase */
+		if (!wpa_dbus_dict_append_string(&dict_iter, "Passphrase",
+					wpa_s->current_ssid->passphrase))
+			goto err_no_mem;
+		/* Parse WPS Vendor Extensions sent in Beacon/Probe Response */
+		for (i = 0; i < MAX_WPS_VENDOR_EXTENSIONS; i++) {
+			if (hapd->conf->wps_vendor_ext[i] == NULL)
+				continue;
+			vendor_ext[num_vendor_ext++] =
+				hapd->conf->wps_vendor_ext[i];
+		}
+		if (!wpa_dbus_dict_append_wpabuf_array(&dict_iter,
+					"WPSVendorExtensions",
+					vendor_ext, num_vendor_ext))
+			goto err_no_mem;
+	} else {
+		/* If not a GO, provide the PSK */
+		if (!wpa_dbus_dict_append_byte_array(&dict_iter, "PSK",
+					(const char *)wpa_s->current_ssid->psk,
+					32))
+			goto err_no_mem;
 	}
 
-	if (!wpa_dbus_dict_append_wpabuf_array(&dict_iter,
-					       "WPSVendorExtensions",
-					       vendor_ext, num_vendor_ext))
-		goto err_no_mem;
-
 	if (!wpa_dbus_dict_close_write(&variant_iter, &dict_iter) ||
 	    !dbus_message_iter_close_container(iter, &variant_iter))
 		goto err_no_mem;