* [PATCH] staging: rtl8723au: Mark type casts to __le32 as intentional @ 2015-09-22 7:24 Lars Svensson 2015-09-22 21:30 ` Larry Finger 0 siblings, 1 reply; 5+ messages in thread From: Lars Svensson @ 2015-09-22 7:24 UTC (permalink / raw) To: Larry.Finger Cc: Jes.Sorensen, gregkh, m.v.b, gdonald, joe, ruchandani.tina, linux-wireless, devel, linux-kernel, Lars Svensson, Lars Svensson Fixing Sparse warnings about intentional type casts in rtw_security.c as below. CHECK drivers/staging/rtl8723au/core/rtw_security.c drivers/staging/rtl8723au/core/rtw_security.c:248:22: \ warning: cast to restricted __le32 drivers/staging/rtl8723au/core/rtw_security.c:249:24: \ warning: cast to restricted __le32 drivers/staging/rtl8723au/core/rtw_security.c:776:22: \ warning: cast to restricted __le32 drivers/staging/rtl8723au/core/rtw_security.c:777:24: \ warning: cast to restricted __le32 Signed-off-by: Lars Svensson <Lars1.Svensson@sonymobile.com> --- drivers/staging/rtl8723au/core/rtw_security.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c index 3d40bab..311dfc1 100644 --- a/drivers/staging/rtl8723au/core/rtw_security.c +++ b/drivers/staging/rtl8723au/core/rtw_security.c @@ -213,6 +213,7 @@ void rtw_wep_decrypt23a(struct rtw_adapter *padapter, { /* exclude ICV */ u32 actual_crc, expected_crc; + __le32 crc_le; struct arc4context mycontext; int length; u32 keylength; @@ -245,8 +246,10 @@ void rtw_wep_decrypt23a(struct rtw_adapter *padapter, arcfour_encrypt(&mycontext, payload, payload, length); /* calculate icv and compare the icv */ - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); + crc_le = (__force __le32)getcrc32(payload, length - 4); + actual_crc = le32_to_cpu(crc_le); + crc_le = (__force __le32)get_unaligned_le32(&payload[length - 4]); + expected_crc = le32_to_cpu(crc_le); if (actual_crc != expected_crc) { RT_TRACE(_module_rtl871x_security_c_, _drv_err_, @@ -717,6 +720,7 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, u8 rc4key[16]; u8 ttkey[16]; u32 actual_crc, expected_crc; + __le32 crc_le; struct arc4context mycontext; int length; u32 prwskeylen; @@ -772,9 +776,10 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, /* 4 decrypt payload include icv */ arcfour_init(&mycontext, rc4key, 16); arcfour_encrypt(&mycontext, payload, payload, length); - - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); + crc_le = (__force __le32)getcrc32(payload, length - 4); + actual_crc = le32_to_cpu(crc_le); + crc_le = (__force __le32)get_unaligned_le32(&payload[length - 4]); + expected_crc = le32_to_cpu(crc_le); if (actual_crc != expected_crc) { RT_TRACE(_module_rtl871x_security_c_, _drv_err_, -- 2.4.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: rtl8723au: Mark type casts to __le32 as intentional 2015-09-22 7:24 [PATCH] staging: rtl8723au: Mark type casts to __le32 as intentional Lars Svensson @ 2015-09-22 21:30 ` Larry Finger 2015-09-23 10:11 ` Lars Svensson 0 siblings, 1 reply; 5+ messages in thread From: Larry Finger @ 2015-09-22 21:30 UTC (permalink / raw) To: Lars Svensson Cc: Jes.Sorensen, gregkh, m.v.b, gdonald, joe, ruchandani.tina, linux-wireless, devel, linux-kernel On 09/22/2015 02:24 AM, Lars Svensson wrote: > Fixing Sparse warnings about intentional type casts in rtw_security.c > as below. > > CHECK drivers/staging/rtl8723au/core/rtw_security.c > drivers/staging/rtl8723au/core/rtw_security.c:248:22: \ > warning: cast to restricted __le32 > drivers/staging/rtl8723au/core/rtw_security.c:249:24: \ > warning: cast to restricted __le32 > drivers/staging/rtl8723au/core/rtw_security.c:776:22: \ > warning: cast to restricted __le32 > drivers/staging/rtl8723au/core/rtw_security.c:777:24: \ > warning: cast to restricted __le32 > > Signed-off-by: Lars Svensson <Lars1.Svensson@sonymobile.com> > --- > drivers/staging/rtl8723au/core/rtw_security.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c > index 3d40bab..311dfc1 100644 > --- a/drivers/staging/rtl8723au/core/rtw_security.c > +++ b/drivers/staging/rtl8723au/core/rtw_security.c > @@ -213,6 +213,7 @@ void rtw_wep_decrypt23a(struct rtw_adapter *padapter, > { > /* exclude ICV */ > u32 actual_crc, expected_crc; > + __le32 crc_le; > struct arc4context mycontext; > int length; > u32 keylength; > @@ -245,8 +246,10 @@ void rtw_wep_decrypt23a(struct rtw_adapter *padapter, > arcfour_encrypt(&mycontext, payload, payload, length); > > /* calculate icv and compare the icv */ > - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); > - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); > + crc_le = (__force __le32)getcrc32(payload, length - 4); > + actual_crc = le32_to_cpu(crc_le); > + crc_le = (__force __le32)get_unaligned_le32(&payload[length - 4]); > + expected_crc = le32_to_cpu(crc_le); > > if (actual_crc != expected_crc) { > RT_TRACE(_module_rtl871x_security_c_, _drv_err_, > @@ -717,6 +720,7 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, > u8 rc4key[16]; > u8 ttkey[16]; > u32 actual_crc, expected_crc; > + __le32 crc_le; > struct arc4context mycontext; > int length; > u32 prwskeylen; > @@ -772,9 +776,10 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, > /* 4 decrypt payload include icv */ > arcfour_init(&mycontext, rc4key, 16); > arcfour_encrypt(&mycontext, payload, payload, length); > - > - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); > - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); > + crc_le = (__force __le32)getcrc32(payload, length - 4); > + actual_crc = le32_to_cpu(crc_le); > + crc_le = (__force __le32)get_unaligned_le32(&payload[length - 4]); > + expected_crc = le32_to_cpu(crc_le); > > if (actual_crc != expected_crc) { > RT_TRACE(_module_rtl871x_security_c_, _drv_err_, > You may have silenced the Sparse warnings, but the code was not wrong. Your version is also correct; however, you end up with some really ugly casts. Here is my analysis of these two, identical sections: The output of getcrc32() is an unsigned, 4-byte quantity that has the endianess of the cpu. Therefore, the le32_to_cpu() conversion is suspect for a big-endian machine. Those statements should be be a simple "actual_crc = getcrc32(....). The expected crc comes from a byte string that is in little-endian order. For that reason, it needs to be converted on big-endian machines, which is exactly what get_unaligned_le32() does. Thus, the second statement in each block becomes "expected_crc = get_unaligned_le32(....)". Both the original code and your patch byte swap both quantities, thus they get the correct result. at least if all you are doing is to compare the two results. The *complete* patch to fix the Sparse warnings is Index: wireless-drivers-next-new/drivers/staging/rtl8723au/core/rtw_security.c =================================================================== --- wireless-drivers-next-new.orig/drivers/staging/rtl8723au/core/rtw_security.c +++ wireless-drivers-next-new/drivers/staging/rtl8723au/core/rtw_security.c @@ -245,8 +245,8 @@ void rtw_wep_decrypt23a(struct rtw_adapt arcfour_encrypt(&mycontext, payload, payload, length); /* calculate icv and compare the icv */ - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); + actual_crc = getcrc32(payload, length - 4); + expected_crc = get_unaligned_le32(&payload[length - 4]); if (actual_crc != expected_crc) { RT_TRACE(_module_rtl871x_security_c_, _drv_err_, @@ -773,8 +773,8 @@ int rtw_tkip_decrypt23a(struct rtw_adapt arcfour_init(&mycontext, rc4key, 16); arcfour_encrypt(&mycontext, payload, payload, length); - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); + actual_crc = getcrc32(payload, length - 4); + expected_crc = get_unaligned_le32(&payload[length - 4]); if (actual_crc != expected_crc) { RT_TRACE(_module_rtl871x_security_c_, _drv_err_, The above compiles with no Sparse warnings, and I think it would work on both LE and BE architectures; however, it has only been compile tested. Larry ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: rtl8723au: Mark type casts to __le32 as intentional 2015-09-22 21:30 ` Larry Finger @ 2015-09-23 10:11 ` Lars Svensson 2015-09-24 7:11 ` [PATCH V2] staging: rtl8723au: Remove unneeded endianness conversions Lars Svensson 0 siblings, 1 reply; 5+ messages in thread From: Lars Svensson @ 2015-09-23 10:11 UTC (permalink / raw) To: Larry Finger Cc: Jes.Sorensen@redhat.com, gregkh@linuxfoundation.org, m.v.b@runbox.com, gdonald@gmail.com, joe@perches.com, ruchandani.tina@gmail.com, linux-wireless@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org On Tue, Sep 22, 2015 at 11:30:54PM +0200, Larry Finger wrote: > > You may have silenced the Sparse warnings, but the code was not wrong. Your > version is also correct; however, you end up with some really ugly casts. > > Here is my analysis of these two, identical sections: > > The output of getcrc32() is an unsigned, 4-byte quantity that has the endianess > of the cpu. Therefore, the le32_to_cpu() conversion is suspect for a big-endian > machine. Those statements should be be a simple "actual_crc = getcrc32(....). > > The expected crc comes from a byte string that is in little-endian order. For > that reason, it needs to be converted on big-endian machines, which is exactly > what get_unaligned_le32() does. Thus, the second statement in each block becomes > "expected_crc = get_unaligned_le32(....)". > > Both the original code and your patch byte swap both quantities, thus they get > the correct result. at least if all you are doing is to compare the two results. I agree, it seems obvious when You spell it out like this. I was more focused on producing an identical result while removing the sparse warnings. Your suggestion is much nicer, I will change the patch accordingly. > > The above compiles with no Sparse warnings, and I think it would work on both LE > and BE architectures; however, it has only been compile tested. I have no big endian machine to try it on, but it seems reasonable to assume that at least the compare will work. The printk in case of a mismatch im not sure about, though. Assuming the analysis is correct, I think the printed values would be wrong on a BE machine before this change? Thanks, //Lars ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V2] staging: rtl8723au: Remove unneeded endianness conversions 2015-09-23 10:11 ` Lars Svensson @ 2015-09-24 7:11 ` Lars Svensson 2015-09-26 17:54 ` Larry Finger 0 siblings, 1 reply; 5+ messages in thread From: Lars Svensson @ 2015-09-24 7:11 UTC (permalink / raw) To: Larry.Finger Cc: Jes.Sorensen, gregkh, m.v.b, gdonald, joe, ruchandani.tina, linux-wireless, devel, linux-kernel, Lars Svensson, Lars Svensson Fixing Sparse warnings in rtw_security.c. When checking crc, both actual and expected value was converted to cpu endianness before comparing, causing sparse warnings as below. Since the values are read from the buffer in correct byte order the extra conversions should not be needed. Thanks to Larry Finger for help sorting this out. CHECK drivers/staging/rtl8723au/core/rtw_security.c drivers/staging/rtl8723au/core/rtw_security.c:248:22: \ warning: cast to restricted __le32 drivers/staging/rtl8723au/core/rtw_security.c:249:24: \ warning: cast to restricted __le32 drivers/staging/rtl8723au/core/rtw_security.c:776:22: \ warning: cast to restricted __le32 drivers/staging/rtl8723au/core/rtw_security.c:777:24: \ warning: cast to restricted __le32 Signed-off-by: Lars Svensson <Lars1.Svensson@sonymobile.com> --- Patch V2: Reworked as adviced. --- drivers/staging/rtl8723au/core/rtw_security.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c index 3d40bab..a44c606 100644 --- a/drivers/staging/rtl8723au/core/rtw_security.c +++ b/drivers/staging/rtl8723au/core/rtw_security.c @@ -245,8 +245,8 @@ void rtw_wep_decrypt23a(struct rtw_adapter *padapter, arcfour_encrypt(&mycontext, payload, payload, length); /* calculate icv and compare the icv */ - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); + actual_crc = getcrc32(payload, length - 4); + expected_crc = get_unaligned_le32(&payload[length - 4]); if (actual_crc != expected_crc) { RT_TRACE(_module_rtl871x_security_c_, _drv_err_, @@ -772,9 +772,8 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, /* 4 decrypt payload include icv */ arcfour_init(&mycontext, rc4key, 16); arcfour_encrypt(&mycontext, payload, payload, length); - - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); + actual_crc = getcrc32(payload, length - 4); + expected_crc = get_unaligned_le32(&payload[length - 4]); if (actual_crc != expected_crc) { RT_TRACE(_module_rtl871x_security_c_, _drv_err_, -- 2.4.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V2] staging: rtl8723au: Remove unneeded endianness conversions 2015-09-24 7:11 ` [PATCH V2] staging: rtl8723au: Remove unneeded endianness conversions Lars Svensson @ 2015-09-26 17:54 ` Larry Finger 0 siblings, 0 replies; 5+ messages in thread From: Larry Finger @ 2015-09-26 17:54 UTC (permalink / raw) To: Lars Svensson Cc: Jes.Sorensen, gregkh, m.v.b, gdonald, joe, ruchandani.tina, linux-wireless, devel, linux-kernel On 09/24/2015 02:11 AM, Lars Svensson wrote: > Fixing Sparse warnings in rtw_security.c. When checking crc, both > actual and expected value was converted to cpu endianness before > comparing, causing sparse warnings as below. Since the values are > read from the buffer in correct byte order the extra conversions > should not be needed. > > Thanks to Larry Finger for help sorting this out. > > CHECK drivers/staging/rtl8723au/core/rtw_security.c > drivers/staging/rtl8723au/core/rtw_security.c:248:22: \ > warning: cast to restricted __le32 > drivers/staging/rtl8723au/core/rtw_security.c:249:24: \ > warning: cast to restricted __le32 > drivers/staging/rtl8723au/core/rtw_security.c:776:22: \ > warning: cast to restricted __le32 > drivers/staging/rtl8723au/core/rtw_security.c:777:24: \ > warning: cast to restricted __le32 > > Signed-off-by: Lars Svensson <Lars1.Svensson@sonymobile.com> > --- > Patch V2: Reworked as adviced. Acked-by: Larry Finger <Larry.Finger@lwfinger.net> Larry > --- > drivers/staging/rtl8723au/core/rtw_security.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c > index 3d40bab..a44c606 100644 > --- a/drivers/staging/rtl8723au/core/rtw_security.c > +++ b/drivers/staging/rtl8723au/core/rtw_security.c > @@ -245,8 +245,8 @@ void rtw_wep_decrypt23a(struct rtw_adapter *padapter, > arcfour_encrypt(&mycontext, payload, payload, length); > > /* calculate icv and compare the icv */ > - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); > - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); > + actual_crc = getcrc32(payload, length - 4); > + expected_crc = get_unaligned_le32(&payload[length - 4]); > > if (actual_crc != expected_crc) { > RT_TRACE(_module_rtl871x_security_c_, _drv_err_, > @@ -772,9 +772,8 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, > /* 4 decrypt payload include icv */ > arcfour_init(&mycontext, rc4key, 16); > arcfour_encrypt(&mycontext, payload, payload, length); > - > - actual_crc = le32_to_cpu(getcrc32(payload, length - 4)); > - expected_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); > + actual_crc = getcrc32(payload, length - 4); > + expected_crc = get_unaligned_le32(&payload[length - 4]); > > if (actual_crc != expected_crc) { > RT_TRACE(_module_rtl871x_security_c_, _drv_err_, > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-09-26 17:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-22 7:24 [PATCH] staging: rtl8723au: Mark type casts to __le32 as intentional Lars Svensson 2015-09-22 21:30 ` Larry Finger 2015-09-23 10:11 ` Lars Svensson 2015-09-24 7:11 ` [PATCH V2] staging: rtl8723au: Remove unneeded endianness conversions Lars Svensson 2015-09-26 17:54 ` Larry Finger
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).