* [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
@ 2021-07-01 13:38 Fabio M. De Francesco
2021-07-01 14:52 ` David Laight
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-07-01 13:38 UTC (permalink / raw)
To: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel
Cc: Fabio M. De Francesco
Use crc32_le in place of the custom getcrc32. This change makes GCC
to warn about incorrect castings to the restricted type __le32, but
they can be safely ignored because crc32_le calculates bitwise
little-endian Ethernet AUTODIN II CRC32.
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/rtl8188eu/core/rtw_security.c | 22 ++++---------------
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c b/drivers/staging/rtl8188eu/core/rtw_security.c
index 1b2cb6196463..5f010cb66970 100644
--- a/drivers/staging/rtl8188eu/core/rtw_security.c
+++ b/drivers/staging/rtl8188eu/core/rtw_security.c
@@ -111,21 +111,6 @@ static void crc32_init(void)
bcrc32initialized = 1;
}
-static __le32 getcrc32(u8 *buf, int len)
-{
- u8 *p;
- u32 crc;
-
- if (bcrc32initialized == 0)
- crc32_init();
-
- crc = 0xffffffff; /* preload shift register, per CRC-32 spec */
-
- for (p = buf; len > 0; ++p, --len)
- crc = crc32_table[(crc ^ *p) & 0xff] ^ (crc >> 8);
- return cpu_to_le32(~crc); /* transmit complement, per CRC-32 spec */
-}
-
/* Need to consider the fragment situation */
void rtw_wep_encrypt(struct adapter *padapter, struct xmit_frame *pxmitframe)
{
@@ -609,14 +594,15 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, struct xmit_frame *pxmitframe)
if ((curfragnum + 1) == pattrib->nr_frags) { /* 4 the last fragment */
length = pattrib->last_txcmdsz - pattrib->hdrlen - pattrib->iv_len - pattrib->icv_len;
- *((__le32 *)crc) = getcrc32(payload, length);/* modified by Amy*/
+ *((__le32 *)crc) = ~crc32_le(~0, payload, length);
arcfour_init(&mycontext, rc4key, 16);
arcfour_encrypt(&mycontext, payload, payload, length);
arcfour_encrypt(&mycontext, payload + length, crc, 4);
} else {
length = pxmitpriv->frag_len - pattrib->hdrlen - pattrib->iv_len - pattrib->icv_len;
- *((__le32 *)crc) = getcrc32(payload, length);/* modified by Amy*/
+ *((__le32 *)crc) = ~crc32_le(~0, payload, length);
+
arcfour_init(&mycontext, rc4key, 16);
arcfour_encrypt(&mycontext, payload, payload, length);
arcfour_encrypt(&mycontext, payload + length, crc, 4);
@@ -682,7 +668,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, struct recv_frame *precvframe)
arcfour_init(&mycontext, rc4key, 16);
arcfour_encrypt(&mycontext, payload, payload, length);
- *((__le32 *)crc) = getcrc32(payload, length - 4);
+ *((__le32 *)crc) = ~crc32_le(~0, payload, length - 4);
if (crc[3] != payload[length - 1] ||
crc[2] != payload[length - 2] ||
--
2.32.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* RE: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
2021-07-01 13:38 [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le() Fabio M. De Francesco
@ 2021-07-01 14:52 ` David Laight
2021-07-01 15:23 ` Fabio M. De Francesco
2021-07-10 0:25 ` kernel test robot
2021-07-13 12:50 ` Greg Kroah-Hartman
2 siblings, 1 reply; 10+ messages in thread
From: David Laight @ 2021-07-01 14:52 UTC (permalink / raw)
To: 'Fabio M. De Francesco',
Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel
From: Fabio M. De Francesco
> Sent: 01 July 2021 14:38
>
> Use crc32_le in place of the custom getcrc32. This change makes GCC
> to warn about incorrect castings to the restricted type __le32, but
> they can be safely ignored because crc32_le calculates bitwise
> little-endian Ethernet AUTODIN II CRC32.
>
...
> - *((__le32 *)crc) = getcrc32(payload, length);/* modified by Amy*/
> + *((__le32 *)crc) = ~crc32_le(~0, payload, length);
Haven't we been round this before?
You need to get rid of these crappy casts.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
2021-07-01 14:52 ` David Laight
@ 2021-07-01 15:23 ` Fabio M. De Francesco
2021-07-01 15:54 ` David Laight
0 siblings, 1 reply; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-07-01 15:23 UTC (permalink / raw)
To: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel,
David Laight, Al Viro
On Thursday, July 1, 2021 4:52:08 PM CEST David Laight wrote:
> From: Fabio M. De Francesco
>
> > Sent: 01 July 2021 14:38
> >
> > Use crc32_le in place of the custom getcrc32. This change makes GCC
> > to warn about incorrect castings to the restricted type __le32, but
> > they can be safely ignored because crc32_le calculates bitwise
> > little-endian Ethernet AUTODIN II CRC32.
>
> ...
>
> > - *((__le32 *)crc) =
getcrc32(payload, length);/* modified by Amy*/
> > + *((__le32 *)crc) =
~crc32_le(~0, payload, length);
>
> Haven't we been round this before?
>
No, I don't think so. At least, not you and I.
Unfortunately, I don't get paid to read every conversation sent to the Linux
related mailing lists.
However, since you pointed out that somewhere, in an unspecified time, this
topic had already been discussed I've just found an email of yours: https://
lore.kernel.org/lkml/bdc0c31a7d28426995d229eb9014cd2b@AcuMS.aculab.com/
I think that there you are talking of something else.
Perhaps, in the same thread there's an email from Al Viro that seems to be
more pertinent: https://lore.kernel.org/lkml/YMz0DH0+v39xsCYU@zeniv-ca.linux.org.uk/
In particular, Al Viro writes "[...] Either make crc __le32 (and turn
assignment into crc = cpu_to_le32(...)), or
make that *(__le32 *)crc = ...".
> You need to get rid of these crappy casts.
So, what am I missing?
Thanks,
Fabio
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
1PT, UK
> Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
2021-07-01 15:23 ` Fabio M. De Francesco
@ 2021-07-01 15:54 ` David Laight
2021-07-01 16:10 ` Fabio M. De Francesco
0 siblings, 1 reply; 10+ messages in thread
From: David Laight @ 2021-07-01 15:54 UTC (permalink / raw)
To: 'Fabio M. De Francesco',
Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel,
Al Viro
From: Fabio M. De Francesco
> Sent: 01 July 2021 16:24
>
> On Thursday, July 1, 2021 4:52:08 PM CEST David Laight wrote:
> > From: Fabio M. De Francesco
> >
> > > Sent: 01 July 2021 14:38
> > >
> > > Use crc32_le in place of the custom getcrc32. This change makes GCC
> > > to warn about incorrect castings to the restricted type __le32, but
> > > they can be safely ignored because crc32_le calculates bitwise
> > > little-endian Ethernet AUTODIN II CRC32.
> >
> > ...
> >
> > > - *((__le32 *)crc) =
> getcrc32(payload, length);/* modified by Amy*/
> > > + *((__le32 *)crc) =
> ~crc32_le(~0, payload, length);
> >
> > Haven't we been round this before?
> >
> No, I don't think so. At least, not you and I.
>
That was rt1872 this is rtl8188 but I think it is the same crap.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
2021-07-01 15:54 ` David Laight
@ 2021-07-01 16:10 ` Fabio M. De Francesco
0 siblings, 0 replies; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-07-01 16:10 UTC (permalink / raw)
To: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel,
Al Viro, David Laight
On Thursday, July 1, 2021 5:54:49 PM CEST David Laight wrote:
> From: Fabio M. De Francesco
>
> > Sent: 01 July 2021 16:24
> >
> > On Thursday, July 1, 2021 4:52:08 PM CEST David Laight wrote:
> >
> > > From: Fabio M. De Francesco
> > >
> > >
> > >
> > > > Sent: 01 July 2021 14:38
> > > >
> > > >
> > > >
> > > > Use crc32_le in place of the custom getcrc32. This change makes GCC
> > > > to warn about incorrect castings to the restricted type __le32, but
> > > > they can be safely ignored because crc32_le calculates bitwise
> > > > little-endian Ethernet AUTODIN II CRC32.
> > >
> > >
> > >
> > > ...
> > >
> > >
> > >
> > > > - *((__le32 *)crc) =
> >
> > getcrc32(payload, length);/* modified by Amy*/
> >
> > > > + *((__le32 *)crc) =
> >
> > ~crc32_le(~0, payload, length);
> >
> > >
> > >
> > > Haven't we been round this before?
> > >
> > >
> >
> > No, I don't think so. At least, not you and I.
> >
>
>
> That was rt1872 this is rtl8188 but I think it is the same crap.
>
Perhaps it is the same crap... However, the patch is in accordance to one of
the two solution that Al Viro wrote about.
I think I'll leave the patch as is and wait for the final review by Greg K-H.
Thanks,
Fabio
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
1PT, UK
> Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
2021-07-01 13:38 [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le() Fabio M. De Francesco
2021-07-01 14:52 ` David Laight
@ 2021-07-10 0:25 ` kernel test robot
2021-07-10 14:38 ` Fabio M. De Francesco
2021-07-13 12:50 ` Greg Kroah-Hartman
2 siblings, 1 reply; 10+ messages in thread
From: kernel test robot @ 2021-07-10 0:25 UTC (permalink / raw)
To: Fabio M. De Francesco, Larry Finger, Greg Kroah-Hartman,
linux-staging, linux-kernel
Cc: kbuild-all, Fabio M. De Francesco
[-- Attachment #1: Type: text/plain, Size: 5721 bytes --]
Hi "Fabio,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
url: https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/staging-rtl8188eu-Replace-a-custom-function-with-crc32_le/20210701-213922
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 77ad1f0e99bd00af024e650b862cfda3137af660
config: powerpc64-randconfig-s032-20210709 (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/c296b1e11db85710adc1831c4cdb2295192e447f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Fabio-M-De-Francesco/staging-rtl8188eu-Replace-a-custom-function-with-crc32_le/20210701-213922
git checkout c296b1e11db85710adc1831c4cdb2295192e447f
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=powerpc64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/staging/rtl8188eu/core/rtw_security.c:597:58: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] @@ got unsigned int @@
drivers/staging/rtl8188eu/core/rtw_security.c:597:58: sparse: expected restricted __le32 [usertype]
drivers/staging/rtl8188eu/core/rtw_security.c:597:58: sparse: got unsigned int
drivers/staging/rtl8188eu/core/rtw_security.c:604:58: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] @@ got unsigned int @@
drivers/staging/rtl8188eu/core/rtw_security.c:604:58: sparse: expected restricted __le32 [usertype]
drivers/staging/rtl8188eu/core/rtw_security.c:604:58: sparse: got unsigned int
drivers/staging/rtl8188eu/core/rtw_security.c:671:42: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] @@ got unsigned int @@
drivers/staging/rtl8188eu/core/rtw_security.c:671:42: sparse: expected restricted __le32 [usertype]
drivers/staging/rtl8188eu/core/rtw_security.c:671:42: sparse: got unsigned int
vim +597 drivers/staging/rtl8188eu/core/rtw_security.c
544
545 /* The hlen isn't include the IV */
546 u32 rtw_tkip_encrypt(struct adapter *padapter, struct xmit_frame *pxmitframe)
547 { /* exclude ICV */
548 u16 pnl;
549 u32 pnh;
550 u8 rc4key[16];
551 u8 ttkey[16];
552 u8 crc[4];
553 u8 hw_hdr_offset = 0;
554 struct arc4context mycontext;
555 int curfragnum, length;
556
557 u8 *pframe, *payload, *iv, *prwskey;
558 union pn48 dot11txpn;
559 struct sta_info *stainfo;
560 struct pkt_attrib *pattrib = &pxmitframe->attrib;
561 struct security_priv *psecuritypriv = &padapter->securitypriv;
562 struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
563 u32 res = _SUCCESS;
564
565 if (!pxmitframe->buf_addr)
566 return _FAIL;
567
568 hw_hdr_offset = TXDESC_SIZE +
569 (pxmitframe->pkt_offset * PACKET_OFFSET_SZ);
570 pframe = pxmitframe->buf_addr + hw_hdr_offset;
571 /* 4 start to encrypt each fragment */
572 if (pattrib->encrypt == _TKIP_) {
573 if (pattrib->psta)
574 stainfo = pattrib->psta;
575 else
576 stainfo = rtw_get_stainfo(&padapter->stapriv, &pattrib->ra[0]);
577
578 if (stainfo) {
579 if (is_multicast_ether_addr(pattrib->ra))
580 prwskey = psecuritypriv->dot118021XGrpKey[psecuritypriv->dot118021XGrpKeyid].skey;
581 else
582 prwskey = &stainfo->dot118021x_UncstKey.skey[0];
583
584 for (curfragnum = 0; curfragnum < pattrib->nr_frags; curfragnum++) {
585 iv = pframe + pattrib->hdrlen;
586 payload = pframe + pattrib->iv_len + pattrib->hdrlen;
587
588 GET_TKIP_PN(iv, dot11txpn);
589
590 pnl = (u16)(dot11txpn.val);
591 pnh = (u32)(dot11txpn.val >> 16);
592 phase1((u16 *)&ttkey[0], prwskey, &pattrib->ta[0], pnh);
593 phase2(&rc4key[0], prwskey, (u16 *)&ttkey[0], pnl);
594
595 if ((curfragnum + 1) == pattrib->nr_frags) { /* 4 the last fragment */
596 length = pattrib->last_txcmdsz - pattrib->hdrlen - pattrib->iv_len - pattrib->icv_len;
> 597 *((__le32 *)crc) = ~crc32_le(~0, payload, length);
598
599 arcfour_init(&mycontext, rc4key, 16);
600 arcfour_encrypt(&mycontext, payload, payload, length);
601 arcfour_encrypt(&mycontext, payload + length, crc, 4);
602 } else {
603 length = pxmitpriv->frag_len - pattrib->hdrlen - pattrib->iv_len - pattrib->icv_len;
604 *((__le32 *)crc) = ~crc32_le(~0, payload, length);
605
606 arcfour_init(&mycontext, rc4key, 16);
607 arcfour_encrypt(&mycontext, payload, payload, length);
608 arcfour_encrypt(&mycontext, payload + length, crc, 4);
609
610 pframe += pxmitpriv->frag_len;
611 pframe = (u8 *)round_up((size_t)(pframe), 4);
612 }
613 }
614 } else {
615 res = _FAIL;
616 }
617 }
618 return res;
619 }
620
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40008 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
2021-07-10 0:25 ` kernel test robot
@ 2021-07-10 14:38 ` Fabio M. De Francesco
2021-07-10 20:42 ` David Laight
0 siblings, 1 reply; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-07-10 14:38 UTC (permalink / raw)
To: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel,
kernel test robot
Cc: kbuild-all, fabioaiuto83
On Saturday, July 10, 2021 2:25:12 AM CEST kernel test robot wrote:
> Hi "Fabio,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on staging/staging-testing]
>
> url:
> https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/staging-rtl8188eu-Replace
> -a-custom-function-with-crc32_le/20210701-213922 base:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> 77ad1f0e99bd00af024e650b862cfda3137af660 config: powerpc64-randconfig-
s032-20210709
[CUT]
>
> sparse warnings: (new ones prefixed by >>)
>
> >> drivers/staging/rtl8188eu/core/rtw_security.c:597:58: sparse: sparse:
incorrect type
> >> in assignment (different base types) @@ expected restricted __le32
[usertype] @@
> >> got unsigned int @@
> drivers/staging/rtl8188eu/core/rtw_security.c:597:58: sparse:
expected restricted
> __le32 [usertype] drivers/staging/rtl8188eu/core/rtw_security.c:597:58:
sparse: got
> unsigned int drivers/staging/rtl8188eu/core/rtw_security.c:604:58: sparse:
sparse:
> incorrect type in assignment (different base types) @@ expected
restricted __le32
> [usertype] @@ got unsigned int @@
> drivers/staging/rtl8188eu/core/rtw_security.c:604:58: sparse: expected
restricted
> __le32 [usertype] drivers/staging/rtl8188eu/core/rtw_security.c:604:58:
sparse: got
> unsigned int drivers/staging/rtl8188eu/core/rtw_security.c:671:42: sparse:
sparse:
> incorrect type in assignment (different base types) @@ expected
restricted __le32
> [usertype] @@ got unsigned int @@
> drivers/staging/rtl8188eu/core/rtw_security.c:671:42: sparse: expected
restricted
> __le32 [usertype] drivers/staging/rtl8188eu/core/rtw_security.c:671:42:
sparse: got
> unsigned int
[CUT]
I suppose that these warnings are false positives for the reasons explained in
my patch.
Furthermore, I found a commit by Fabio Aiuto <fabioaiuto83@gmail.com> that, as
far as I understand, do the same changes and that has been accepted by Greg K-
H and merged in his tree: b97fad10de387c09ae46f607955c7237afa96654. So, I
think there one reason more to suspect of false positives.
I'd appreciate comments on this topic.
Thanks,
Fabio
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
2021-07-10 14:38 ` Fabio M. De Francesco
@ 2021-07-10 20:42 ` David Laight
0 siblings, 0 replies; 10+ messages in thread
From: David Laight @ 2021-07-10 20:42 UTC (permalink / raw)
To: 'Fabio M. De Francesco',
Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel,
kernel test robot
Cc: kbuild-all, fabioaiuto83
From: Fabio M. De Francesco
> Sent: 10 July 2021 15:39
...
> [CUT]
[PASTE}
> 597 *((__le32 *)crc) = ~crc32_le(~0, payload, length);
>
> I suppose that these warnings are false positives for the reasons explained in
> my patch.
No they are an indication you aren't doing things 'right'.
You shouldn't need an __le32 cast in this code.
'crc' should be defined as __le32 (not u8[4]) and IIRC
get_unaligned_le32() used in the latter check.
Actually what is this code actually doing.
ISTR it is doing a crc32() and then comparing the result with the
crc in the buffer?
No hardware ever does that.
What happens is the receiver does the crc of the whole buffer
including the transmitted crc and then checks the value is the
required 'magic' constant'.
This all works because the crc is inverted before transmission.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
2021-07-01 13:38 [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le() Fabio M. De Francesco
2021-07-01 14:52 ` David Laight
2021-07-10 0:25 ` kernel test robot
@ 2021-07-13 12:50 ` Greg Kroah-Hartman
2021-07-13 17:52 ` Fabio M. De Francesco
2 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2021-07-13 12:50 UTC (permalink / raw)
To: Fabio M. De Francesco; +Cc: Larry Finger, linux-staging, linux-kernel
On Thu, Jul 01, 2021 at 03:38:09PM +0200, Fabio M. De Francesco wrote:
> Use crc32_le in place of the custom getcrc32. This change makes GCC
> to warn about incorrect castings to the restricted type __le32, but
> they can be safely ignored because crc32_le calculates bitwise
> little-endian Ethernet AUTODIN II CRC32.
>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> drivers/staging/rtl8188eu/core/rtw_security.c | 22 ++++---------------
> 1 file changed, 4 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c b/drivers/staging/rtl8188eu/core/rtw_security.c
> index 1b2cb6196463..5f010cb66970 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_security.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_security.c
> @@ -111,21 +111,6 @@ static void crc32_init(void)
> bcrc32initialized = 1;
> }
>
> -static __le32 getcrc32(u8 *buf, int len)
> -{
> - u8 *p;
> - u32 crc;
> -
> - if (bcrc32initialized == 0)
> - crc32_init();
> -
> - crc = 0xffffffff; /* preload shift register, per CRC-32 spec */
> -
> - for (p = buf; len > 0; ++p, --len)
> - crc = crc32_table[(crc ^ *p) & 0xff] ^ (crc >> 8);
> - return cpu_to_le32(~crc); /* transmit complement, per CRC-32 spec */
> -}
> -
> /* Need to consider the fragment situation */
> void rtw_wep_encrypt(struct adapter *padapter, struct xmit_frame *pxmitframe)
> {
> @@ -609,14 +594,15 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, struct xmit_frame *pxmitframe)
>
> if ((curfragnum + 1) == pattrib->nr_frags) { /* 4 the last fragment */
> length = pattrib->last_txcmdsz - pattrib->hdrlen - pattrib->iv_len - pattrib->icv_len;
> - *((__le32 *)crc) = getcrc32(payload, length);/* modified by Amy*/
> + *((__le32 *)crc) = ~crc32_le(~0, payload, length);
Why are you casting a native endian return value to a little endian
pointer? Are you _SURE_ that is correct?
We can not just ignore warnings, they are there for a reason. Or if
not, then fix the code up to not have the warnings, but I can't take
this as-is, sorry.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
2021-07-13 12:50 ` Greg Kroah-Hartman
@ 2021-07-13 17:52 ` Fabio M. De Francesco
0 siblings, 0 replies; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-07-13 17:52 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Larry Finger, linux-staging, linux-kernel
On Tuesday, July 13, 2021 2:50:47 PM CEST Greg Kroah-Hartman wrote:
> On Thu, Jul 01, 2021 at 03:38:09PM +0200, Fabio M. De Francesco wrote:
> > Use crc32_le in place of the custom getcrc32. This change makes GCC
> > to warn about incorrect castings to the restricted type __le32, but
> > they can be safely ignored because crc32_le calculates bitwise
> > little-endian Ethernet AUTODIN II CRC32.
> >
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> >
> > drivers/staging/rtl8188eu/core/rtw_security.c | 22 ++++---------------
> > 1 file changed, 4 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c
> > b/drivers/staging/rtl8188eu/core/rtw_security.c index
1b2cb6196463..5f010cb66970
> > 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_security.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_security.c
> > @@ -111,21 +111,6 @@ static void crc32_init(void)
> >
[...]
>
> Why are you casting a native endian return value to a little endian
> pointer?
>
Actually, I was not sure whether or not crc32_le() returns native or little
endian values:
(1) That "_le" in crc32_le() made me think it calculates little endian values
and the documentation of the function is a little misleading where it affirms
that "[crc32_le()] calculates *bitwise
little-endian* Ethernet AUTODIN II CRC32".
(2) Furthermore, I took commit b97fad10de387 as a model for my patch. You
applied that commit and it makes exactly the same change that I make in my
patch. I still cannot understand why you took that. Maybe I'm missing
something that I can't still see. What is it?
>
> Are you _SURE_ that is correct?
>
To summarize, I thought that crc32_le is some way returning an LE value in an
array of four u8 (aka an unsigned integer).
Now I suppose I have to change "*((__le32 *)crc) = ~crc32_le(~0, payload,
length);" to "*((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)".
Thanks,
Fabio
> We can not just ignore warnings, they are there for a reason. Or if
> not, then fix the code up to not have the warnings, but I can't take
> this as-is, sorry.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-07-13 17:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-01 13:38 [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le() Fabio M. De Francesco
2021-07-01 14:52 ` David Laight
2021-07-01 15:23 ` Fabio M. De Francesco
2021-07-01 15:54 ` David Laight
2021-07-01 16:10 ` Fabio M. De Francesco
2021-07-10 0:25 ` kernel test robot
2021-07-10 14:38 ` Fabio M. De Francesco
2021-07-10 20:42 ` David Laight
2021-07-13 12:50 ` Greg Kroah-Hartman
2021-07-13 17:52 ` Fabio M. De Francesco
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).