Patchwork P2P When UUID gets changed and more pbc session entries are created, remove logic should check for loop entry address

login
register
mail settings
Submitter Neeraj Garg
Date May 24, 2012, 8:37 a.m.
Message ID <F764D07D6734DF489C733939E1A6F85A062FE4@SJEXCHMB12.corp.ad.broadcom.com>
Download mbox | patch
Permalink /patch/161076/
State Under Review
Headers show

Comments

Neeraj Garg - May 24, 2012, 8:37 a.m.
When UUID gets changed between a received probe request (for wps) and the M1 message, we might endup creating 2 entries in pbc sessions list (with same device address and different UUID). But when we try to remove the entries, we need to compare that pbc sessions list entry->address. This will ensure that both entries gets removed.
Please let me know if the below patch is OK or I am missing something in my understanding.

From 1785eabf9c28bcd3012cc18bc510711f7f8febb9 Mon Sep 17 00:00:00 2001
From: Neeraj Garg <neerajkg@broadcom.com>
Date: Thu, 24 May 2012 14:01:40 +0530
Subject: [PATCH] Patch:P2P When UUID gets changed and more pbc session entries are created, remove logic should check for loop entry address
Signed-off-by: Neeraj Garg <neerajkg@broadcom.com>

---
 src/wps/wps_registrar.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 src/wps/wps_registrar.c
Jouni Malinen - June 9, 2012, 2:15 p.m.
On Thu, May 24, 2012 at 08:37:27AM +0000, Neeraj Kumar Garg wrote:
> When UUID gets changed between a received probe request (for wps) and the M1 message, we might endup creating 2 entries in pbc sessions list (with same device address and different UUID). But when we try to remove the entries, we need to compare that pbc sessions list entry->address. This will ensure that both entries gets removed.
> Please let me know if the below patch is OK or I am missing something in my understanding.

I'm not sure I understand what this is trying to accomplish.. First of
all, the UUID should never change for a device, so I'd assume this is
referring to the case where we need to work around some already deployed
P2P implementations that use multiple UUIDs.

Would you be able to provide a debug log showing a case where this
change is needed?

> diff --git a/src/wps/wps_registrar.c b/src/wps/wps_registrar.c
> @@ -313,8 +313,8 @@ static void wps_registrar_remove_pbc_session(struct wps_registrar *reg,
>  	pbc = reg->pbc_sessions;
>  	while (pbc) {
>  		if (os_memcmp(pbc->uuid_e, uuid_e, WPS_UUID_LEN) == 0 ||
> -		    (p2p_dev_addr && !is_zero_ether_addr(reg->p2p_dev_addr) &&
> -		     os_memcmp(reg->p2p_dev_addr, p2p_dev_addr, ETH_ALEN) ==
> +		    (p2p_dev_addr && !is_zero_ether_addr(pbc->addr) &&
> +		     os_memcmp(pbc->addr, p2p_dev_addr, ETH_ALEN) ==

This does not look correct. pbc->addr is the MAC address that the
enrollee device used, i.e., it would be P2P Interface Address in case of
a P2P Client. That cannot be compared to P2P Device Address.

Patch

diff --git a/src/wps/wps_registrar.c b/src/wps/wps_registrar.c
old mode 100644
new mode 100755
index e6ec04c..678375b
--- a/src/wps/wps_registrar.c
+++ b/src/wps/wps_registrar.c
@@ -313,8 +313,8 @@  static void wps_registrar_remove_pbc_session(struct wps_registrar *reg,
 	pbc = reg->pbc_sessions;
 	while (pbc) {
 		if (os_memcmp(pbc->uuid_e, uuid_e, WPS_UUID_LEN) == 0 ||
-		    (p2p_dev_addr && !is_zero_ether_addr(reg->p2p_dev_addr) &&
-		     os_memcmp(reg->p2p_dev_addr, p2p_dev_addr, ETH_ALEN) ==
+		    (p2p_dev_addr && !is_zero_ether_addr(pbc->addr) &&
+		     os_memcmp(pbc->addr, p2p_dev_addr, ETH_ALEN) ==
 		     0)) {
 			if (prev)
 				prev->next = pbc->next;