Patchwork [wps] Don't allocate 'struct wps_parse_attr' on the stack due to its size

login
register
mail settings
Submitter Solomon Peachy
Date April 23, 2013, 2:41 p.m.
Message ID <1366728077-11643-1-git-send-email-pizza@shaftnet.org>
Download mbox | patch
Permalink /patch/238932/
State New
Headers show

Comments

Solomon Peachy - April 23, 2013, 2:41 p.m.
Some functions allocate up to three of these structs at a time, and the
calls are nested, badly busting the stacks of deeply embedded systems.

Allocate them from the heap instead.

Signed-off-by: Solomon Peachy <pizza@shaftnet.org>
---
 src/wps/wps.c           |  95 ++++++++++++++++++++----------
 src/wps/wps_enrollee.c  | 153 +++++++++++++++++++++++++++++++++---------------
 src/wps/wps_registrar.c | 141 +++++++++++++++++++++++++++++---------------
 3 files changed, 262 insertions(+), 127 deletions(-)

Patch

diff --git a/src/wps/wps.c b/src/wps/wps.c
index ff4b20d..73b0a10 100644
--- a/src/wps/wps.c
+++ b/src/wps/wps.c
@@ -344,23 +344,46 @@  int wps_is_addr_authorized(const struct wpabuf *msg, const u8 *addr,
 int wps_ap_priority_compar(const struct wpabuf *wps_a,
 			   const struct wpabuf *wps_b)
 {
-	struct wps_parse_attr attr_a, attr_b;
+	struct wps_parse_attr *attr_a, *attr_b;
 	int sel_a, sel_b;
+	int ret = 0;
 
-	if (wps_a == NULL || wps_parse_msg(wps_a, &attr_a) < 0)
-		return 1;
-	if (wps_b == NULL || wps_parse_msg(wps_b, &attr_b) < 0)
-		return -1;
+	attr_a = os_malloc(sizeof(struct wps_parse_attr));
+	attr_b = os_malloc(sizeof(struct wps_parse_attr));
 
-	sel_a = attr_a.selected_registrar && *attr_a.selected_registrar != 0;
-	sel_b = attr_b.selected_registrar && *attr_b.selected_registrar != 0;
+	if (!attr_a || !attr_b) {
+		ret = -99;
+		goto done;
+	}
 
-	if (sel_a && !sel_b)
-		return -1;
-	if (!sel_a && sel_b)
-		return 1;
+	if (wps_a == NULL || wps_parse_msg(wps_a, attr_a) < 0) {
+		ret = 1;
+		goto done;
+	}
+	if (wps_b == NULL || wps_parse_msg(wps_b, attr_b) < 0) {
+		ret = -1;
+		goto done;
+	}
 
-	return 0;
+	sel_a = attr_a->selected_registrar && *attr_a->selected_registrar != 0;
+	sel_b = attr_b->selected_registrar && *attr_b->selected_registrar != 0;
+
+	if (sel_a && !sel_b) {
+		ret = -1;
+		goto done;
+	}
+	if (!sel_a && sel_b) {
+		ret = 1;
+		goto done;
+	}
+
+ done:
+	if (attr_a)
+		os_free(attr_a);
+	if (attr_b)
+		os_free(attr_b);
+
+	return ret;
 }
 
 
@@ -542,18 +565,24 @@  void wps_free_pending_msgs(struct upnp_pending_message *msgs)
 
 int wps_attr_text(struct wpabuf *data, char *buf, char *end)
 {
-	struct wps_parse_attr attr;
+	struct wps_parse_attr *attr;
 	char *pos = buf;
 	int ret;
 
-	if (wps_parse_msg(data, &attr) < 0)
+	attr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!attr)
 		return -1;
 
-	if (attr.wps_state) {
-		if (*attr.wps_state == WPS_STATE_NOT_CONFIGURED)
+	if (wps_parse_msg(data, attr) < 0) {
+		buf++; /* So we return -1 */
+		goto done;
+	}
+
+	if (attr->wps_state) {
+		if (*attr->wps_state == WPS_STATE_NOT_CONFIGURED)
 			ret = os_snprintf(pos, end - pos,
 					  "wps_state=unconfigured\n");
-		else if (*attr.wps_state == WPS_STATE_CONFIGURED)
+		else if (*attr->wps_state == WPS_STATE_CONFIGURED)
 			ret = os_snprintf(pos, end - pos,
 					  "wps_state=configured\n");
 		else
@@ -563,7 +592,7 @@  int wps_attr_text(struct wpabuf *data, char *buf, char *end)
 		pos += ret;
 	}
 
-	if (attr.ap_setup_locked && *attr.ap_setup_locked) {
+	if (attr->ap_setup_locked && *attr->ap_setup_locked) {
 		ret = os_snprintf(pos, end - pos,
 				  "wps_ap_setup_locked=1\n");
 		if (ret < 0 || ret >= end - pos)
@@ -571,7 +600,7 @@  int wps_attr_text(struct wpabuf *data, char *buf, char *end)
 		pos += ret;
 	}
 
-	if (attr.selected_registrar && *attr.selected_registrar) {
+	if (attr->selected_registrar && *attr->selected_registrar) {
 		ret = os_snprintf(pos, end - pos,
 				  "wps_selected_registrar=1\n");
 		if (ret < 0 || ret >= end - pos)
@@ -579,30 +608,30 @@  int wps_attr_text(struct wpabuf *data, char *buf, char *end)
 		pos += ret;
 	}
 
-	if (attr.dev_password_id) {
+	if (attr->dev_password_id) {
 		ret = os_snprintf(pos, end - pos,
 				  "wps_device_password_id=%u\n",
-				  WPA_GET_BE16(attr.dev_password_id));
+				  WPA_GET_BE16(attr->dev_password_id));
 		if (ret < 0 || ret >= end - pos)
 			return pos - buf;
 		pos += ret;
 	}
 
-	if (attr.sel_reg_config_methods) {
+	if (attr->sel_reg_config_methods) {
 		ret = os_snprintf(pos, end - pos,
 				  "wps_selected_registrar_config_methods="
 				  "0x%04x\n",
-				  WPA_GET_BE16(attr.sel_reg_config_methods));
+				  WPA_GET_BE16(attr->sel_reg_config_methods));
 		if (ret < 0 || ret >= end - pos)
 			return pos - buf;
 		pos += ret;
 	}
 
-	if (attr.primary_dev_type) {
+	if (attr->primary_dev_type) {
 		char devtype[WPS_DEV_TYPE_BUFSIZE];
 		ret = os_snprintf(pos, end - pos,
 				  "wps_primary_device_type=%s\n",
-				  wps_dev_type_bin2str(attr.primary_dev_type,
+				  wps_dev_type_bin2str(attr->primary_dev_type,
 						       devtype,
 						       sizeof(devtype)));
 		if (ret < 0 || ret >= end - pos)
@@ -610,16 +639,16 @@  int wps_attr_text(struct wpabuf *data, char *buf, char *end)
 		pos += ret;
 	}
 
-	if (attr.dev_name) {
-		char *str = os_malloc(attr.dev_name_len + 1);
+	if (attr->dev_name) {
+		char *str = os_malloc(attr->dev_name_len + 1);
 		size_t i;
 		if (str == NULL)
 			return pos - buf;
-		for (i = 0; i < attr.dev_name_len; i++) {
-			if (attr.dev_name[i] < 32)
+		for (i = 0; i < attr->dev_name_len; i++) {
+			if (attr->dev_name[i] < 32)
 				str[i] = '_';
 			else
-				str[i] = attr.dev_name[i];
+				str[i] = attr->dev_name[i];
 		}
 		str[i] = '\0';
 		ret = os_snprintf(pos, end - pos, "wps_device_name=%s\n", str);
@@ -629,14 +658,16 @@  int wps_attr_text(struct wpabuf *data, char *buf, char *end)
 		pos += ret;
 	}
 
-	if (attr.config_methods) {
+	if (attr->config_methods) {
 		ret = os_snprintf(pos, end - pos,
 				  "wps_config_methods=0x%04x\n",
-				  WPA_GET_BE16(attr.config_methods));
+				  WPA_GET_BE16(attr->config_methods));
 		if (ret < 0 || ret >= end - pos)
 			return pos - buf;
 		pos += ret;
 	}
 
+done:
+	os_free(attr);
 	return pos - buf;
 }
diff --git a/src/wps/wps_enrollee.c b/src/wps/wps_enrollee.c
index 9c0cebb..c39e47e 100644
--- a/src/wps/wps_enrollee.c
+++ b/src/wps/wps_enrollee.c
@@ -646,16 +646,24 @@  static int wps_process_r_snonce2(struct wps_data *wps, const u8 *r_snonce2)
 static int wps_process_cred_e(struct wps_data *wps, const u8 *cred,
 			      size_t cred_len, int wps2)
 {
-	struct wps_parse_attr attr;
+	struct wps_parse_attr *attr;
 	struct wpabuf msg;
 	int ret = 0;
 
 	wpa_printf(MSG_DEBUG, "WPS: Received Credential");
 	os_memset(&wps->cred, 0, sizeof(wps->cred));
 	wpabuf_set(&msg, cred, cred_len);
-	if (wps_parse_msg(&msg, &attr) < 0 ||
-	    wps_process_cred(&attr, &wps->cred))
+
+	attr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!attr)
+		return -1;
+
+	if (wps_parse_msg(&msg, attr) < 0 ||
+	    wps_process_cred(attr, &wps->cred)) {
+		os_free(attr);
 		return -1;
+	}
+	os_free(attr);
 
 	if (os_memcmp(wps->cred.mac_addr, wps->wps->dev.mac_addr, ETH_ALEN) !=
 	    0) {
@@ -979,7 +987,7 @@  static enum wps_process_res wps_process_m4(struct wps_data *wps,
 					   struct wps_parse_attr *attr)
 {
 	struct wpabuf *decrypted;
-	struct wps_parse_attr eattr;
+	struct wps_parse_attr *eattr;
 
 	wpa_printf(MSG_DEBUG, "WPS: Received M4");
 
@@ -1015,14 +1023,22 @@  static enum wps_process_res wps_process_m4(struct wps_data *wps,
 
 	wpa_printf(MSG_DEBUG, "WPS: Processing decrypted Encrypted Settings "
 		   "attribute");
-	if (wps_parse_msg(decrypted, &eattr) < 0 ||
-	    wps_process_key_wrap_auth(wps, decrypted, eattr.key_wrap_auth) ||
-	    wps_process_r_snonce1(wps, eattr.r_snonce1)) {
+
+	eattr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!eattr)
+		return WPS_FAILURE;
+
+	if (wps_parse_msg(decrypted, eattr) < 0 ||
+	    wps_process_key_wrap_auth(wps, decrypted, eattr->key_wrap_auth) ||
+	    wps_process_r_snonce1(wps, eattr->r_snonce1)) {
 		wpabuf_free(decrypted);
+		os_free(eattr);
+
 		wps->state = SEND_WSC_NACK;
 		return WPS_CONTINUE;
 	}
 	wpabuf_free(decrypted);
+	os_free(eattr);
 
 	wps->state = SEND_M5;
 	return WPS_CONTINUE;
@@ -1034,7 +1050,7 @@  static enum wps_process_res wps_process_m6(struct wps_data *wps,
 					   struct wps_parse_attr *attr)
 {
 	struct wpabuf *decrypted;
-	struct wps_parse_attr eattr;
+	struct wps_parse_attr *eattr;
 
 	wpa_printf(MSG_DEBUG, "WPS: Received M6");
 
@@ -1068,14 +1084,22 @@  static enum wps_process_res wps_process_m6(struct wps_data *wps,
 
 	wpa_printf(MSG_DEBUG, "WPS: Processing decrypted Encrypted Settings "
 		   "attribute");
-	if (wps_parse_msg(decrypted, &eattr) < 0 ||
-	    wps_process_key_wrap_auth(wps, decrypted, eattr.key_wrap_auth) ||
-	    wps_process_r_snonce2(wps, eattr.r_snonce2)) {
+
+	eattr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!eattr)
+		return WPS_FAILURE;
+
+	if (wps_parse_msg(decrypted, eattr) < 0 ||
+	    wps_process_key_wrap_auth(wps, decrypted, eattr->key_wrap_auth) ||
+	    wps_process_r_snonce2(wps, eattr->r_snonce2)) {
 		wpabuf_free(decrypted);
+		os_free(eattr);
+
 		wps->state = SEND_WSC_NACK;
 		return WPS_CONTINUE;
 	}
 	wpabuf_free(decrypted);
+	os_free(eattr);
 
 	if (wps->wps->ap)
 		wps->wps->event_cb(wps->wps->cb_ctx, WPS_EV_AP_PIN_SUCCESS,
@@ -1091,7 +1115,7 @@  static enum wps_process_res wps_process_m8(struct wps_data *wps,
 					   struct wps_parse_attr *attr)
 {
 	struct wpabuf *decrypted;
-	struct wps_parse_attr eattr;
+	struct wps_parse_attr *eattr;
 
 	wpa_printf(MSG_DEBUG, "WPS: Received M8");
 
@@ -1139,17 +1163,25 @@  static enum wps_process_res wps_process_m8(struct wps_data *wps,
 
 	wpa_printf(MSG_DEBUG, "WPS: Processing decrypted Encrypted Settings "
 		   "attribute");
-	if (wps_parse_msg(decrypted, &eattr) < 0 ||
-	    wps_process_key_wrap_auth(wps, decrypted, eattr.key_wrap_auth) ||
-	    wps_process_creds(wps, eattr.cred, eattr.cred_len,
-			      eattr.num_cred, attr->version2 != NULL) ||
-	    wps_process_ap_settings_e(wps, &eattr, decrypted,
+
+	eattr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!eattr)
+		return WPS_FAILURE;
+
+	if (wps_parse_msg(decrypted, eattr) < 0 ||
+	    wps_process_key_wrap_auth(wps, decrypted, eattr->key_wrap_auth) ||
+	    wps_process_creds(wps, eattr->cred, eattr->cred_len,
+			      eattr->num_cred, attr->version2 != NULL) ||
+	    wps_process_ap_settings_e(wps, eattr, decrypted,
 				      attr->version2 != NULL)) {
 		wpabuf_free(decrypted);
+		os_free(eattr);
+
 		wps->state = SEND_WSC_NACK;
 		return WPS_CONTINUE;
 	}
 	wpabuf_free(decrypted);
+	os_free(eattr);
 
 	wps->state = WPS_MSG_DONE;
 	return WPS_CONTINUE;
@@ -1159,65 +1191,83 @@  static enum wps_process_res wps_process_m8(struct wps_data *wps,
 static enum wps_process_res wps_process_wsc_msg(struct wps_data *wps,
 						const struct wpabuf *msg)
 {
-	struct wps_parse_attr attr;
+	struct wps_parse_attr *attr;
 	enum wps_process_res ret = WPS_CONTINUE;
 
 	wpa_printf(MSG_DEBUG, "WPS: Received WSC_MSG");
 
-	if (wps_parse_msg(msg, &attr) < 0)
+	attr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!attr)
 		return WPS_FAILURE;
 
-	if (attr.enrollee_nonce == NULL ||
-	    os_memcmp(wps->nonce_e, attr.enrollee_nonce, WPS_NONCE_LEN) != 0) {
+	if (wps_parse_msg(msg, attr) < 0) {
+		ret = WPS_FAILURE;
+		goto done;
+	}
+
+	if (attr->enrollee_nonce == NULL ||
+	    os_memcmp(wps->nonce_e, attr->enrollee_nonce, WPS_NONCE_LEN) != 0) {
 		wpa_printf(MSG_DEBUG, "WPS: Mismatch in enrollee nonce");
-		return WPS_FAILURE;
+		ret = WPS_FAILURE;
+		goto done;
 	}
 
-	if (attr.msg_type == NULL) {
+	if (attr->msg_type == NULL) {
 		wpa_printf(MSG_DEBUG, "WPS: No Message Type attribute");
 		wps->state = SEND_WSC_NACK;
-		return WPS_CONTINUE;
+		goto done;
 	}
 
-	switch (*attr.msg_type) {
+	switch (*attr->msg_type) {
 	case WPS_M2:
-		if (wps_validate_m2(msg) < 0)
-			return WPS_FAILURE;
-		ret = wps_process_m2(wps, msg, &attr);
+		if (wps_validate_m2(msg) < 0) {
+			ret = WPS_FAILURE;
+			goto done;
+		}
+		ret = wps_process_m2(wps, msg, attr);
 		break;
 	case WPS_M2D:
-		if (wps_validate_m2d(msg) < 0)
-			return WPS_FAILURE;
-		ret = wps_process_m2d(wps, &attr);
+		if (wps_validate_m2d(msg) < 0) {
+			ret = WPS_FAILURE;
+			goto done;
+		}
+		ret = wps_process_m2d(wps, attr);
 		break;
 	case WPS_M4:
-		if (wps_validate_m4(msg) < 0)
-			return WPS_FAILURE;
-		ret = wps_process_m4(wps, msg, &attr);
+		if (wps_validate_m4(msg) < 0) {
+			ret = WPS_FAILURE;
+			goto done;
+		}
+		ret = wps_process_m4(wps, msg, attr);
 		if (ret == WPS_FAILURE || wps->state == SEND_WSC_NACK)
 			wps_fail_event(wps->wps, WPS_M4, wps->config_error,
 				       wps->error_indication);
 		break;
 	case WPS_M6:
-		if (wps_validate_m6(msg) < 0)
-			return WPS_FAILURE;
-		ret = wps_process_m6(wps, msg, &attr);
+		if (wps_validate_m6(msg) < 0) {
+			ret = WPS_FAILURE;
+			goto done;
+		}
+		ret = wps_process_m6(wps, msg, attr);
 		if (ret == WPS_FAILURE || wps->state == SEND_WSC_NACK)
 			wps_fail_event(wps->wps, WPS_M6, wps->config_error,
 				       wps->error_indication);
 		break;
 	case WPS_M8:
-		if (wps_validate_m8(msg) < 0)
-			return WPS_FAILURE;
-		ret = wps_process_m8(wps, msg, &attr);
+		if (wps_validate_m8(msg) < 0) {
+			ret = WPS_FAILURE;
+			goto done;
+		}
+		ret = wps_process_m8(wps, msg, attr);
 		if (ret == WPS_FAILURE || wps->state == SEND_WSC_NACK)
 			wps_fail_event(wps->wps, WPS_M8, wps->config_error,
 				       wps->error_indication);
 		break;
 	default:
 		wpa_printf(MSG_DEBUG, "WPS: Unsupported Message Type %d",
-			   *attr.msg_type);
-		return WPS_FAILURE;
+			   *attr->msg_type);
+		ret = WPS_FAILURE;
+		break;
 	}
 
 	/*
@@ -1227,13 +1277,17 @@  static enum wps_process_res wps_process_wsc_msg(struct wps_data *wps,
 	 * following M2 to be processed correctly by using the previously sent
 	 * M1 in Authenticator derivation.
 	 */
-	if (ret == WPS_CONTINUE && *attr.msg_type != WPS_M2D) {
+	if (ret == WPS_CONTINUE && *attr->msg_type != WPS_M2D) {
 		/* Save a copy of the last message for Authenticator derivation
 		 */
 		wpabuf_free(wps->last_msg);
 		wps->last_msg = wpabuf_dup(msg);
 	}
 
+done:
+	if (attr)
+		os_free(attr);
+
 	return ret;
 }
 
@@ -1373,13 +1427,18 @@  enum wps_process_res wps_enrollee_process_msg(struct wps_data *wps,
 
 	if (op_code == WSC_UPnP) {
 		/* Determine the OpCode based on message type attribute */
-		struct wps_parse_attr attr;
-		if (wps_parse_msg(msg, &attr) == 0 && attr.msg_type) {
-			if (*attr.msg_type == WPS_WSC_ACK)
+		struct wps_parse_attr *attr;
+		attr = os_malloc(sizeof(struct wps_parse_attr));
+		if (!attr)
+			return WPS_FAILURE;
+
+		if (wps_parse_msg(msg, attr) == 0 && attr->msg_type) {
+			if (*attr->msg_type == WPS_WSC_ACK)
 				op_code = WSC_ACK;
-			else if (*attr.msg_type == WPS_WSC_NACK)
+			else if (*attr->msg_type == WPS_WSC_NACK)
 				op_code = WSC_NACK;
 		}
+		os_free(attr);
 	}
 
 	switch (op_code) {
diff --git a/src/wps/wps_registrar.c b/src/wps/wps_registrar.c
index a26b8ee..1a3c6d9 100644
--- a/src/wps/wps_registrar.c
+++ b/src/wps/wps_registrar.c
@@ -2643,7 +2643,7 @@  static enum wps_process_res wps_process_m5(struct wps_data *wps,
 					   struct wps_parse_attr *attr)
 {
 	struct wpabuf *decrypted;
-	struct wps_parse_attr eattr;
+	struct wps_parse_attr *eattr;
 
 	wpa_printf(MSG_DEBUG, "WPS: Received M5");
 
@@ -2686,14 +2686,22 @@  static enum wps_process_res wps_process_m5(struct wps_data *wps,
 
 	wpa_printf(MSG_DEBUG, "WPS: Processing decrypted Encrypted Settings "
 		   "attribute");
-	if (wps_parse_msg(decrypted, &eattr) < 0 ||
-	    wps_process_key_wrap_auth(wps, decrypted, eattr.key_wrap_auth) ||
-	    wps_process_e_snonce1(wps, eattr.e_snonce1)) {
+
+	eattr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!eattr)
+		return WPS_FAILURE;
+
+	if (wps_parse_msg(decrypted, eattr) < 0 ||
+	    wps_process_key_wrap_auth(wps, decrypted, eattr->key_wrap_auth) ||
+	    wps_process_e_snonce1(wps, eattr->e_snonce1)) {
 		wpabuf_free(decrypted);
+		os_free(eattr);
+
 		wps->state = SEND_WSC_NACK;
 		return WPS_CONTINUE;
 	}
 	wpabuf_free(decrypted);
+	os_free(eattr);
 
 	wps->state = SEND_M6;
 	return WPS_CONTINUE;
@@ -2794,7 +2802,7 @@  static enum wps_process_res wps_process_m7(struct wps_data *wps,
 					   struct wps_parse_attr *attr)
 {
 	struct wpabuf *decrypted;
-	struct wps_parse_attr eattr;
+	struct wps_parse_attr *eattr;
 
 	wpa_printf(MSG_DEBUG, "WPS: Received M7");
 
@@ -2838,16 +2846,24 @@  static enum wps_process_res wps_process_m7(struct wps_data *wps,
 
 	wpa_printf(MSG_DEBUG, "WPS: Processing decrypted Encrypted Settings "
 		   "attribute");
-	if (wps_parse_msg(decrypted, &eattr) < 0 ||
-	    wps_process_key_wrap_auth(wps, decrypted, eattr.key_wrap_auth) ||
-	    wps_process_e_snonce2(wps, eattr.e_snonce2) ||
-	    wps_process_ap_settings_r(wps, &eattr)) {
+
+	eattr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!eattr)
+		return WPS_FAILURE;
+
+	if (wps_parse_msg(decrypted, eattr) < 0 ||
+	    wps_process_key_wrap_auth(wps, decrypted, eattr->key_wrap_auth) ||
+	    wps_process_e_snonce2(wps, eattr->e_snonce2) ||
+	    wps_process_ap_settings_r(wps, eattr)) {
 		wpabuf_free(decrypted);
+		os_free(eattr);
+
 		wps->state = SEND_WSC_NACK;
 		return WPS_CONTINUE;
 	}
 
 	wpabuf_free(decrypted);
+	os_free(eattr);
 
 	wps->state = SEND_M8;
 	return WPS_CONTINUE;
@@ -2857,73 +2873,87 @@  static enum wps_process_res wps_process_m7(struct wps_data *wps,
 static enum wps_process_res wps_process_wsc_msg(struct wps_data *wps,
 						const struct wpabuf *msg)
 {
-	struct wps_parse_attr attr;
+	struct wps_parse_attr *attr;
 	enum wps_process_res ret = WPS_CONTINUE;
 
 	wpa_printf(MSG_DEBUG, "WPS: Received WSC_MSG");
 
-	if (wps_parse_msg(msg, &attr) < 0)
+	attr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!attr)
 		return WPS_FAILURE;
 
-	if (attr.msg_type == NULL) {
+	if (wps_parse_msg(msg, attr) < 0)
+		return WPS_FAILURE;
+
+	if (attr->msg_type == NULL) {
 		wpa_printf(MSG_DEBUG, "WPS: No Message Type attribute");
 		wps->state = SEND_WSC_NACK;
-		return WPS_CONTINUE;
+		goto done;
 	}
 
-	if (*attr.msg_type != WPS_M1 &&
-	    (attr.registrar_nonce == NULL ||
-	     os_memcmp(wps->nonce_r, attr.registrar_nonce,
+	if (*attr->msg_type != WPS_M1 &&
+	    (attr->registrar_nonce == NULL ||
+	     os_memcmp(wps->nonce_r, attr->registrar_nonce,
 		       WPS_NONCE_LEN) != 0)) {
 		wpa_printf(MSG_DEBUG, "WPS: Mismatch in registrar nonce");
-		return WPS_FAILURE;
+		ret = WPS_FAILURE;
+		goto done;
 	}
 
-	switch (*attr.msg_type) {
+	switch (*attr->msg_type) {
 	case WPS_M1:
-		if (wps_validate_m1(msg) < 0)
-			return WPS_FAILURE;
+		if (wps_validate_m1(msg) < 0) {
+			ret = WPS_FAILURE;
+			goto done;
+		}
 #ifdef CONFIG_WPS_UPNP
-		if (wps->wps->wps_upnp && attr.mac_addr) {
+		if (wps->wps->wps_upnp && attr->mac_addr) {
 			/* Remove old pending messages when starting new run */
 			wps_free_pending_msgs(wps->wps->upnp_msgs);
 			wps->wps->upnp_msgs = NULL;
 
 			upnp_wps_device_send_wlan_event(
-				wps->wps->wps_upnp, attr.mac_addr,
+				wps->wps->wps_upnp, attr->mac_addr,
 				UPNP_WPS_WLANEVENT_TYPE_EAP, msg);
 		}
 #endif /* CONFIG_WPS_UPNP */
-		ret = wps_process_m1(wps, &attr);
+		ret = wps_process_m1(wps, attr);
 		break;
 	case WPS_M3:
-		if (wps_validate_m3(msg) < 0)
-			return WPS_FAILURE;
-		ret = wps_process_m3(wps, msg, &attr);
+		if (wps_validate_m3(msg) < 0) {
+			ret = WPS_FAILURE;
+			goto done;
+		}
+		ret = wps_process_m3(wps, msg, attr);
 		if (ret == WPS_FAILURE || wps->state == SEND_WSC_NACK)
 			wps_fail_event(wps->wps, WPS_M3, wps->config_error,
 				       wps->error_indication);
 		break;
 	case WPS_M5:
-		if (wps_validate_m5(msg) < 0)
-			return WPS_FAILURE;
-		ret = wps_process_m5(wps, msg, &attr);
+		if (wps_validate_m5(msg) < 0) {
+			ret = WPS_FAILURE;
+			goto done;
+		}
+		ret = wps_process_m5(wps, msg, attr);
 		if (ret == WPS_FAILURE || wps->state == SEND_WSC_NACK)
 			wps_fail_event(wps->wps, WPS_M5, wps->config_error,
 				       wps->error_indication);
 		break;
 	case WPS_M7:
-		if (wps_validate_m7(msg) < 0)
-			return WPS_FAILURE;
-		ret = wps_process_m7(wps, msg, &attr);
+		if (wps_validate_m7(msg) < 0) {
+			ret = WPS_FAILURE;
+			goto done;
+		}
+		ret = wps_process_m7(wps, msg, attr);
 		if (ret == WPS_FAILURE || wps->state == SEND_WSC_NACK)
 			wps_fail_event(wps->wps, WPS_M7, wps->config_error,
 				       wps->error_indication);
 		break;
 	default:
 		wpa_printf(MSG_DEBUG, "WPS: Unsupported Message Type %d",
-			   *attr.msg_type);
-		return WPS_FAILURE;
+			   *attr->msg_type);
+		ret = WPS_FAILURE;
+		goto done;
 	}
 
 	if (ret == WPS_CONTINUE) {
@@ -2933,6 +2963,9 @@  static enum wps_process_res wps_process_wsc_msg(struct wps_data *wps,
 		wps->last_msg = wpabuf_dup(msg);
 	}
 
+done:
+	if (attr)
+		os_free(attr);
 	return ret;
 }
 
@@ -3089,7 +3122,7 @@  static enum wps_process_res wps_process_wsc_nack(struct wps_data *wps,
 static enum wps_process_res wps_process_wsc_done(struct wps_data *wps,
 						 const struct wpabuf *msg)
 {
-	struct wps_parse_attr attr;
+	struct wps_parse_attr *attr;
 
 	wpa_printf(MSG_DEBUG, "WPS: Received WSC_Done");
 
@@ -3100,18 +3133,22 @@  static enum wps_process_res wps_process_wsc_done(struct wps_data *wps,
 		return WPS_FAILURE;
 	}
 
-	if (wps_parse_msg(msg, &attr) < 0)
+	attr = os_malloc(sizeof(struct wps_parse_attr));
+	if (!attr)
 		return WPS_FAILURE;
 
-	if (attr.msg_type == NULL) {
+	if (wps_parse_msg(msg, attr) < 0)
+		goto fail;
+
+	if (attr->msg_type == NULL) {
 		wpa_printf(MSG_DEBUG, "WPS: No Message Type attribute");
-		return WPS_FAILURE;
+		goto fail;
 	}
 
-	if (*attr.msg_type != WPS_WSC_DONE) {
+	if (*attr->msg_type != WPS_WSC_DONE) {
 		wpa_printf(MSG_DEBUG, "WPS: Invalid Message Type %d",
-			   *attr.msg_type);
-		return WPS_FAILURE;
+			   *attr->msg_type);
+		goto fail;
 	}
 
 #ifdef CONFIG_WPS_UPNP
@@ -3120,21 +3157,21 @@  static enum wps_process_res wps_process_wsc_done(struct wps_data *wps,
 			   "Registrar completed successfully");
 		wps_device_store(wps->wps->registrar, &wps->peer_dev,
 				 wps->uuid_e);
-		return WPS_DONE;
+		goto done;
 	}
 #endif /* CONFIG_WPS_UPNP */
 
-	if (attr.registrar_nonce == NULL ||
-	    os_memcmp(wps->nonce_r, attr.registrar_nonce, WPS_NONCE_LEN) != 0)
+	if (attr->registrar_nonce == NULL ||
+	    os_memcmp(wps->nonce_r, attr->registrar_nonce, WPS_NONCE_LEN) != 0)
 	{
 		wpa_printf(MSG_DEBUG, "WPS: Mismatch in registrar nonce");
-		return WPS_FAILURE;
+		goto fail;
 	}
 
-	if (attr.enrollee_nonce == NULL ||
-	    os_memcmp(wps->nonce_e, attr.enrollee_nonce, WPS_NONCE_LEN) != 0) {
+	if (attr->enrollee_nonce == NULL ||
+	    os_memcmp(wps->nonce_e, attr->enrollee_nonce, WPS_NONCE_LEN) != 0) {
 		wpa_printf(MSG_DEBUG, "WPS: Mismatch in enrollee nonce");
-		return WPS_FAILURE;
+		goto fail;
 	}
 
 	wpa_printf(MSG_DEBUG, "WPS: Negotiation completed successfully");
@@ -3199,7 +3236,15 @@  static enum wps_process_res wps_process_wsc_done(struct wps_data *wps,
 
 	wps_success_event(wps->wps);
 
+#ifdef CONFIG_WPS_UPNP
+done:
+#endif
+	os_free(attr);
 	return WPS_DONE;
+fail:
+
+	os_free(attr);
+	return WPS_FAILURE;
 }