netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Fix some bugs in staging drivers
@ 2014-04-09 16:12 Larry Finger
  2014-04-09 16:12 ` [PATCH 1/3] staging: r8188eu: Calling rtw_get_stainfo() with a NULL sta_addr will return NULL Larry Finger
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Larry Finger @ 2014-04-09 16:12 UTC (permalink / raw)
  To: gregkh; +Cc: devel, netdev, Larry Finger

While analyzing parts of the recently merged r8723au driver, Jes Sorensen
found two bugs that are found in other Reaktek drivers in staging. This
set of patches fixes them.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>


Larry Finger (3):
  staging: r8188eu: Calling rtw_get_stainfo() with a NULL sta_addr will
    return NULL
  staging: r8188eu: Fix case where ethtype was never obtained and always
    be checked against 0
  staging: r8712u: Fix case where ethtype was never obtained and always
    be checked against 0

 drivers/staging/rtl8188eu/core/rtw_recv.c | 24 ++++++++++--------------
 drivers/staging/rtl8712/rtl871x_recv.c    | 15 +++++++--------
 2 files changed, 17 insertions(+), 22 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] staging: r8188eu: Calling rtw_get_stainfo() with a NULL sta_addr will return NULL
  2014-04-09 16:12 [PATCH 0/3] Fix some bugs in staging drivers Larry Finger
@ 2014-04-09 16:12 ` Larry Finger
  2014-04-09 16:12 ` [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0 Larry Finger
  2014-04-09 16:13 ` [PATCH 3/3] staging: r8712u: " Larry Finger
  2 siblings, 0 replies; 16+ messages in thread
From: Larry Finger @ 2014-04-09 16:12 UTC (permalink / raw)
  To: gregkh; +Cc: devel, netdev, Stable, Larry Finger

