diff mbox

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

Message ID F764D07D6734DF489C733939E1A6F85A062FE4@SJEXCHMB12.corp.ad.broadcom.com
State Changes Requested
Headers show

Commit Message

Neeraj Garg May 24, 2012, 8:37 a.m. UTC 
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

Comments

Jouni Malinen June 9, 2012, 2:15 p.m. UTC | #1 
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.
diff mbox

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;