[patch 1/4] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[patch 1/4] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Dan Carpenter]

If "offset" is negative then we can get past this check:
	if (offset > CONTROL_BUFFER_SIZE)
Or if we pick a very high "req_ie_len" then we can get around the check:
	if (offset + req_ie_len > CONTROL_BUFFER_SIZE)

I made "resp_ie_len" and "req_ie_len" unsigned.  I don't know if it was
intentional that they were signed in the original.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index a330c69..6d8a986 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -2755,9 +2755,10 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 	struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
 	struct ndis_80211_assoc_info *info = NULL;
 	u8 bssid[ETH_ALEN];
-	int resp_ie_len, req_ie_len;
+	unsigned int resp_ie_len, req_ie_len;
+	unsigned int offset;
 	u8 *req_ie, *resp_ie;
-	int ret, offset;
+	int ret;
 	bool roamed = false;
 	bool match_bss;
 
@@ -2785,6 +2786,8 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 		ret = get_association_info(usbdev, info, CONTROL_BUFFER_SIZE);
 		if (!ret) {
 			req_ie_len = le32_to_cpu(info->req_ie_length);
+			if (req_ie_len > CONTROL_BUFFER_SIZE)
+				req_ie_len = CONTROL_BUFFER_SIZE;
 			if (req_ie_len > 0) {
 				offset = le32_to_cpu(info->offset_req_ies);
 
@@ -2799,6 +2802,8 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 			}
 
 			resp_ie_len = le32_to_cpu(info->resp_ie_length);
+			if (resp_ie_len > CONTROL_BUFFER_SIZE)
+				resp_ie_len = CONTROL_BUFFER_SIZE;
 			if (resp_ie_len > 0) {
 				offset = le32_to_cpu(info->offset_resp_ies);
 

[patch 1/4] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Dan Carpenter]

If "offset" is negative then we can get past this check:
	if (offset > CONTROL_BUFFER_SIZE)
Or if we pick a very high "req_ie_len" then we can get around the check:
	if (offset + req_ie_len > CONTROL_BUFFER_SIZE)

I made "resp_ie_len" and "req_ie_len" unsigned.  I don't know if it was
intentional that they were signed in the original.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index a330c69..6d8a986 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -2755,9 +2755,10 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 	struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
 	struct ndis_80211_assoc_info *info = NULL;
 	u8 bssid[ETH_ALEN];
-	int resp_ie_len, req_ie_len;
+	unsigned int resp_ie_len, req_ie_len;
+	unsigned int offset;
 	u8 *req_ie, *resp_ie;
-	int ret, offset;
+	int ret;
 	bool roamed = false;
 	bool match_bss;
 
@@ -2785,6 +2786,8 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 		ret = get_association_info(usbdev, info, CONTROL_BUFFER_SIZE);
 		if (!ret) {
 			req_ie_len = le32_to_cpu(info->req_ie_length);
+			if (req_ie_len > CONTROL_BUFFER_SIZE)
+				req_ie_len = CONTROL_BUFFER_SIZE;
 			if (req_ie_len > 0) {
 				offset = le32_to_cpu(info->offset_req_ies);
 
@@ -2799,6 +2802,8 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 			}
 
 			resp_ie_len = le32_to_cpu(info->resp_ie_length);
+			if (resp_ie_len > CONTROL_BUFFER_SIZE)
+				resp_ie_len = CONTROL_BUFFER_SIZE;
 			if (resp_ie_len > 0) {
 				offset = le32_to_cpu(info->offset_resp_ies);
 

Re: [patch 1/4] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[walter harms]

Am 29.02.2012 07:35, schrieb Dan Carpenter:
> If "offset" is negative then we can get past this check:
> 	if (offset > CONTROL_BUFFER_SIZE)
> Or if we pick a very high "req_ie_len" then we can get around the check:
> 	if (offset + req_ie_len > CONTROL_BUFFER_SIZE)
> 
> I made "resp_ie_len" and "req_ie_len" unsigned.  I don't know if it was
> intentional that they were signed in the original.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
> index a330c69..6d8a986 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -2755,9 +2755,10 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
>  	struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
>  	struct ndis_80211_assoc_info *info = NULL;
>  	u8 bssid[ETH_ALEN];
> -	int resp_ie_len, req_ie_len;
> +	unsigned int resp_ie_len, req_ie_len;
> +	unsigned int offset;
>  	u8 *req_ie, *resp_ie;
> -	int ret, offset;
> +	int ret;
>  	bool roamed = false;
>  	bool match_bss;
>  
> @@ -2785,6 +2786,8 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
>  		ret = get_association_info(usbdev, info, CONTROL_BUFFER_SIZE);
>  		if (!ret) {
>  			req_ie_len = le32_to_cpu(info->req_ie_length);
> +			if (req_ie_len > CONTROL_BUFFER_SIZE)
> +				req_ie_len = CONTROL_BUFFER_SIZE;
>  			if (req_ie_len > 0) {
>  				offset = le32_to_cpu(info->offset_req_ies);
>  
> @@ -2799,6 +2802,8 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
>  			}
>  
>  			resp_ie_len = le32_to_cpu(info->resp_ie_length);
> +			if (resp_ie_len > CONTROL_BUFFER_SIZE)
> +				resp_ie_len = CONTROL_BUFFER_SIZE;
>  			if (resp_ie_len > 0) {
>  				offset = le32_to_cpu(info->offset_resp_ies);
> 


hi dan,
the check below  "if (resp_ie_len > 0)" looks strange for an unsigned.

re,
 wh


> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [patch 1/4] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[walter harms]

Am 29.02.2012 07:35, schrieb Dan Carpenter:
> If "offset" is negative then we can get past this check:
> 	if (offset > CONTROL_BUFFER_SIZE)
> Or if we pick a very high "req_ie_len" then we can get around the check:
> 	if (offset + req_ie_len > CONTROL_BUFFER_SIZE)
> 
> I made "resp_ie_len" and "req_ie_len" unsigned.  I don't know if it was
> intentional that they were signed in the original.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
> index a330c69..6d8a986 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -2755,9 +2755,10 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
>  	struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
>  	struct ndis_80211_assoc_info *info = NULL;
>  	u8 bssid[ETH_ALEN];
> -	int resp_ie_len, req_ie_len;
> +	unsigned int resp_ie_len, req_ie_len;
> +	unsigned int offset;
>  	u8 *req_ie, *resp_ie;
> -	int ret, offset;
> +	int ret;
>  	bool roamed = false;
>  	bool match_bss;
>  
> @@ -2785,6 +2786,8 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
>  		ret = get_association_info(usbdev, info, CONTROL_BUFFER_SIZE);
>  		if (!ret) {
>  			req_ie_len = le32_to_cpu(info->req_ie_length);
> +			if (req_ie_len > CONTROL_BUFFER_SIZE)
> +				req_ie_len = CONTROL_BUFFER_SIZE;
>  			if (req_ie_len > 0) {
>  				offset = le32_to_cpu(info->offset_req_ies);
>  
> @@ -2799,6 +2802,8 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
>  			}
>  
>  			resp_ie_len = le32_to_cpu(info->resp_ie_length);
> +			if (resp_ie_len > CONTROL_BUFFER_SIZE)
> +				resp_ie_len = CONTROL_BUFFER_SIZE;
>  			if (resp_ie_len > 0) {
>  				offset = le32_to_cpu(info->offset_resp_ies);
> 


hi dan,
the check below  "if (resp_ie_len > 0)" looks strange for an unsigned.

re,
 wh


> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [patch 1/4] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Wed, Feb 29, 2012 at 09:21:29AM +0100, walter harms wrote:
> >  			resp_ie_len = le32_to_cpu(info->resp_ie_length);
> > +			if (resp_ie_len > CONTROL_BUFFER_SIZE)
> > +				resp_ie_len = CONTROL_BUFFER_SIZE;
> >  			if (resp_ie_len > 0) {
> >  				offset = le32_to_cpu(info->offset_resp_ies);
> > 
> 
> 
> hi dan,
> the check below  "if (resp_ie_len > 0)" looks strange for an unsigned.
> 

Good point.  I'll resend.

regards,
dan carpenter


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch 1/4] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Wed, Feb 29, 2012 at 09:21:29AM +0100, walter harms wrote:
> >  			resp_ie_len = le32_to_cpu(info->resp_ie_length);
> > +			if (resp_ie_len > CONTROL_BUFFER_SIZE)
> > +				resp_ie_len = CONTROL_BUFFER_SIZE;
> >  			if (resp_ie_len > 0) {
> >  				offset = le32_to_cpu(info->offset_resp_ies);
> > 
> 
> 
> hi dan,
> the check below  "if (resp_ie_len > 0)" looks strange for an unsigned.
> 

Good point.  I'll resend.

regards,
dan carpenter


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[patch 1/4 v2] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 1929 bytes --]

If "offset" is negative then we can get past this check:
	if (offset > CONTROL_BUFFER_SIZE)
Or if we pick a very high "req_ie_len" then we can get around the check:
	if (offset + req_ie_len > CONTROL_BUFFER_SIZE)

I made "resp_ie_len" and "req_ie_len" unsigned.  I don't know if it was
intentional that they were signed in the original.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Fixed a style issue for Walter Harms.  Changed > 0 to != 0.

diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index a330c69..dde45ef 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -2755,9 +2755,10 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 	struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
 	struct ndis_80211_assoc_info *info = NULL;
 	u8 bssid[ETH_ALEN];
-	int resp_ie_len, req_ie_len;
+	unsigned int resp_ie_len, req_ie_len;
+	unsigned int offset;
 	u8 *req_ie, *resp_ie;
-	int ret, offset;
+	int ret;
 	bool roamed = false;
 	bool match_bss;
 
@@ -2785,7 +2786,9 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 		ret = get_association_info(usbdev, info, CONTROL_BUFFER_SIZE);
 		if (!ret) {
 			req_ie_len = le32_to_cpu(info->req_ie_length);
-			if (req_ie_len > 0) {
+			if (req_ie_len > CONTROL_BUFFER_SIZE)
+				req_ie_len = CONTROL_BUFFER_SIZE;
+			if (req_ie_len != 0) {
 				offset = le32_to_cpu(info->offset_req_ies);
 
 				if (offset > CONTROL_BUFFER_SIZE)
@@ -2799,7 +2802,9 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 			}
 
 			resp_ie_len = le32_to_cpu(info->resp_ie_length);
-			if (resp_ie_len > 0) {
+			if (resp_ie_len > CONTROL_BUFFER_SIZE)
+				resp_ie_len = CONTROL_BUFFER_SIZE;
+			if (resp_ie_len != 0) {
 				offset = le32_to_cpu(info->offset_resp_ies);
 
 				if (offset > CONTROL_BUFFER_SIZE)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[patch 1/4 v2] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 1929 bytes --]

If "offset" is negative then we can get past this check:
	if (offset > CONTROL_BUFFER_SIZE)
Or if we pick a very high "req_ie_len" then we can get around the check:
	if (offset + req_ie_len > CONTROL_BUFFER_SIZE)

I made "resp_ie_len" and "req_ie_len" unsigned.  I don't know if it was
intentional that they were signed in the original.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Fixed a style issue for Walter Harms.  Changed > 0 to != 0.

diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index a330c69..dde45ef 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -2755,9 +2755,10 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 	struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
 	struct ndis_80211_assoc_info *info = NULL;
 	u8 bssid[ETH_ALEN];
-	int resp_ie_len, req_ie_len;
+	unsigned int resp_ie_len, req_ie_len;
+	unsigned int offset;
 	u8 *req_ie, *resp_ie;
-	int ret, offset;
+	int ret;
 	bool roamed = false;
 	bool match_bss;
 
@@ -2785,7 +2786,9 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 		ret = get_association_info(usbdev, info, CONTROL_BUFFER_SIZE);
 		if (!ret) {
 			req_ie_len = le32_to_cpu(info->req_ie_length);
-			if (req_ie_len > 0) {
+			if (req_ie_len > CONTROL_BUFFER_SIZE)
+				req_ie_len = CONTROL_BUFFER_SIZE;
+			if (req_ie_len != 0) {
 				offset = le32_to_cpu(info->offset_req_ies);
 
 				if (offset > CONTROL_BUFFER_SIZE)
@@ -2799,7 +2802,9 @@ static void rndis_wlan_do_link_up_work(struct usbnet *usbdev)
 			}
 
 			resp_ie_len = le32_to_cpu(info->resp_ie_length);
-			if (resp_ie_len > 0) {
+			if (resp_ie_len > CONTROL_BUFFER_SIZE)
+				resp_ie_len = CONTROL_BUFFER_SIZE;
+			if (resp_ie_len != 0) {
 				offset = le32_to_cpu(info->offset_resp_ies);
 
 				if (offset > CONTROL_BUFFER_SIZE)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch 1/4 v2] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[bojan prtvar]

Hi,


On Thu, Mar 1, 2012 at 8:02 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
>
>
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Fixed a style issue for Walter Harms.  Changed > 0 to != 0.
>

Why not just  if (req_ie_len) and if (resp_ie_len) ?

Regards,
Bojan

Re: [patch 1/4 v2] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[bojan prtvar]

Hi,


On Thu, Mar 1, 2012 at 8:02 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
>
>
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Fixed a style issue for Walter Harms.  Changed > 0 to != 0.
>

Why not just  if (req_ie_len) and if (resp_ie_len) ?

Regards,
Bojan

Re: [patch 1/4] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Jussi Kivilinna]

Quoting Dan Carpenter <dan.carpenter@oracle.com>:

> If "offset" is negative then we can get past this check:
> 	if (offset > CONTROL_BUFFER_SIZE)
> Or if we pick a very high "req_ie_len" then we can get around the check:
> 	if (offset + req_ie_len > CONTROL_BUFFER_SIZE)
>
> I made "resp_ie_len" and "req_ie_len" unsigned.  I don't know if it was
> intentional that they were signed in the original.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/net/wireless/rndis_wlan.c  
> b/drivers/net/wireless/rndis_wlan.c
> index a330c69..6d8a986 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -2755,9 +2755,10 @@ static void rndis_wlan_do_link_up_work(struct  
> usbnet *usbdev)
>  	struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
>  	struct ndis_80211_assoc_info *info = NULL;
>  	u8 bssid[ETH_ALEN];
> -	int resp_ie_len, req_ie_len;
> +	unsigned int resp_ie_len, req_ie_len;
> +	unsigned int offset;
>  	u8 *req_ie, *resp_ie;
> -	int ret, offset;
> +	int ret;
>  	bool roamed = false;
>  	bool match_bss;
>
> @@ -2785,6 +2786,8 @@ static void rndis_wlan_do_link_up_work(struct  
> usbnet *usbdev)
>  		ret = get_association_info(usbdev, info, CONTROL_BUFFER_SIZE);
>  		if (!ret) {
>  			req_ie_len = le32_to_cpu(info->req_ie_length);
> +			if (req_ie_len > CONTROL_BUFFER_SIZE)
> +				req_ie_len = CONTROL_BUFFER_SIZE;
>  			if (req_ie_len > 0) {
>  				offset = le32_to_cpu(info->offset_req_ies);
>
> @@ -2799,6 +2802,8 @@ static void rndis_wlan_do_link_up_work(struct  
> usbnet *usbdev)
>  			}
>
>  			resp_ie_len = le32_to_cpu(info->resp_ie_length);
> +			if (resp_ie_len > CONTROL_BUFFER_SIZE)
> +				resp_ie_len = CONTROL_BUFFER_SIZE;
>  			if (resp_ie_len > 0) {

(resp_ie_len > 0), (resp_ie_len != 0), (resp_ie_len) .. all fine by me,

Acked-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>

>  				offset = le32_to_cpu(info->offset_resp_ies);
>
>
>

Re: [patch 1/4] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Jussi Kivilinna]

Quoting Dan Carpenter <dan.carpenter@oracle.com>:

> If "offset" is negative then we can get past this check:
> 	if (offset > CONTROL_BUFFER_SIZE)
> Or if we pick a very high "req_ie_len" then we can get around the check:
> 	if (offset + req_ie_len > CONTROL_BUFFER_SIZE)
>
> I made "resp_ie_len" and "req_ie_len" unsigned.  I don't know if it was
> intentional that they were signed in the original.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/net/wireless/rndis_wlan.c  
> b/drivers/net/wireless/rndis_wlan.c
> index a330c69..6d8a986 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -2755,9 +2755,10 @@ static void rndis_wlan_do_link_up_work(struct  
> usbnet *usbdev)
>  	struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
>  	struct ndis_80211_assoc_info *info = NULL;
>  	u8 bssid[ETH_ALEN];
> -	int resp_ie_len, req_ie_len;
> +	unsigned int resp_ie_len, req_ie_len;
> +	unsigned int offset;
>  	u8 *req_ie, *resp_ie;
> -	int ret, offset;
> +	int ret;
>  	bool roamed = false;
>  	bool match_bss;
>
> @@ -2785,6 +2786,8 @@ static void rndis_wlan_do_link_up_work(struct  
> usbnet *usbdev)
>  		ret = get_association_info(usbdev, info, CONTROL_BUFFER_SIZE);
>  		if (!ret) {
>  			req_ie_len = le32_to_cpu(info->req_ie_length);
> +			if (req_ie_len > CONTROL_BUFFER_SIZE)
> +				req_ie_len = CONTROL_BUFFER_SIZE;
>  			if (req_ie_len > 0) {
>  				offset = le32_to_cpu(info->offset_req_ies);
>
> @@ -2799,6 +2802,8 @@ static void rndis_wlan_do_link_up_work(struct  
> usbnet *usbdev)
>  			}
>
>  			resp_ie_len = le32_to_cpu(info->resp_ie_length);
> +			if (resp_ie_len > CONTROL_BUFFER_SIZE)
> +				resp_ie_len = CONTROL_BUFFER_SIZE;
>  			if (resp_ie_len > 0) {

(resp_ie_len > 0), (resp_ie_len != 0), (resp_ie_len) .. all fine by me,

Acked-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>

>  				offset = le32_to_cpu(info->offset_resp_ies);
>
>
>

Re: [patch 1/4 v2] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

On Thu, Mar 01, 2012 at 10:51:37AM +0100, bojan prtvar wrote:
> Hi,
> 
> 
> On Thu, Mar 1, 2012 at 8:02 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> >
> >
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: Fixed a style issue for Walter Harms.  Changed > 0 to != 0.
> >
> 
> Why not just  if (req_ie_len) and if (resp_ie_len) ?
> 

It could go either way.  I wrote it that way first, then I decided
that zero was a special enough case to draw attention to it.  In
this case it felt like zero was its own thing.

For allocation failures I would do:
	foo = kmalloc();
	if (!foo)
		return -ENOMEM;
Allocation failures are not interesting and the NULL doesn't have a
special meaning and doesn't need explanation.

Hard to explain.

regards,
dan carpenter

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch 1/4 v2] rndis_wlan: integer overflows in rndis_wlan_do_link_up_work()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

On Thu, Mar 01, 2012 at 10:51:37AM +0100, bojan prtvar wrote:
> Hi,
> 
> 
> On Thu, Mar 1, 2012 at 8:02 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> >
> >
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: Fixed a style issue for Walter Harms.  Changed > 0 to != 0.
> >
> 
> Why not just  if (req_ie_len) and if (resp_ie_len) ?
> 

It could go either way.  I wrote it that way first, then I decided
that zero was a special enough case to draw attention to it.  In
this case it felt like zero was its own thing.

For allocation failures I would do:
	foo = kmalloc();
	if (!foo)
		return -ENOMEM;
Allocation failures are not interesting and the NULL doesn't have a
special meaning and doesn't need explanation.

Hard to explain.

regards,
dan carpenter

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]