* [PATCH V2] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
@ 2022-04-15 2:09 Haowen Bai
2022-04-15 5:22 ` Fabio M. De Francesco
2022-04-15 5:25 ` [PATCH V2] " Dan Carpenter
0 siblings, 2 replies; 21+ messages in thread
From: Haowen Bai @ 2022-04-15 2:09 UTC (permalink / raw)
To: gregkh, davem, dan.carpenter, len.baker, dave, edumazet
Cc: linux-staging, linux-kernel, Haowen Bai
function rtllib_rx_assoc_resp () unsigned errcode receive auth_parse()'s
errcode -ENOMEM.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
V1->V2: reduce return random value; print its own error message.
drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index 82bf05eb1cbf..8a0961e64a8c 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
spin_unlock_irqrestore(&ieee->lock, flags);
}
-static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
+static inline s16 auth_parse(struct net_device *dev, struct sk_buff *skb,
u8 **challenge, int *chlen)
{
struct rtllib_authentication *a;
@@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
if (skb->len < (sizeof(struct rtllib_authentication) -
sizeof(struct rtllib_info_element))) {
netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
- return 0xcafe;
+ return -EINVAL;
}
*challenge = NULL;
a = (struct rtllib_authentication *) skb->data;
@@ -1787,7 +1787,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
return -ENOMEM;
}
}
- return le16_to_cpu(a->status);
+ return a->status;
}
static int auth_rq_parse(struct net_device *dev, struct sk_buff *skb, u8 *dest)
@@ -2282,7 +2282,7 @@ rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb,
static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
{
- u16 errcode;
+ s16 errcode;
u8 *challenge;
int chlen = 0;
bool bSupportNmode = true, bHalfSupportNmode = false;
@@ -2292,8 +2292,8 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
if (errcode) {
ieee->softmac_stats.rx_auth_rs_err++;
netdev_info(ieee->dev,
- "Authentication response status code 0x%x",
- errcode);
+ "Authentication response status code %d",
+ le16_to_cpu(errcode));
rtllib_associate_abort(ieee);
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V2] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 2:09 [PATCH V2] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp() Haowen Bai
@ 2022-04-15 5:22 ` Fabio M. De Francesco
2022-04-15 5:31 ` Dan Carpenter
2022-04-15 5:25 ` [PATCH V2] " Dan Carpenter
1 sibling, 1 reply; 21+ messages in thread
From: Fabio M. De Francesco @ 2022-04-15 5:22 UTC (permalink / raw)
To: gregkh, davem, dan.carpenter, len.baker, dave, edumazet, Haowen Bai
Cc: linux-staging, linux-kernel, Haowen Bai
On venerdì 15 aprile 2022 04:09:31 CEST Haowen Bai wrote:
> function rtllib_rx_assoc_resp () unsigned errcode receive auth_parse()'s
> errcode -ENOMEM.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
> V1->V2: reduce return random value; print its own error message.
>
> drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> [snip]
It looks like you are doing too many things and that those aren't even
discussed in your commit message.
> @@ -2292,8 +2292,8 @@ static void rtllib_rx_auth_resp(struct
rtllib_device *ieee, struct sk_buff *skb)
> if (errcode) {
> ieee->softmac_stats.rx_auth_rs_err++;
> netdev_info(ieee->dev,
> - "Authentication response status code
0x%x",
> - errcode);
> + "Authentication response status code %d",
> + le16_to_cpu(errcode));
Why did you call le16_to_cpu(errcode)?
If I'm not missing something, it looks that auth_parse() already returns
native endian u16 values.
Thanks,
Fabio M. De Francesco
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 2:09 [PATCH V2] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp() Haowen Bai
2022-04-15 5:22 ` Fabio M. De Francesco
@ 2022-04-15 5:25 ` Dan Carpenter
1 sibling, 0 replies; 21+ messages in thread
From: Dan Carpenter @ 2022-04-15 5:25 UTC (permalink / raw)
To: Haowen Bai
Cc: gregkh, davem, len.baker, dave, edumazet, linux-staging, linux-kernel
On Fri, Apr 15, 2022 at 10:09:31AM +0800, Haowen Bai wrote:
> function rtllib_rx_assoc_resp () unsigned errcode receive auth_parse()'s
> errcode -ENOMEM.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
> V1->V2: reduce return random value; print its own error message.
>
> drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
> index 82bf05eb1cbf..8a0961e64a8c 100644
> --- a/drivers/staging/rtl8192e/rtllib_softmac.c
> +++ b/drivers/staging/rtl8192e/rtllib_softmac.c
> @@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
> spin_unlock_irqrestore(&ieee->lock, flags);
> }
>
> -static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> +static inline s16 auth_parse(struct net_device *dev, struct sk_buff *skb,
Could you make this an int instead of s16. s16 is always a bit weird.
> u8 **challenge, int *chlen)
> {
> struct rtllib_authentication *a;
> @@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> if (skb->len < (sizeof(struct rtllib_authentication) -
> sizeof(struct rtllib_info_element))) {
> netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
> - return 0xcafe;
> + return -EINVAL;
> }
> *challenge = NULL;
> a = (struct rtllib_authentication *) skb->data;
> @@ -1787,7 +1787,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> return -ENOMEM;
> }
> }
> - return le16_to_cpu(a->status);
> + return a->status;
But then just say:
if (a->status) {
netdev_info(ieee->dev, "blah blah failed");
return -EINVAL;
}
return 0;
If you look up what a->status is, it can only be
WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG which is not worth preserving really.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 5:22 ` Fabio M. De Francesco
@ 2022-04-15 5:31 ` Dan Carpenter
2022-04-15 5:50 ` [PATCH V3] " Haowen Bai
0 siblings, 1 reply; 21+ messages in thread
From: Dan Carpenter @ 2022-04-15 5:31 UTC (permalink / raw)
To: Fabio M. De Francesco
Cc: gregkh, davem, len.baker, dave, edumazet, Haowen Bai,
linux-staging, linux-kernel
On Fri, Apr 15, 2022 at 07:22:36AM +0200, Fabio M. De Francesco wrote:
> On venerdì 15 aprile 2022 04:09:31 CEST Haowen Bai wrote:
> > function rtllib_rx_assoc_resp () unsigned errcode receive auth_parse()'s
> > errcode -ENOMEM.
> >
> > Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> > ---
> > V1->V2: reduce return random value; print its own error message.
> >
> > drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
>
> > [snip]
>
> It looks like you are doing too many things and that those aren't even
> discussed in your commit message.
>
Nope. The patch is one thing, but maybe it could be described better.
Here is my proposed commit message:
"The rtllib_rx_assoc_resp() function has a signedness bug because it's
a declared as a u16 but it return -ENOMEM. When you look at it more
closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
Clean it up to just return standard kernel error codes. We can print
out the a->status before returning a regular error code. The printks
in the caller need to be adjusted as well."
regards,
dan carpenter
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V3] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 5:31 ` Dan Carpenter
@ 2022-04-15 5:50 ` Haowen Bai
2022-04-15 6:06 ` Fabio M. De Francesco
2022-04-15 6:18 ` [PATCH V3] " Dan Carpenter
0 siblings, 2 replies; 21+ messages in thread
From: Haowen Bai @ 2022-04-15 5:50 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Haowen Bai, linux-staging, linux-kernel
The rtllib_rx_assoc_resp() function has a signedness bug because it's
a declared as a u16 but it return -ENOMEM. When you look at it more
closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
Clean it up to just return standard kernel error codes. We can print
out the a->status before returning a regular error code. The printks
in the caller need to be adjusted as well.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
V1->V2: reduce return random value; print its own error message.
V2->V3: change commit message; change s16 -> int.
drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index 82bf05eb1cbf..4a1b9a94930f 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
spin_unlock_irqrestore(&ieee->lock, flags);
}
-static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
+static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
u8 **challenge, int *chlen)
{
struct rtllib_authentication *a;
@@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
if (skb->len < (sizeof(struct rtllib_authentication) -
sizeof(struct rtllib_info_element))) {
netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
- return 0xcafe;
+ return -EINVAL;
}
*challenge = NULL;
a = (struct rtllib_authentication *) skb->data;
@@ -1787,7 +1787,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
return -ENOMEM;
}
}
- return le16_to_cpu(a->status);
+ return a->status;
}
static int auth_rq_parse(struct net_device *dev, struct sk_buff *skb, u8 *dest)
@@ -2282,7 +2282,7 @@ rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb,
static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
{
- u16 errcode;
+ int errcode;
u8 *challenge;
int chlen = 0;
bool bSupportNmode = true, bHalfSupportNmode = false;
@@ -2292,8 +2292,8 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
if (errcode) {
ieee->softmac_stats.rx_auth_rs_err++;
netdev_info(ieee->dev,
- "Authentication response status code 0x%x",
- errcode);
+ "Authentication response status code %d",
+ le16_to_cpu(errcode));
rtllib_associate_abort(ieee);
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V3] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 5:50 ` [PATCH V3] " Haowen Bai
@ 2022-04-15 6:06 ` Fabio M. De Francesco
2022-04-15 6:10 ` Fabio M. De Francesco
2022-04-15 6:18 ` [PATCH V3] " Dan Carpenter
1 sibling, 1 reply; 21+ messages in thread
From: Fabio M. De Francesco @ 2022-04-15 6:06 UTC (permalink / raw)
To: Greg Kroah-Hartman, Haowen Bai; +Cc: Haowen Bai, linux-staging, linux-kernel
On venerdì 15 aprile 2022 07:50:36 CEST Haowen Bai wrote:
> The rtllib_rx_assoc_resp() function has a signedness bug because it's
> a declared as a u16 but it return -ENOMEM. When you look at it more
> closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
> a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
>
> Clean it up to just return standard kernel error codes. We can print
> out the a->status before returning a regular error code. The printks
> in the caller need to be adjusted as well.
This commit message suggested by Dan Carpenter is much better. The previous
one made me think that you were doing several different logical changes.
>
> [snip]
>
> static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct
sk_buff *skb)
> {
> - u16 errcode;
> + int errcode;
> u8 *challenge;
> int chlen = 0;
> bool bSupportNmode = true, bHalfSupportNmode = false;
> @@ -2292,8 +2292,8 @@ static void rtllib_rx_auth_resp(struct
rtllib_device *ieee, struct sk_buff *skb)
> if (errcode) {
> ieee->softmac_stats.rx_auth_rs_err++;
> netdev_info(ieee->dev,
> - "Authentication response status code
0x%x",
> - errcode);
> + "Authentication response status code %d",
> + le16_to_cpu(errcode));
This is something that I'm still missing. Why do we need that call to
le16_to_cpu on "errcode"?
"errcode" is returned by auth_parse()? I see that this function already
changes the endianness of the returned value.
Thanks,
Fabio
> rtllib_associate_abort(ieee);
> return;
> }
> --
> 2.7.4
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V3] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 6:06 ` Fabio M. De Francesco
@ 2022-04-15 6:10 ` Fabio M. De Francesco
2022-04-15 6:15 ` [PATCH V4] " Haowen Bai
0 siblings, 1 reply; 21+ messages in thread
From: Fabio M. De Francesco @ 2022-04-15 6:10 UTC (permalink / raw)
To: Greg Kroah-Hartman, Haowen Bai; +Cc: Haowen Bai, linux-staging, linux-kernel
On venerdì 15 aprile 2022 08:06:17 CEST Fabio M. De Francesco wrote:
> On venerdì 15 aprile 2022 07:50:36 CEST Haowen Bai wrote:
> > The rtllib_rx_assoc_resp() function has a signedness bug because it's
> > a declared as a u16 but it return -ENOMEM. When you look at it more
> > closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
> > a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
> >
> > Clean it up to just return standard kernel error codes. We can print
> > out the a->status before returning a regular error code. The printks
> > in the caller need to be adjusted as well.
>
> This commit message suggested by Dan Carpenter is much better. The
previous
> one made me think that you were doing several different logical changes.
>
> >
> > [snip]
> >
> > static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct
> sk_buff *skb)
> > {
> > - u16 errcode;
> > + int errcode;
> > u8 *challenge;
> > int chlen = 0;
> > bool bSupportNmode = true, bHalfSupportNmode = false;
> > @@ -2292,8 +2292,8 @@ static void rtllib_rx_auth_resp(struct
> rtllib_device *ieee, struct sk_buff *skb)
> > if (errcode) {
> > ieee->softmac_stats.rx_auth_rs_err++;
> > netdev_info(ieee->dev,
> > - "Authentication response status code
> 0x%x",
> > - errcode);
> > + "Authentication response status code %d",
> > + le16_to_cpu(errcode));
>
> This is something that I'm still missing. Why do we need that call to
> le16_to_cpu on "errcode"?
>
> "errcode" is returned by auth_parse()? I see that this function already
> changes the endianness of the returned value.
Sorry, I missed that you also changed auth_code().
Fabio
>
> Thanks,
>
> Fabio
>
>
> > rtllib_associate_abort(ieee);
> > return;
> > }
> > --
> > 2.7.4
> >
> >
> >
>
>
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V4] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 6:10 ` Fabio M. De Francesco
@ 2022-04-15 6:15 ` Haowen Bai
2022-04-15 6:20 ` Dan Carpenter
0 siblings, 1 reply; 21+ messages in thread
From: Haowen Bai @ 2022-04-15 6:15 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Haowen Bai, linux-staging, linux-kernel
This commit message suggested by Dan Carpenter as below:
The rtllib_rx_assoc_resp() function has a signedness bug because it's
a declared as a u16 but it return -ENOMEM. When you look at it more
closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
Clean it up to just return standard kernel error codes. We can print
out the a->status before returning a regular error code. The printks
in the caller need to be adjusted as well.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
V1->V2: reduce return random value; print its own error message.
V2->V3: change commit message; change s16 -> int.
V3->V4: add message suggested by in title.
drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index 82bf05eb1cbf..4a1b9a94930f 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
spin_unlock_irqrestore(&ieee->lock, flags);
}
-static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
+static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
u8 **challenge, int *chlen)
{
struct rtllib_authentication *a;
@@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
if (skb->len < (sizeof(struct rtllib_authentication) -
sizeof(struct rtllib_info_element))) {
netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
- return 0xcafe;
+ return -EINVAL;
}
*challenge = NULL;
a = (struct rtllib_authentication *) skb->data;
@@ -1787,7 +1787,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
return -ENOMEM;
}
}
- return le16_to_cpu(a->status);
+ return a->status;
}
static int auth_rq_parse(struct net_device *dev, struct sk_buff *skb, u8 *dest)
@@ -2282,7 +2282,7 @@ rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb,
static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
{
- u16 errcode;
+ int errcode;
u8 *challenge;
int chlen = 0;
bool bSupportNmode = true, bHalfSupportNmode = false;
@@ -2292,8 +2292,8 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
if (errcode) {
ieee->softmac_stats.rx_auth_rs_err++;
netdev_info(ieee->dev,
- "Authentication response status code 0x%x",
- errcode);
+ "Authentication response status code %d",
+ le16_to_cpu(errcode));
rtllib_associate_abort(ieee);
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V3] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 5:50 ` [PATCH V3] " Haowen Bai
2022-04-15 6:06 ` Fabio M. De Francesco
@ 2022-04-15 6:18 ` Dan Carpenter
1 sibling, 0 replies; 21+ messages in thread
From: Dan Carpenter @ 2022-04-15 6:18 UTC (permalink / raw)
To: Haowen Bai; +Cc: Greg Kroah-Hartman, linux-staging, linux-kernel
On Fri, Apr 15, 2022 at 01:50:36PM +0800, Haowen Bai wrote:
> The rtllib_rx_assoc_resp() function has a signedness bug because it's
> a declared as a u16 but it return -ENOMEM. When you look at it more
> closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
> a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
>
> Clean it up to just return standard kernel error codes. We can print
> out the a->status before returning a regular error code. The printks
> in the caller need to be adjusted as well.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
> V1->V2: reduce return random value; print its own error message.
> V2->V3: change commit message; change s16 -> int.
>
> drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
> index 82bf05eb1cbf..4a1b9a94930f 100644
> --- a/drivers/staging/rtl8192e/rtllib_softmac.c
> +++ b/drivers/staging/rtl8192e/rtllib_softmac.c
> @@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
> spin_unlock_irqrestore(&ieee->lock, flags);
> }
>
> -static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> +static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
> u8 **challenge, int *chlen)
> {
> struct rtllib_authentication *a;
> @@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> if (skb->len < (sizeof(struct rtllib_authentication) -
> sizeof(struct rtllib_info_element))) {
> netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
> - return 0xcafe;
> + return -EINVAL;
> }
> *challenge = NULL;
> a = (struct rtllib_authentication *) skb->data;
> @@ -1787,7 +1787,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> return -ENOMEM;
> }
> }
> - return le16_to_cpu(a->status);
> + return a->status;
Please do not return a->status. See my previous email. You're sending
new versions too quickly. Wait a day between new versions.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V4] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 6:15 ` [PATCH V4] " Haowen Bai
@ 2022-04-15 6:20 ` Dan Carpenter
2022-04-15 6:39 ` Haowen Bai
0 siblings, 1 reply; 21+ messages in thread
From: Dan Carpenter @ 2022-04-15 6:20 UTC (permalink / raw)
To: Haowen Bai; +Cc: Greg Kroah-Hartman, linux-staging, linux-kernel
On Fri, Apr 15, 2022 at 02:15:25PM +0800, Haowen Bai wrote:
> This commit message suggested by Dan Carpenter as below:
>
> The rtllib_rx_assoc_resp() function has a signedness bug because it's
> a declared as a u16 but it return -ENOMEM. When you look at it more
> closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
> a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
>
> Clean it up to just return standard kernel error codes. We can print
> out the a->status before returning a regular error code. The printks
> in the caller need to be adjusted as well.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
> V1->V2: reduce return random value; print its own error message.
> V2->V3: change commit message; change s16 -> int.
> V3->V4: add message suggested by in title.
>
> drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
> index 82bf05eb1cbf..4a1b9a94930f 100644
> --- a/drivers/staging/rtl8192e/rtllib_softmac.c
> +++ b/drivers/staging/rtl8192e/rtllib_softmac.c
> @@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
> spin_unlock_irqrestore(&ieee->lock, flags);
> }
>
> -static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> +static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
> u8 **challenge, int *chlen)
> {
> struct rtllib_authentication *a;
> @@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> if (skb->len < (sizeof(struct rtllib_authentication) -
> sizeof(struct rtllib_info_element))) {
> netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
> - return 0xcafe;
> + return -EINVAL;
> }
> *challenge = NULL;
> a = (struct rtllib_authentication *) skb->data;
> @@ -1787,7 +1787,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> return -ENOMEM;
> }
> }
> - return le16_to_cpu(a->status);
> + return a->status;
See previous responses.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V4] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 6:20 ` Dan Carpenter
@ 2022-04-15 6:39 ` Haowen Bai
2022-04-15 6:58 ` Dan Carpenter
0 siblings, 1 reply; 21+ messages in thread
From: Haowen Bai @ 2022-04-15 6:39 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Haowen Bai, linux-staging, linux-kernel
This commit message suggested by Dan Carpenter as below:
The rtllib_rx_assoc_resp() function has a signedness bug because it's
a declared as a u16 but it return -ENOMEM. When you look at it more
closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
Clean it up to just return standard kernel error codes. We can print
out the a->status before returning a regular error code. The printks
in the caller need to be adjusted as well.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
V1->V2: reduce return random value; print its own error message.
V2->V3: change commit message; change s16 -> int.
V3->V4: add message suggested by in title. If you look up what a->status
is, it can only be WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG which is not worth
preserving really
drivers/staging/rtl8192e/rtllib_softmac.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index 82bf05eb1cbf..ac0d131c1248 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
spin_unlock_irqrestore(&ieee->lock, flags);
}
-static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
+static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
u8 **challenge, int *chlen)
{
struct rtllib_authentication *a;
@@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
if (skb->len < (sizeof(struct rtllib_authentication) -
sizeof(struct rtllib_info_element))) {
netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
- return 0xcafe;
+ return -EINVAL;
}
*challenge = NULL;
a = (struct rtllib_authentication *) skb->data;
@@ -1787,7 +1787,13 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
return -ENOMEM;
}
}
- return le16_to_cpu(a->status);
+
+ if (a->status) {
+ netdev_info(ieee->dev, "auth_parse() failed");
+ return -EINVAL;
+ }
+
+ return 0;
}
static int auth_rq_parse(struct net_device *dev, struct sk_buff *skb, u8 *dest)
@@ -2282,7 +2288,7 @@ rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb,
static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
{
- u16 errcode;
+ int errcode;
u8 *challenge;
int chlen = 0;
bool bSupportNmode = true, bHalfSupportNmode = false;
@@ -2292,8 +2298,8 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
if (errcode) {
ieee->softmac_stats.rx_auth_rs_err++;
netdev_info(ieee->dev,
- "Authentication response status code 0x%x",
- errcode);
+ "Authentication response status code %d",
+ le16_to_cpu(errcode));
rtllib_associate_abort(ieee);
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V4] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 6:39 ` Haowen Bai
@ 2022-04-15 6:58 ` Dan Carpenter
2022-04-15 7:21 ` Haowen Bai
2022-04-18 1:48 ` [PATCH V4] " baihaowen
0 siblings, 2 replies; 21+ messages in thread
From: Dan Carpenter @ 2022-04-15 6:58 UTC (permalink / raw)
To: Haowen Bai; +Cc: Greg Kroah-Hartman, linux-staging, linux-kernel
On Fri, Apr 15, 2022 at 02:39:27PM +0800, Haowen Bai wrote:
> This commit message suggested by Dan Carpenter as below:
You don't need to add this. If you feel you must then put it below the
--- cut off line and it will be stored on lore.kernel.org until the end
of time.
> @@ -2292,8 +2298,8 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
> if (errcode) {
> ieee->softmac_stats.rx_auth_rs_err++;
> netdev_info(ieee->dev,
> - "Authentication response status code 0x%x",
> - errcode);
> + "Authentication response status code %d",
> + le16_to_cpu(errcode));
The error code is not a le16. It's just an int. The way to prevent
these kinds of issues in the future is to run Sparse with the endian
checking enabled.
https://lwn.net/Articles/205624/
regards,
dan carpenter
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V4] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 6:58 ` Dan Carpenter
@ 2022-04-15 7:21 ` Haowen Bai
2022-04-20 16:41 ` Greg Kroah-Hartman
2022-04-18 1:48 ` [PATCH V4] " baihaowen
1 sibling, 1 reply; 21+ messages in thread
From: Haowen Bai @ 2022-04-15 7:21 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Haowen Bai, linux-staging, linux-kernel
The rtllib_rx_assoc_resp() function has a signedness bug because it's
a declared as a u16 but it return -ENOMEM. When you look at it more
closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
Clean it up to just return standard kernel error codes. We can print
out the a->status before returning a regular error code. The printks
in the caller need to be adjusted as well.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
V1->V2: reduce return random value; print its own error message.
V2->V3: change commit message; change s16 -> int.
V3->V4: add message suggested by Dan Carpenter. If you look up what
a->status is, it can only be WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG which
is not worth preserving really.
drivers/staging/rtl8192e/rtllib_softmac.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index 82bf05eb1cbf..38ac733c3245 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
spin_unlock_irqrestore(&ieee->lock, flags);
}
-static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
+static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
u8 **challenge, int *chlen)
{
struct rtllib_authentication *a;
@@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
if (skb->len < (sizeof(struct rtllib_authentication) -
sizeof(struct rtllib_info_element))) {
netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
- return 0xcafe;
+ return -EINVAL;
}
*challenge = NULL;
a = (struct rtllib_authentication *) skb->data;
@@ -1787,7 +1787,13 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
return -ENOMEM;
}
}
- return le16_to_cpu(a->status);
+
+ if (a->status) {
+ netdev_info(ieee->dev, "auth_parse() failed");
+ return -EINVAL;
+ }
+
+ return 0;
}
static int auth_rq_parse(struct net_device *dev, struct sk_buff *skb, u8 *dest)
@@ -2282,7 +2288,7 @@ rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb,
static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
{
- u16 errcode;
+ int errcode;
u8 *challenge;
int chlen = 0;
bool bSupportNmode = true, bHalfSupportNmode = false;
@@ -2292,8 +2298,7 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
if (errcode) {
ieee->softmac_stats.rx_auth_rs_err++;
netdev_info(ieee->dev,
- "Authentication response status code 0x%x",
- errcode);
+ "Authentication response status code %d", errcode);
rtllib_associate_abort(ieee);
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V4] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 6:58 ` Dan Carpenter
2022-04-15 7:21 ` Haowen Bai
@ 2022-04-18 1:48 ` baihaowen
1 sibling, 0 replies; 21+ messages in thread
From: baihaowen @ 2022-04-18 1:48 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Greg Kroah-Hartman, linux-staging, linux-kernel
在 4/15/22 2:58 PM, Dan Carpenter 写道:
> On Fri, Apr 15, 2022 at 02:39:27PM +0800, Haowen Bai wrote:
>> This commit message suggested by Dan Carpenter as below:
> You don't need to add this. If you feel you must then put it below the
> --- cut off line and it will be stored on lore.kernel.org until the end
> of time.
>
>> @@ -2292,8 +2298,8 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
>> if (errcode) {
>> ieee->softmac_stats.rx_auth_rs_err++;
>> netdev_info(ieee->dev,
>> - "Authentication response status code 0x%x",
>> - errcode);
>> + "Authentication response status code %d",
>> + le16_to_cpu(errcode));
> The error code is not a le16. It's just an int. The way to prevent
> these kinds of issues in the future is to run Sparse with the endian
> checking enabled.
>
> https://lwn.net/Articles/205624/
>
> regards,
> dan carpenter
>
Dear Dan Carpenter
Got it, thank you for your professional and patience.
--
Haowen Bai
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V4] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-15 7:21 ` Haowen Bai
@ 2022-04-20 16:41 ` Greg Kroah-Hartman
2022-04-21 1:34 ` Haowen Bai
0 siblings, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2022-04-20 16:41 UTC (permalink / raw)
To: Haowen Bai; +Cc: linux-staging, linux-kernel
On Fri, Apr 15, 2022 at 03:21:35PM +0800, Haowen Bai wrote:
> The rtllib_rx_assoc_resp() function has a signedness bug because it's
> a declared as a u16 but it return -ENOMEM. When you look at it more
> closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
> a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
>
> Clean it up to just return standard kernel error codes. We can print
> out the a->status before returning a regular error code. The printks
> in the caller need to be adjusted as well.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
> V1->V2: reduce return random value; print its own error message.
> V2->V3: change commit message; change s16 -> int.
> V3->V4: add message suggested by Dan Carpenter. If you look up what
> a->status is, it can only be WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG which
> is not worth preserving really.
I see 3 different v4 patches. Obviously something went wrong, please
submit a new one, and properly number it and say what changed between
them all.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V4] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-20 16:41 ` Greg Kroah-Hartman
@ 2022-04-21 1:34 ` Haowen Bai
2022-04-21 8:09 ` kernel test robot
0 siblings, 1 reply; 21+ messages in thread
From: Haowen Bai @ 2022-04-21 1:34 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Haowen Bai, linux-staging, linux-kernel
The rtllib_rx_assoc_resp() function has a signedness bug because it's
a declared as a u16 but it return -ENOMEM. When you look at it more
closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
Clean it up to just return standard kernel error codes. We can print
out the a->status before returning a regular error code. The printks
in the caller need to be adjusted as well.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
V1->V2: reduce return random value; print its own error message.
V2->V3: change commit message; change s16 -> int.
V3->V4:
1. change message suggested by Dan Carpenter;
2. hold a->status in auth_parse() and return error code or 0 on success.
3. print le16_to_cpu(errcode) -> int %d.
drivers/staging/rtl8192e/rtllib_softmac.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index 82bf05eb1cbf..38ac733c3245 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
spin_unlock_irqrestore(&ieee->lock, flags);
}
-static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
+static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
u8 **challenge, int *chlen)
{
struct rtllib_authentication *a;
@@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
if (skb->len < (sizeof(struct rtllib_authentication) -
sizeof(struct rtllib_info_element))) {
netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
- return 0xcafe;
+ return -EINVAL;
}
*challenge = NULL;
a = (struct rtllib_authentication *) skb->data;
@@ -1787,7 +1787,13 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
return -ENOMEM;
}
}
- return le16_to_cpu(a->status);
+
+ if (a->status) {
+ netdev_info(ieee->dev, "auth_parse() failed");
+ return -EINVAL;
+ }
+
+ return 0;
}
static int auth_rq_parse(struct net_device *dev, struct sk_buff *skb, u8 *dest)
@@ -2282,7 +2288,7 @@ rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb,
static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
{
- u16 errcode;
+ int errcode;
u8 *challenge;
int chlen = 0;
bool bSupportNmode = true, bHalfSupportNmode = false;
@@ -2292,8 +2298,7 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
if (errcode) {
ieee->softmac_stats.rx_auth_rs_err++;
netdev_info(ieee->dev,
- "Authentication response status code 0x%x",
- errcode);
+ "Authentication response status code %d", errcode);
rtllib_associate_abort(ieee);
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V4] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-21 1:34 ` Haowen Bai
@ 2022-04-21 8:09 ` kernel test robot
2022-04-21 8:21 ` Haowen Bai
0 siblings, 1 reply; 21+ messages in thread
From: kernel test robot @ 2022-04-21 8:09 UTC (permalink / raw)
To: Haowen Bai, Greg Kroah-Hartman
Cc: kbuild-all, Haowen Bai, linux-staging, linux-kernel
Hi Haowen,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v5.18-rc3]
[also build test ERROR on next-20220421]
[cannot apply to staging/staging-testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Haowen-Bai/staging-rtl8192e-Fix-signedness-bug-in-rtllib_rx_assoc_resp/20220421-093531
base: b2d229d4ddb17db541098b83524d901257e93845
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220421/202204211558.TapLZO4j-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/e1395cdafd4dea7a368f2d9599832636035a03d2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Haowen-Bai/staging-rtl8192e-Fix-signedness-bug-in-rtllib_rx_assoc_resp/20220421-093531
git checkout e1395cdafd4dea7a368f2d9599832636035a03d2
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/staging/rtl8192e/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/staging/rtl8192e/rtllib_softmac.c: In function 'auth_parse':
>> drivers/staging/rtl8192e/rtllib_softmac.c:1792:29: error: 'ieee' undeclared (first use in this function)
1792 | netdev_info(ieee->dev, "auth_parse() failed");
| ^~~~
drivers/staging/rtl8192e/rtllib_softmac.c:1792:29: note: each undeclared identifier is reported only once for each function it appears in
vim +/ieee +1792 drivers/staging/rtl8192e/rtllib_softmac.c
1766
1767 static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
1768 u8 **challenge, int *chlen)
1769 {
1770 struct rtllib_authentication *a;
1771 u8 *t;
1772
1773 if (skb->len < (sizeof(struct rtllib_authentication) -
1774 sizeof(struct rtllib_info_element))) {
1775 netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
1776 return -EINVAL;
1777 }
1778 *challenge = NULL;
1779 a = (struct rtllib_authentication *) skb->data;
1780 if (skb->len > (sizeof(struct rtllib_authentication) + 3)) {
1781 t = skb->data + sizeof(struct rtllib_authentication);
1782
1783 if (*(t++) == MFIE_TYPE_CHALLENGE) {
1784 *chlen = *(t++);
1785 *challenge = kmemdup(t, *chlen, GFP_ATOMIC);
1786 if (!*challenge)
1787 return -ENOMEM;
1788 }
1789 }
1790
1791 if (a->status) {
> 1792 netdev_info(ieee->dev, "auth_parse() failed");
1793 return -EINVAL;
1794 }
1795
1796 return 0;
1797 }
1798
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V5] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-21 8:09 ` kernel test robot
@ 2022-04-21 8:21 ` Haowen Bai
0 siblings, 0 replies; 21+ messages in thread
From: Haowen Bai @ 2022-04-21 8:21 UTC (permalink / raw)
To: lkp; +Cc: baihaowen, gregkh, kbuild-all, linux-kernel, linux-staging
The rtllib_rx_assoc_resp() function has a signedness bug because it's
a declared as a u16 but it return -ENOMEM. When you look at it more
closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
Clean it up to just return standard kernel error codes. We can print
out the a->status before returning a regular error code. The printks
in the caller need to be adjusted as well.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
V1->V2: reduce return random value; print its own error message.
V2->V3: change commit message; change s16 -> int.
V3->V4:
1. change message suggested by Dan Carpenter;
2. hold a->status in auth_parse() and return error code or 0 on success.
3. print le16_to_cpu(errcode) -> int %d.
V4->V5: fix compile error.
drivers/staging/rtl8192e/rtllib_softmac.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index 82bf05eb1cbf..38ac733c3245 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
spin_unlock_irqrestore(&ieee->lock, flags);
}
-static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
+static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
u8 **challenge, int *chlen)
{
struct rtllib_authentication *a;
@@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
if (skb->len < (sizeof(struct rtllib_authentication) -
sizeof(struct rtllib_info_element))) {
netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
- return 0xcafe;
+ return -EINVAL;
}
*challenge = NULL;
a = (struct rtllib_authentication *) skb->data;
@@ -1787,7 +1787,13 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
return -ENOMEM;
}
}
- return le16_to_cpu(a->status);
+
+ if (a->status) {
+ netdev_dbg(dev, "auth_parse() failed\n");
+ return -EINVAL;
+ }
+
+ return 0;
}
static int auth_rq_parse(struct net_device *dev, struct sk_buff *skb, u8 *dest)
@@ -2282,7 +2288,7 @@ rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb,
static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
{
- u16 errcode;
+ int errcode;
u8 *challenge;
int chlen = 0;
bool bSupportNmode = true, bHalfSupportNmode = false;
@@ -2292,8 +2298,7 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
if (errcode) {
ieee->softmac_stats.rx_auth_rs_err++;
netdev_info(ieee->dev,
- "Authentication response status code 0x%x",
- errcode);
+ "Authentication response status code %d", errcode);
rtllib_associate_abort(ieee);
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH V5] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
@ 2022-04-21 8:21 ` Haowen Bai
0 siblings, 0 replies; 21+ messages in thread
From: Haowen Bai @ 2022-04-21 8:21 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3029 bytes --]
The rtllib_rx_assoc_resp() function has a signedness bug because it's
a declared as a u16 but it return -ENOMEM. When you look at it more
closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
Clean it up to just return standard kernel error codes. We can print
out the a->status before returning a regular error code. The printks
in the caller need to be adjusted as well.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
V1->V2: reduce return random value; print its own error message.
V2->V3: change commit message; change s16 -> int.
V3->V4:
1. change message suggested by Dan Carpenter;
2. hold a->status in auth_parse() and return error code or 0 on success.
3. print le16_to_cpu(errcode) -> int %d.
V4->V5: fix compile error.
drivers/staging/rtl8192e/rtllib_softmac.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index 82bf05eb1cbf..38ac733c3245 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
spin_unlock_irqrestore(&ieee->lock, flags);
}
-static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
+static inline int auth_parse(struct net_device *dev, struct sk_buff *skb,
u8 **challenge, int *chlen)
{
struct rtllib_authentication *a;
@@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
if (skb->len < (sizeof(struct rtllib_authentication) -
sizeof(struct rtllib_info_element))) {
netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
- return 0xcafe;
+ return -EINVAL;
}
*challenge = NULL;
a = (struct rtllib_authentication *) skb->data;
@@ -1787,7 +1787,13 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
return -ENOMEM;
}
}
- return le16_to_cpu(a->status);
+
+ if (a->status) {
+ netdev_dbg(dev, "auth_parse() failed\n");
+ return -EINVAL;
+ }
+
+ return 0;
}
static int auth_rq_parse(struct net_device *dev, struct sk_buff *skb, u8 *dest)
@@ -2282,7 +2288,7 @@ rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb,
static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
{
- u16 errcode;
+ int errcode;
u8 *challenge;
int chlen = 0;
bool bSupportNmode = true, bHalfSupportNmode = false;
@@ -2292,8 +2298,7 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
if (errcode) {
ieee->softmac_stats.rx_auth_rs_err++;
netdev_info(ieee->dev,
- "Authentication response status code 0x%x",
- errcode);
+ "Authentication response status code %d", errcode);
rtllib_associate_abort(ieee);
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V5] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
2022-04-21 8:21 ` Haowen Bai
@ 2022-04-21 16:22 ` Dan Carpenter
-1 siblings, 0 replies; 21+ messages in thread
From: Dan Carpenter @ 2022-04-21 16:22 UTC (permalink / raw)
To: Haowen Bai; +Cc: lkp, gregkh, kbuild-all, linux-kernel, linux-staging
On Thu, Apr 21, 2022 at 04:21:17PM +0800, Haowen Bai wrote:
> The rtllib_rx_assoc_resp() function has a signedness bug because it's
> a declared as a u16 but it return -ENOMEM. When you look at it more
> closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
> a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
>
> Clean it up to just return standard kernel error codes. We can print
> out the a->status before returning a regular error code. The printks
> in the caller need to be adjusted as well.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
> V1->V2: reduce return random value; print its own error message.
> V2->V3: change commit message; change s16 -> int.
> V3->V4:
> 1. change message suggested by Dan Carpenter;
> 2. hold a->status in auth_parse() and return error code or 0 on success.
> 3. print le16_to_cpu(errcode) -> int %d.
> V4->V5: fix compile error.
>
Looks ok.
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V5] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp()
@ 2022-04-21 16:22 ` Dan Carpenter
0 siblings, 0 replies; 21+ messages in thread
From: Dan Carpenter @ 2022-04-21 16:22 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]
On Thu, Apr 21, 2022 at 04:21:17PM +0800, Haowen Bai wrote:
> The rtllib_rx_assoc_resp() function has a signedness bug because it's
> a declared as a u16 but it return -ENOMEM. When you look at it more
> closely it returns a mix of error codes including 0xcafe, -ENOMEM, and
> a->status which is WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG. This is a mess.
>
> Clean it up to just return standard kernel error codes. We can print
> out the a->status before returning a regular error code. The printks
> in the caller need to be adjusted as well.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
> V1->V2: reduce return random value; print its own error message.
> V2->V3: change commit message; change s16 -> int.
> V3->V4:
> 1. change message suggested by Dan Carpenter;
> 2. hold a->status in auth_parse() and return error code or 0 on success.
> 3. print le16_to_cpu(errcode) -> int %d.
> V4->V5: fix compile error.
>
Looks ok.
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-04-21 16:22 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-15 2:09 [PATCH V2] staging: rtl8192e: Fix signedness bug in rtllib_rx_assoc_resp() Haowen Bai
2022-04-15 5:22 ` Fabio M. De Francesco
2022-04-15 5:31 ` Dan Carpenter
2022-04-15 5:50 ` [PATCH V3] " Haowen Bai
2022-04-15 6:06 ` Fabio M. De Francesco
2022-04-15 6:10 ` Fabio M. De Francesco
2022-04-15 6:15 ` [PATCH V4] " Haowen Bai
2022-04-15 6:20 ` Dan Carpenter
2022-04-15 6:39 ` Haowen Bai
2022-04-15 6:58 ` Dan Carpenter
2022-04-15 7:21 ` Haowen Bai
2022-04-20 16:41 ` Greg Kroah-Hartman
2022-04-21 1:34 ` Haowen Bai
2022-04-21 8:09 ` kernel test robot
2022-04-21 8:21 ` [PATCH V5] " Haowen Bai
2022-04-21 8:21 ` Haowen Bai
2022-04-21 16:22 ` Dan Carpenter
2022-04-21 16:22 ` Dan Carpenter
2022-04-18 1:48 ` [PATCH V4] " baihaowen
2022-04-15 6:18 ` [PATCH V3] " Dan Carpenter
2022-04-15 5:25 ` [PATCH V2] " Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.