This makes the follow-on check for psta != NULL pointless and makes
the whole exercise rather pointless. This is another case of why
blindly zero-initializing variables when they are declared is bad.

Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org>
---
 drivers/staging/rtl8188eu/core/rtw_recv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 636ec55..01fcabc 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -545,7 +545,7 @@ static struct recv_frame *decryptor(struct adapter *padapter,
 static struct recv_frame *portctrl(struct adapter *adapter,
 				   struct recv_frame *precv_frame)
 {
-	u8   *psta_addr = NULL, *ptr;
+	u8   *psta_addr, *ptr;
 	uint  auth_alg;
 	struct recv_frame *pfhdr;
 	struct sta_info *psta;
@@ -558,7 +558,6 @@ static struct recv_frame *portctrl(struct adapter *adapter,
 
 
 	pstapriv = &adapter->stapriv;
-	psta = rtw_get_stainfo(pstapriv, psta_addr);
 
 	auth_alg = adapter->securitypriv.dot11AuthAlgrthm;
 
@@ -566,6 +565,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
 	pfhdr = precv_frame;
 	pattrib = &pfhdr->attrib;
 	psta_addr = pattrib->ta;
+	psta = rtw_get_stainfo(pstapriv, psta_addr);
 
 	prtnframe = NULL;
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:12 [PATCH 0/3] Fix some bugs in staging drivers Larry Finger
  2014-04-09 16:12 ` [PATCH 1/3] staging: r8188eu: Calling rtw_get_stainfo() with a NULL sta_addr will return NULL Larry Finger
@ 2014-04-09 16:12 ` Larry Finger
  2014-04-09 16:24   ` Sergei Shtylyov
                     ` (2 more replies)
  2014-04-09 16:13 ` [PATCH 3/3] staging: r8712u: " Larry Finger
  2 siblings, 3 replies; 16+ messages in thread
From: Larry Finger @ 2014-04-09 16:12 UTC (permalink / raw)
  To: gregkh; +Cc: devel, netdev, Stable, Larry Finger

Zero-initializing ether_type masked that the ether type would never be
obtained for 8021x packets and the comparition against eapol_type
would always fail.

Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org>
---
 drivers/staging/rtl8188eu/core/rtw_recv.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 01fcabc..61084d6 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -551,7 +551,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
 	struct sta_info *psta;
 	struct sta_priv *pstapriv;
 	struct recv_frame *prtnframe;
-	u16	ether_type = 0;
+	u16	ether_type;
 	u16  eapol_type = 0x888e;/* for Funia BD's WPA issue */
 	struct rx_pkt_attrib *pattrib;
 	__be16 be_tmp;
@@ -571,19 +571,17 @@ static struct recv_frame *portctrl(struct adapter *adapter,
 
 	RT_TRACE(_module_rtl871x_recv_c_, _drv_info_, ("########portctrl:adapter->securitypriv.dot11AuthAlgrthm=%d\n", adapter->securitypriv.dot11AuthAlgrthm));
 
-	if (auth_alg == 2) {
+	if (auth_alg == dot11AuthAlgrthm_8021X) {
+		/* get ether_type */
+		ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;
+		memcpy(&ether_type, ptr, 2);
+		ether_type = ntohs((unsigned short)ether_type);
+
 		if ((psta != NULL) && (psta->ieee8021x_blocked)) {
 			/* blocked */
 			/* only accept EAPOL frame */
 			RT_TRACE(_module_rtl871x_recv_c_, _drv_info_, ("########portctrl:psta->ieee8021x_blocked==1\n"));
 
-			prtnframe = precv_frame;
-
-			/* get ether_type */
-			ptr = ptr+pfhdr->attrib.hdrlen+pfhdr->attrib.iv_len+LLC_HEADER_SIZE;
-			memcpy(&be_tmp, ptr, 2);
-			ether_type = ntohs(be_tmp);
-
 			if (ether_type == eapol_type) {
 				prtnframe = precv_frame;
 			} else {
@@ -616,9 +614,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
 	} else {
 		prtnframe = precv_frame;
 	}
-
-
-		return prtnframe;
+	return prtnframe;
 }
 
 static int recv_decache(struct recv_frame *precv_frame, u8 bretry,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] staging: r8712u: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:12 [PATCH 0/3] Fix some bugs in staging drivers Larry Finger
  2014-04-09 16:12 ` [PATCH 1/3] staging: r8188eu: Calling rtw_get_stainfo() with a NULL sta_addr will return NULL Larry Finger
  2014-04-09 16:12 ` [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0 Larry Finger
@ 2014-04-09 16:13 ` Larry Finger
  2014-04-09 16:25   ` Sergei Shtylyov
  2014-04-16 18:46   ` Greg KH
  2 siblings, 2 replies; 16+ messages in thread
From: Larry Finger @ 2014-04-09 16:13 UTC (permalink / raw)
  To: gregkh; +Cc: devel, netdev, Stable, Larry Finger

Zero-initializing ether_type masked that the ether type would never be
obtained for 8021x packets and the comparition against eapol_type
would always fail.

Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org>
---
 drivers/staging/rtl8712/rtl871x_recv.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_recv.c b/drivers/staging/rtl8712/rtl871x_recv.c
index 23ec684..d8d1a76 100644
--- a/drivers/staging/rtl8712/rtl871x_recv.c
+++ b/drivers/staging/rtl8712/rtl871x_recv.c
@@ -254,7 +254,7 @@ union recv_frame *r8712_portctrl(struct _adapter *adapter,
 	struct sta_info *psta;
 	struct	sta_priv *pstapriv;
 	union recv_frame *prtnframe;
-	u16 ether_type = 0;
+	u16 ether_type;
 
 	pstapriv = &adapter->stapriv;
 	ptr = get_recvframe_data(precv_frame);
@@ -262,16 +262,15 @@ union recv_frame *r8712_portctrl(struct _adapter *adapter,
 	psta_addr = pfhdr->attrib.ta;
 	psta = r8712_get_stainfo(pstapriv, psta_addr);
 	auth_alg = adapter->securitypriv.AuthAlgrthm;
-	if (auth_alg == 2) {
+	if (auth_alg == dot11AuthAlgrthm_8021X) {
+		/* get ether_type */
+		ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;
+		memcpy(&ether_type, ptr, 2);
+		ether_type = ntohs((unsigned short)ether_type);
+
 		if ((psta != NULL) && (psta->ieee8021x_blocked)) {
 			/* blocked
 			 * only accept EAPOL frame */
-			prtnframe = precv_frame;
-			/*get ether_type */
-			ptr = ptr + pfhdr->attrib.hdrlen +
-			      pfhdr->attrib.iv_len + LLC_HEADER_SIZE;
-			memcpy(&ether_type, ptr, 2);
-			ether_type = ntohs((unsigned short)ether_type);
 			if (ether_type == 0x888e)
 				prtnframe = precv_frame;
 			else {
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:12 ` [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0 Larry Finger
@ 2014-04-09 16:24   ` Sergei Shtylyov
  2014-04-09 17:44     ` Larry Finger
  2014-04-09 19:02     ` Dan Carpenter
  2014-04-09 18:51   ` Dan Carpenter
  2014-04-16 18:45   ` Greg KH
  2 siblings, 2 replies; 16+ messages in thread
From: Sergei Shtylyov @ 2014-04-09 16:24 UTC (permalink / raw)
  To: Larry Finger, gregkh; +Cc: devel, netdev, Stable

On 04/09/2014 08:12 PM, Larry Finger wrote:

> Zero-initializing ether_type masked that the ether type would never be
> obtained for 8021x packets and the comparition against eapol_type
> would always fail.

> Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@vger.kernel.org>
> ---
>   drivers/staging/rtl8188eu/core/rtw_recv.c | 20 ++++++++------------
>   1 file changed, 8 insertions(+), 12 deletions(-)

> diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
> index 01fcabc..61084d6 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_recv.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
> @@ -551,7 +551,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
>   	struct sta_info *psta;
>   	struct sta_priv *pstapriv;
>   	struct recv_frame *prtnframe;
> -	u16	ether_type = 0;
> +	u16	ether_type;

    I suggest:

	u16 ethertype;

>   	u16  eapol_type = 0x888e;/* for Funia BD's WPA issue */
>   	struct rx_pkt_attrib *pattrib;
>   	__be16 be_tmp;
> @@ -571,19 +571,17 @@ static struct recv_frame *portctrl(struct adapter *adapter,
>
>   	RT_TRACE(_module_rtl871x_recv_c_, _drv_info_, ("########portctrl:adapter->securitypriv.dot11AuthAlgrthm=%d\n", adapter->securitypriv.dot11AuthAlgrthm));
>
> -	if (auth_alg == 2) {
> +	if (auth_alg == dot11AuthAlgrthm_8021X) {
> +		/* get ether_type */
> +		ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;

    Why not:

		ptr += pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;

WBR, Sergei

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] staging: r8712u: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:13 ` [PATCH 3/3] staging: r8712u: " Larry Finger
@ 2014-04-09 16:25   ` Sergei Shtylyov
  2014-04-09 17:45     ` Larry Finger
  2014-04-16 18:46   ` Greg KH
  1 sibling, 1 reply; 16+ messages in thread
From: Sergei Shtylyov @ 2014-04-09 16:25 UTC (permalink / raw)
  To: Larry Finger, gregkh; +Cc: devel, netdev, Stable

On 04/09/2014 08:13 PM, Larry Finger wrote:

> Zero-initializing ether_type masked that the ether type would never be
> obtained for 8021x packets and the comparition against eapol_type
> would always fail.

> Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@vger.kernel.org>
> ---
>   drivers/staging/rtl8712/rtl871x_recv.c | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)

> diff --git a/drivers/staging/rtl8712/rtl871x_recv.c b/drivers/staging/rtl8712/rtl871x_recv.c
> index 23ec684..d8d1a76 100644
> --- a/drivers/staging/rtl8712/rtl871x_recv.c
> +++ b/drivers/staging/rtl8712/rtl871x_recv.c
[...]
> @@ -262,16 +262,15 @@ union recv_frame *r8712_portctrl(struct _adapter *adapter,
>   	psta_addr = pfhdr->attrib.ta;
>   	psta = r8712_get_stainfo(pstapriv, psta_addr);
>   	auth_alg = adapter->securitypriv.AuthAlgrthm;
> -	if (auth_alg == 2) {
> +	if (auth_alg == dot11AuthAlgrthm_8021X) {
> +		/* get ether_type */
> +		ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;

    Why not:

		ptr += pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;

WBR, Sergei

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:24   ` Sergei Shtylyov
@ 2014-04-09 17:44     ` Larry Finger
  2014-04-09 19:02     ` Dan Carpenter
  1 sibling, 0 replies; 16+ messages in thread
From: Larry Finger @ 2014-04-09 17:44 UTC (permalink / raw)
  To: Sergei Shtylyov, gregkh; +Cc: devel, netdev, Stable

On 04/09/2014 11:24 AM, Sergei Shtylyov wrote:
> On 04/09/2014 08:12 PM, Larry Finger wrote:
>
>> Zero-initializing ether_type masked that the ether type would never be
>> obtained for 8021x packets and the comparition against eapol_type
>> would always fail.
>
>> Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Cc: Stable <stable@vger.kernel.org>
>> ---
>>   drivers/staging/rtl8188eu/core/rtw_recv.c | 20 ++++++++------------
>>   1 file changed, 8 insertions(+), 12 deletions(-)
>
>> diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c
>> b/drivers/staging/rtl8188eu/core/rtw_recv.c
>> index 01fcabc..61084d6 100644
>> --- a/drivers/staging/rtl8188eu/core/rtw_recv.c
>> +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
>> @@ -551,7 +551,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
>>       struct sta_info *psta;
>>       struct sta_priv *pstapriv;
>>       struct recv_frame *prtnframe;
>> -    u16    ether_type = 0;
>> +    u16    ether_type;
>
>     I suggest:
>
>      u16 ethertype;
>
>>       u16  eapol_type = 0x888e;/* for Funia BD's WPA issue */
>>       struct rx_pkt_attrib *pattrib;
>>       __be16 be_tmp;
>> @@ -571,19 +571,17 @@ static struct recv_frame *portctrl(struct adapter *adapter,
>>
>>       RT_TRACE(_module_rtl871x_recv_c_, _drv_info_,
>> ("########portctrl:adapter->securitypriv.dot11AuthAlgrthm=%d\n",
>> adapter->securitypriv.dot11AuthAlgrthm));
>>
>> -    if (auth_alg == 2) {
>> +    if (auth_alg == dot11AuthAlgrthm_8021X) {
>> +        /* get ether_type */
>> +        ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;
>
>     Why not:
>
>          ptr += pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;

It seems to me that it is only a matter of preference. Exactly the same object 
code will be generated. If saving 5 characters in the source file is really 
important, then I think I'm in the wrong business.

Larry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] staging: r8712u: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:25   ` Sergei Shtylyov
@ 2014-04-09 17:45     ` Larry Finger
  0 siblings, 0 replies; 16+ messages in thread
From: Larry Finger @ 2014-04-09 17:45 UTC (permalink / raw)
  To: Sergei Shtylyov, gregkh; +Cc: devel, netdev, Stable

On 04/09/2014 11:25 AM, Sergei Shtylyov wrote:
> On 04/09/2014 08:13 PM, Larry Finger wrote:
>
>> Zero-initializing ether_type masked that the ether type would never be
>> obtained for 8021x packets and the comparition against eapol_type
>> would always fail.
>
>> Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Cc: Stable <stable@vger.kernel.org>
>> ---
>>   drivers/staging/rtl8712/rtl871x_recv.c | 15 +++++++--------
>>   1 file changed, 7 insertions(+), 8 deletions(-)
>
>> diff --git a/drivers/staging/rtl8712/rtl871x_recv.c
>> b/drivers/staging/rtl8712/rtl871x_recv.c
>> index 23ec684..d8d1a76 100644
>> --- a/drivers/staging/rtl8712/rtl871x_recv.c
>> +++ b/drivers/staging/rtl8712/rtl871x_recv.c
> [...]
>> @@ -262,16 +262,15 @@ union recv_frame *r8712_portctrl(struct _adapter *adapter,
>>       psta_addr = pfhdr->attrib.ta;
>>       psta = r8712_get_stainfo(pstapriv, psta_addr);
>>       auth_alg = adapter->securitypriv.AuthAlgrthm;
>> -    if (auth_alg == 2) {
>> +    if (auth_alg == dot11AuthAlgrthm_8021X) {
>> +        /* get ether_type */
>> +        ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;
>
>     Why not:
>
>          ptr += pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;

My comment is the same as the one I sent for patch 2.

Larry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:12 ` [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0 Larry Finger
  2014-04-09 16:24   ` Sergei Shtylyov
@ 2014-04-09 18:51   ` Dan Carpenter
  2014-04-09 19:00     ` Dan Carpenter
  2014-04-10  9:14     ` David Laight
  2014-04-16 18:45   ` Greg KH
  2 siblings, 2 replies; 16+ messages in thread
From: Dan Carpenter @ 2014-04-09 18:51 UTC (permalink / raw)
  To: Larry Finger; +Cc: devel, gregkh, Stable, netdev

On Wed, Apr 09, 2014 at 11:12:59AM -0500, Larry Finger wrote:
> diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
> index 01fcabc..61084d6 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_recv.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
> @@ -551,7 +551,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
>  	struct sta_info *psta;
>  	struct sta_priv *pstapriv;
>  	struct recv_frame *prtnframe;
> -	u16	ether_type = 0;
> +	u16	ether_type;
>  	u16  eapol_type = 0x888e;/* for Funia BD's WPA issue */
>  	struct rx_pkt_attrib *pattrib;
>  	__be16 be_tmp;
> @@ -571,19 +571,17 @@ static struct recv_frame *portctrl(struct adapter *adapter,
>  
>  	RT_TRACE(_module_rtl871x_recv_c_, _drv_info_, ("########portctrl:adapter->securitypriv.dot11AuthAlgrthm=%d\n", adapter->securitypriv.dot11AuthAlgrthm));
>  
> -	if (auth_alg == 2) {
> +	if (auth_alg == dot11AuthAlgrthm_8021X) {
> +		/* get ether_type */
> +		ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;
> +		memcpy(&ether_type, ptr, 2);
> +		ether_type = ntohs((unsigned short)ether_type);

This cast doesn't make sense.  u16 and unsigned short are the same
thing.  Anyway, the "ether_type" should be declared as __be16 because
it's network endian.

https://lwn.net/Articles/205624/

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 18:51   ` Dan Carpenter
@ 2014-04-09 19:00     ` Dan Carpenter
  2014-04-10  9:14     ` David Laight
  1 sibling, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2014-04-09 19:00 UTC (permalink / raw)
  To: Larry Finger; +Cc: devel, gregkh, Stable, netdev

On Wed, Apr 09, 2014 at 09:51:53PM +0300, Dan Carpenter wrote:
> > -	if (auth_alg == 2) {
> > +	if (auth_alg == dot11AuthAlgrthm_8021X) {
> > +		/* get ether_type */
> > +		ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;
> > +		memcpy(&ether_type, ptr, 2);
> > +		ether_type = ntohs((unsigned short)ether_type);
> 
> This cast doesn't make sense.  u16 and unsigned short are the same
> thing.  Anyway, the "ether_type" should be declared as __be16 because
> it's network endian.
> 

OOps.  You mostly use ether_type as u16.  But the cast is still wrong it
should be:

		ether_type = ntohs((__be16)ether_type);

Or something.  You could use the be_tmp variable...  Doesn't this patch
introduce an unused variable warning?

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:24   ` Sergei Shtylyov
  2014-04-09 17:44     ` Larry Finger
@ 2014-04-09 19:02     ` Dan Carpenter
  2014-04-09 19:11       ` Sergei Shtylyov
  1 sibling, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2014-04-09 19:02 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: devel, gregkh, netdev, Stable, Larry Finger

On Wed, Apr 09, 2014 at 08:24:08PM +0400, Sergei Shtylyov wrote:
> >diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
> >index 01fcabc..61084d6 100644
> >--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
> >+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
> >@@ -551,7 +551,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
> >  	struct sta_info *psta;
> >  	struct sta_priv *pstapriv;
> >  	struct recv_frame *prtnframe;
> >-	u16	ether_type = 0;
> >+	u16	ether_type;
> 
>    I suggest:
> 
> 	u16 ethertype;
> 

I don't understand this suggestion.  Why is that name prefered?

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 19:02     ` Dan Carpenter
@ 2014-04-09 19:11       ` Sergei Shtylyov
  2014-04-09 19:44         ` Dan Carpenter
  0 siblings, 1 reply; 16+ messages in thread
From: Sergei Shtylyov @ 2014-04-09 19:11 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Larry Finger, gregkh, devel, netdev, Stable

On 04/09/2014 11:02 PM, Dan Carpenter wrote:

>>> diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
>>> index 01fcabc..61084d6 100644
>>> --- a/drivers/staging/rtl8188eu/core/rtw_recv.c
>>> +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
>>> @@ -551,7 +551,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
>>>   	struct sta_info *psta;
>>>   	struct sta_priv *pstapriv;
>>>   	struct recv_frame *prtnframe;
>>> -	u16	ether_type = 0;
>>> +	u16	ether_type;

>>     I suggest:

>> 	u16 ethertype;

> I don't understand this suggestion.  Why is that name prefered?

    Sorry, I've managed to somehow remove the underscore. :-/
My real suggestion was to replace the tab with a space.

> regards,
> dan carpenter

WBR, Sergei

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 19:11       ` Sergei Shtylyov
@ 2014-04-09 19:44         ` Dan Carpenter
  0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2014-04-09 19:44 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: devel, gregkh, netdev, Stable, Larry Finger

On Wed, Apr 09, 2014 at 11:11:52PM +0400, Sergei Shtylyov wrote:
> On 04/09/2014 11:02 PM, Dan Carpenter wrote:
> 
> >>>diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
> >>>index 01fcabc..61084d6 100644
> >>>--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
> >>>+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
> >>>@@ -551,7 +551,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
> >>>  	struct sta_info *psta;
> >>>  	struct sta_priv *pstapriv;
> >>>  	struct recv_frame *prtnframe;
> >>>-	u16	ether_type = 0;
> >>>+	u16	ether_type;
> 
> >>    I suggest:
> 
> >>	u16 ethertype;
> 
> >I don't understand this suggestion.  Why is that name prefered?
> 
>    Sorry, I've managed to somehow remove the underscore. :-/
> My real suggestion was to replace the tab with a space.

Ah, sure.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 18:51   ` Dan Carpenter
  2014-04-09 19:00     ` Dan Carpenter
@ 2014-04-10  9:14     ` David Laight
  1 sibling, 0 replies; 16+ messages in thread
From: David Laight @ 2014-04-10  9:14 UTC (permalink / raw)
  To: 'Dan Carpenter', Larry Finger; +Cc: devel, gregkh, Stable, netdev

From: Dan Carpenter
...
> > -	if (auth_alg == 2) {
> > +	if (auth_alg == dot11AuthAlgrthm_8021X) {
> > +		/* get ether_type */
> > +		ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;
> > +		memcpy(&ether_type, ptr, 2);
> > +		ether_type = ntohs((unsigned short)ether_type);
> 
> This cast doesn't make sense.  u16 and unsigned short are the same
> thing.  Anyway, the "ether_type" should be declared as __be16 because
> it's network endian.

Is it worth doing just:
	ether_type = ptr[0] << 8 | ptr[1];
Or, if 'ptr' can't be misaligned, just reading a BE short.
If the compiler doesn't inline memcpy() the above code is horrid.

Are there MD inline functions for reading misaligned 16/32 bit date
with specific endianness?

	David

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:12 ` [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0 Larry Finger
  2014-04-09 16:24   ` Sergei Shtylyov
  2014-04-09 18:51   ` Dan Carpenter
@ 2014-04-16 18:45   ` Greg KH
  2 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2014-04-16 18:45 UTC (permalink / raw)
  To: Larry Finger; +Cc: devel, netdev, Stable

On Wed, Apr 09, 2014 at 11:12:59AM -0500, Larry Finger wrote:
> Zero-initializing ether_type masked that the ether type would never be
> obtained for 8021x packets and the comparition against eapol_type
> would always fail.
> 
> Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@vger.kernel.org>
> ---
>  drivers/staging/rtl8188eu/core/rtw_recv.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)

Can you fix this one up and resend?  I've applied the other two in this
series now.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] staging: r8712u: Fix case where ethtype was never obtained and always be checked against 0
  2014-04-09 16:13 ` [PATCH 3/3] staging: r8712u: " Larry Finger
  2014-04-09 16:25   ` Sergei Shtylyov
@ 2014-04-16 18:46   ` Greg KH
  1 sibling, 0 replies; 16+ messages in thread
From: Greg KH @ 2014-04-16 18:46 UTC (permalink / raw)
  To: Larry Finger; +Cc: devel, netdev, Stable

On Wed, Apr 09, 2014 at 11:13:00AM -0500, Larry Finger wrote:
> Zero-initializing ether_type masked that the ether type would never be
> obtained for 8021x packets and the comparition against eapol_type
> would always fail.
> 
> Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@vger.kernel.org>
> ---
>  drivers/staging/rtl8712/rtl871x_recv.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)

This breaks the build :(

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-04-16 18:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-09 16:12 [PATCH 0/3] Fix some bugs in staging drivers Larry Finger
2014-04-09 16:12 ` [PATCH 1/3] staging: r8188eu: Calling rtw_get_stainfo() with a NULL sta_addr will return NULL Larry Finger
2014-04-09 16:12 ` [PATCH 2/3] staging: r8188eu: Fix case where ethtype was never obtained and always be checked against 0 Larry Finger
2014-04-09 16:24   ` Sergei Shtylyov
2014-04-09 17:44     ` Larry Finger
2014-04-09 19:02     ` Dan Carpenter
2014-04-09 19:11       ` Sergei Shtylyov
2014-04-09 19:44         ` Dan Carpenter
2014-04-09 18:51   ` Dan Carpenter
2014-04-09 19:00     ` Dan Carpenter
2014-04-10  9:14     ` David Laight
2014-04-16 18:45   ` Greg KH
2014-04-09 16:13 ` [PATCH 3/3] staging: r8712u: " Larry Finger
2014-04-09 16:25   ` Sergei Shtylyov
2014-04-09 17:45     ` Larry Finger
2014-04-16 18:46   ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).