diff mbox

Patch for review

Message ID 01.3C.09100.92A37035@epcpsbgx2.samsung.com
State Changes Requested
Headers show

Commit Message

Rahul Jain Feb. 21, 2014, 11:36 a.m. UTC 
hi,

Please review the attached patch.


From c5311fb4154d81907d23cd9998af09b205f18525 Mon Sep 17 00:00:00 2001
From: Rahul Jain <rahul.jain@samsung.com>

Date: Fri, 21 Feb 2014 17:03:18 +0530
Subject: [PATCH] Fix for checking go intent value before going ahead for
 processing of Go Neg Request Frame


Signed-off-by: Rahul Jain <rahul.jain@samsung.com>

---
 src/p2p/p2p_go_neg.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

-- 
1.7.9.5

Thanks
-Rahul Jain

Comments

Jouni Malinen Feb. 21, 2014, 1:17 p.m. UTC | #1 
On Fri, Feb 21, 2014 at 11:36:09AM +0000, Rahul Jain wrote:
> Please review the attached patch.

> Subject: [PATCH] Fix for checking go intent value before going ahead for
>  processing of Go Neg Request Frame

Please note that it would be useful to use this subject line from the
commit as the subject of the message as well.

> diff --git a/src/p2p/p2p_go_neg.c b/src/p2p/p2p_go_neg.c
> @@ -545,8 +545,14 @@ void p2p_process_go_neg_req(struct p2p_data *p2p, const u8 *sa,
>  #endif /* CONFIG_P2P_STRICT */
>  	}
>  
> -	if (msg.go_intent)
> +	if (msg.go_intent) {
>  		tie_breaker = *msg.go_intent & 0x01;
> +		if ((*msg.go_intent >> 1) > P2P_MAX_GO_INTENT) {
> +			p2p_dbg(p2p, "Invalid GO Intent value (%u) received",
> +				*msg.go_intent >> 1);
> +			goto fail;
> +		}

While the change itself would be a correct validation step, isn't this
already done in the function when GO Negotiation would be allowed to
start? As such, this addition here would have very little change in the
behavior (status code would change for couple of error paths). Is there
a specific case where this is needed? If so, I think I would be fine
with the change, but I would combine it with removing the other check
that would become practically dead code if this is added into the
beginning of the function.
diff mbox

Patch

diff --git a/src/p2p/p2p_go_neg.c b/src/p2p/p2p_go_neg.c
index 2e40db1..c6b47be 100644
--- a/src/p2p/p2p_go_neg.c
+++ b/src/p2p/p2p_go_neg.c
@@ -545,8 +545,14 @@  void p2p_process_go_neg_req(struct p2p_data *p2p, const u8 *sa,
 #endif /* CONFIG_P2P_STRICT */
 	}
 
-	if (msg.go_intent)
+	if (msg.go_intent) {
 		tie_breaker = *msg.go_intent & 0x01;
+		if ((*msg.go_intent >> 1) > P2P_MAX_GO_INTENT) {
+			p2p_dbg(p2p, "Invalid GO Intent value (%u) received",
+				*msg.go_intent >> 1);
+			goto fail;
+		}
+	}
 	else {
 		p2p_dbg(p2p, "Mandatory GO Intent attribute missing from GO Negotiation Request");
 #ifdef CONFIG_P2P_STRICT