drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
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.4On 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
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
>
>
>
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
> >
> >
> >
>
>
>
>
>
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.4On 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
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.4On 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
在 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
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.4On 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
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.4Hi 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
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.4On 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
© 2016 - 2026 Red Hat, Inc.