LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] rtw_security: fix cast to restricted __le32
@ 2021-06-13 12:28 Jhih-Ming Huang
  2021-06-13 12:34 ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: Jhih-Ming Huang @ 2021-06-13 12:28 UTC (permalink / raw)
  To: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel
  Cc: Jhih-Ming Huang

This patch fixes the sparse warning of fix cast to restricted __le32.

Last month, there was a change for replacing private CRC-32 routines with
in-kernel ones.
In that patch, we replaced getcrc32 with crc32_le in calling le32_to_cpu.
le32_to_cpu accepts __le32 type as arg, but crc32_le returns unsigned int.
That how it introduced the sparse warning.

This patch uses __force to fix the warning.

Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index a99f439328f1..2f4da67e31c6 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 		arc4_crypt(ctx, payload, payload,  length);
 
 		/* calculate icv and compare the icv */
-		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+		*((u32 *)crc) = le32_to_cpu((__force __le32)~crc32_le(~0, payload, length - 4));
 
 	}
 }
@@ -618,7 +618,8 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 			arc4_setkey(ctx, rc4key, 16);
 			arc4_crypt(ctx, payload, payload, length);
 
-			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+			*((u32 *)crc) = le32_to_cpu((__force __le32)~crc32_le(~0, payload,
+						length - 4));
 
 			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
 			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
-- 
2.25.1


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

* Re: [PATCH v2] rtw_security: fix cast to restricted __le32
  2021-06-13 12:28 [PATCH v2] rtw_security: fix cast to restricted __le32 Jhih-Ming Huang
@ 2021-06-13 12:34 ` Greg KH
  2021-06-13 16:40   ` Jhih Ming Huang
  0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2021-06-13 12:34 UTC (permalink / raw)
  To: Jhih-Ming Huang
  Cc: fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

On Sun, Jun 13, 2021 at 08:28:58PM +0800, Jhih-Ming Huang wrote:
> This patch fixes the sparse warning of fix cast to restricted __le32.
> 
> Last month, there was a change for replacing private CRC-32 routines with
> in-kernel ones.
> In that patch, we replaced getcrc32 with crc32_le in calling le32_to_cpu.
> le32_to_cpu accepts __le32 type as arg, but crc32_le returns unsigned int.
> That how it introduced the sparse warning.

As crc32_le returns a u32 which is in native-endian format, how can you
cast it to le32?  Why do you cast it to le32?  Isn't that going to be
incorrect for big endian systems?

thanks,

greg k-h

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

* Re: [PATCH v2] rtw_security: fix cast to restricted __le32
  2021-06-13 12:34 ` Greg KH
@ 2021-06-13 16:40   ` Jhih Ming Huang
  2021-06-14 14:14     ` Al Viro
  0 siblings, 1 reply; 19+ messages in thread
From: Jhih Ming Huang @ 2021-06-13 16:40 UTC (permalink / raw)
  To: Greg KH
  Cc: fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

On Sun, Jun 13, 2021 at 8:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jun 13, 2021 at 08:28:58PM +0800, Jhih-Ming Huang wrote:
> > This patch fixes the sparse warning of fix cast to restricted __le32.
> >
> > Last month, there was a change for replacing private CRC-32 routines with
> > in-kernel ones.
> > In that patch, we replaced getcrc32 with crc32_le in calling le32_to_cpu.
> > le32_to_cpu accepts __le32 type as arg, but crc32_le returns unsigned int.
> > That how it introduced the sparse warning.
>
> As crc32_le returns a u32 which is in native-endian format, how can you
> cast it to le32?  Why do you cast it to le32?  Isn't that going to be
> incorrect for big endian systems?
>
> thanks,
>
> greg k-h

Thanks for the fast reply.
Yes, you are right. I did not notice that le32_to_cpu already handles
both of the cases.

So it seems the warning from sparse is false positives, am I right?

thanks

--jmhuang

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

* Re: [PATCH v2] rtw_security: fix cast to restricted __le32
  2021-06-13 16:40   ` Jhih Ming Huang
@ 2021-06-14 14:14     ` Al Viro
  2021-06-14 15:27       ` Jhih Ming Huang
  0 siblings, 1 reply; 19+ messages in thread
From: Al Viro @ 2021-06-14 14:14 UTC (permalink / raw)
  To: Jhih Ming Huang
  Cc: Greg KH, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

On Mon, Jun 14, 2021 at 12:40:27AM +0800, Jhih Ming Huang wrote:
> On Sun, Jun 13, 2021 at 8:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Sun, Jun 13, 2021 at 08:28:58PM +0800, Jhih-Ming Huang wrote:
> > > This patch fixes the sparse warning of fix cast to restricted __le32.
> > >
> > > Last month, there was a change for replacing private CRC-32 routines with
> > > in-kernel ones.
> > > In that patch, we replaced getcrc32 with crc32_le in calling le32_to_cpu.
> > > le32_to_cpu accepts __le32 type as arg, but crc32_le returns unsigned int.
> > > That how it introduced the sparse warning.
> >
> > As crc32_le returns a u32 which is in native-endian format, how can you
> > cast it to le32?  Why do you cast it to le32?  Isn't that going to be
> > incorrect for big endian systems?
> >
> > thanks,
> >
> > greg k-h
> 
> Thanks for the fast reply.
> Yes, you are right. I did not notice that le32_to_cpu already handles
> both of the cases.
> 
> So it seems the warning from sparse is false positives, am I right?

In a sense that on all architectures we would be ever likely to support
le32_to_cpu and cpu_to_le32 do the same bit-shuffling - yes.  In a sense
of having those used correctly it's not a false positive, though - it's
much easier to follow "this variable always hold native-endian, this -
little-endian" and watch for conversions done correctly than to count
the byteswaps and try to prove that it's either even for all execution
histories or odd for all execution histories.

IOW, there's a good reason for keeping separate cpu_to_le32 and le32_to_cpu
and not mixing them with each other - it's easier to prove correctness that
way *and* easier to look for endianness bugs.

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

* Re: [PATCH v2] rtw_security: fix cast to restricted __le32
  2021-06-14 14:14     ` Al Viro
@ 2021-06-14 15:27       ` Jhih Ming Huang
  2021-06-14 17:03         ` Al Viro
  0 siblings, 1 reply; 19+ messages in thread
From: Jhih Ming Huang @ 2021-06-14 15:27 UTC (permalink / raw)
  To: Al Viro
  Cc: Greg KH, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

On Mon, Jun 14, 2021 at 10:14 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Mon, Jun 14, 2021 at 12:40:27AM +0800, Jhih Ming Huang wrote:
> > On Sun, Jun 13, 2021 at 8:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Sun, Jun 13, 2021 at 08:28:58PM +0800, Jhih-Ming Huang wrote:
> > > > This patch fixes the sparse warning of fix cast to restricted __le32.
> > > >
> > > > Last month, there was a change for replacing private CRC-32 routines with
> > > > in-kernel ones.
> > > > In that patch, we replaced getcrc32 with crc32_le in calling le32_to_cpu.
> > > > le32_to_cpu accepts __le32 type as arg, but crc32_le returns unsigned int.
> > > > That how it introduced the sparse warning.
> > >
> > > As crc32_le returns a u32 which is in native-endian format, how can you
> > > cast it to le32?  Why do you cast it to le32?  Isn't that going to be
> > > incorrect for big endian systems?
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Thanks for the fast reply.
> > Yes, you are right. I did not notice that le32_to_cpu already handles
> > both of the cases.
> >
> > So it seems the warning from sparse is false positives, am I right?
>
> In a sense that on all architectures we would be ever likely to support
> le32_to_cpu and cpu_to_le32 do the same bit-shuffling - yes.  In a sense
> of having those used correctly it's not a false positive, though - it's
> much easier to follow "this variable always hold native-endian, this -
> little-endian" and watch for conversions done correctly than to count
> the byteswaps and try to prove that it's either even for all execution
> histories or odd for all execution histories.
>
> IOW, there's a good reason for keeping separate cpu_to_le32 and le32_to_cpu
> and not mixing them with each other - it's easier to prove correctness that
> way *and* easier to look for endianness bugs.

Thanks for your explanation.

To clarify, even though it might be false positives in some senses,
following "hold the variable native-endian and check the conversion
done correctly"
is much easier than the other way. And it's exactly the current implementation.

So it's better to keep the current implementation and ignore the
warnings, right?

Thanks. Regards

--jmhuang

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

* Re: [PATCH v2] rtw_security: fix cast to restricted __le32
  2021-06-14 15:27       ` Jhih Ming Huang
@ 2021-06-14 17:03         ` Al Viro
  2021-06-18 18:17           ` [PATCH v3] " Jhih-Ming Huang
  2021-06-18 18:28           ` [PATCH v2] " Jhih Ming Huang
  0 siblings, 2 replies; 19+ messages in thread
From: Al Viro @ 2021-06-14 17:03 UTC (permalink / raw)
  To: Jhih Ming Huang
  Cc: Greg KH, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

On Mon, Jun 14, 2021 at 11:27:03PM +0800, Jhih Ming Huang wrote:

> Thanks for your explanation.
> 
> To clarify, even though it might be false positives in some senses,
> following "hold the variable native-endian and check the conversion
> done correctly"
> is much easier than the other way. And it's exactly the current implementation.
> 
> So it's better to keep the current implementation and ignore the
> warnings, right?

Umm...  If that's the case, the warnings should go away if you use
cpu_to_le32() for conversions from native to l-e and le32_to_cpu()
for conversions from l-e to native.

IOW, the choice between those should annotate what's going on.

In your case doing
	*((u32 *)crc) = le32_to_cpu((__force __le32)~crc32_le(~0, payload, length - 4));
is wrong - you have
crc32_le(...) native-endian
~crc32_le(...) - ditto
le32_to_cpu(~crc32_le(...)) - byteswapped native-endian on b-e, unchanged on
l-e.  So result will be little-endian representation of ~crc32(...) in all
cases.  IOW, it's cpu_to_le32(~crc32_le(...)), misannotated as native-endian
instead of little-endian it actually is.

Then you store that value (actually __le32) into *(u32 *)crc.  Seeing that
crc is u8[4] there, that *(u32 *) is misleading - you are actually storing
__le32 there (and, AFAICS, doing noting with the result).  The same story
in rtw_tkip_decrypt(), only there you do use the result later.

So just make it __le32 crc and
	crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
with
			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
turned into
			if (memcmp(&crc, payload + length - 4, 4) != 0)
(or (crc != get_unaligned((__le32 *)(payload + length - 4))),
for that matter, to document what's going on and let the damn thing
pick the optimal implementation for given architecture).

Incidentally, your secmicgetuint32() is simply get_unaligned_le32()
and secmicputuint32() - put_unaligned_le32().  No need to reinvent
that wheel...


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

* [PATCH v3] rtw_security: fix cast to restricted __le32
  2021-06-14 17:03         ` Al Viro
@ 2021-06-18 18:17           ` Jhih-Ming Huang
  2021-06-18 19:29             ` Al Viro
  2021-06-18 18:28           ` [PATCH v2] " Jhih Ming Huang
  1 sibling, 1 reply; 19+ messages in thread
From: Jhih-Ming Huang @ 2021-06-18 18:17 UTC (permalink / raw)
  To: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel
  Cc: Jhih-Ming Huang

This patch fixes the sparse warning of fix cast to restricted __le32.

There was a change for replacing private CRC-32 routines with in kernel
ones.
However, the author used le32_to_cpu to convert crc32_le(), and we
should cpu_to_le32.

Ths commit also fixes the payload checking by memcmp instead of checking element
by element.

Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index a99f439328f1..97a7485f8f58 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 		arc4_crypt(ctx, payload, payload,  length);
 
 		/* calculate icv and compare the icv */
-		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+		*crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
 
 	}
 }
@@ -618,10 +618,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 			arc4_setkey(ctx, rc4key, 16);
 			arc4_crypt(ctx, payload, payload, length);
 
-			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+			*crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
 
-			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
-			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
+			if (memcmp(&crc, payload + length - 4, 4) != 0)
 				res = _FAIL;
 		} else {
 			res = _FAIL;
-- 
2.32.0.288


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

* Re: [PATCH v2] rtw_security: fix cast to restricted __le32
  2021-06-14 17:03         ` Al Viro
  2021-06-18 18:17           ` [PATCH v3] " Jhih-Ming Huang
@ 2021-06-18 18:28           ` Jhih Ming Huang
  1 sibling, 0 replies; 19+ messages in thread
From: Jhih Ming Huang @ 2021-06-18 18:28 UTC (permalink / raw)
  To: Al Viro
  Cc: Greg KH, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

On Tue, Jun 15, 2021 at 1:03 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Mon, Jun 14, 2021 at 11:27:03PM +0800, Jhih Ming Huang wrote:
>
> > Thanks for your explanation.
> >
> > To clarify, even though it might be false positives in some senses,
> > following "hold the variable native-endian and check the conversion
> > done correctly"
> > is much easier than the other way. And it's exactly the current implementation.
> >
> > So it's better to keep the current implementation and ignore the
> > warnings, right?
>
> Umm...  If that's the case, the warnings should go away if you use
> cpu_to_le32() for conversions from native to l-e and le32_to_cpu()
> for conversions from l-e to native.
>
> IOW, the choice between those should annotate what's going on.
>
> In your case doing
>         *((u32 *)crc) = le32_to_cpu((__force __le32)~crc32_le(~0, payload, length - 4));
> is wrong - you have
> crc32_le(...) native-endian
> ~crc32_le(...) - ditto
> le32_to_cpu(~crc32_le(...)) - byteswapped native-endian on b-e, unchanged on
> l-e.  So result will be little-endian representation of ~crc32(...) in all
> cases.  IOW, it's cpu_to_le32(~crc32_le(...)), misannotated as native-endian
> instead of little-endian it actually is.
>
> Then you store that value (actually __le32) into *(u32 *)crc.  Seeing that
> crc is u8[4] there, that *(u32 *) is misleading - you are actually storing
> __le32 there (and, AFAICS, doing noting with the result).  The same story
> in rtw_tkip_decrypt(), only there you do use the result later.
>
> So just make it __le32 crc and
>         crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
> with
>                         if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
>                             crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
> turned into
>                         if (memcmp(&crc, payload + length - 4, 4) != 0)
> (or (crc != get_unaligned((__le32 *)(payload + length - 4))),
> for that matter, to document what's going on and let the damn thing
> pick the optimal implementation for given architecture).
>
> Incidentally, your secmicgetuint32() is simply get_unaligned_le32()
> and secmicputuint32() - put_unaligned_le32().  No need to reinvent
> that wheel...
>

Thanks for your comprehensive explanation.

I just sent the v3 PATCH, but I replied to this thread.
Should I create the other thread?

For the secmicgetuint32(), I am not the author of this function,
but you are right we should not reinvent the wheel.

Let's focus on sparse warning fixing in this commit.

thanks.

--jmhuang

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

* Re: [PATCH v3] rtw_security: fix cast to restricted __le32
  2021-06-18 18:17           ` [PATCH v3] " Jhih-Ming Huang
@ 2021-06-18 19:29             ` Al Viro
  2021-06-19  7:52               ` [PATCH v4] " Jhih-Ming Huang
  2021-06-19  9:20               ` [PATCH v3] " Jhih-Ming Huang
  0 siblings, 2 replies; 19+ messages in thread
From: Al Viro @ 2021-06-18 19:29 UTC (permalink / raw)
  To: Jhih-Ming Huang
  Cc: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

On Sat, Jun 19, 2021 at 02:17:51AM +0800, Jhih-Ming Huang wrote:
> This patch fixes the sparse warning of fix cast to restricted __le32.
> 
> There was a change for replacing private CRC-32 routines with in kernel
> ones.
> However, the author used le32_to_cpu to convert crc32_le(), and we
> should cpu_to_le32.
> 
> Ths commit also fixes the payload checking by memcmp instead of checking element
> by element.
> 
> Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_security.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
> index a99f439328f1..97a7485f8f58 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
> @@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
>  		arc4_crypt(ctx, payload, payload,  length);
>  
>  		/* calculate icv and compare the icv */
> -		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
> +		*crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));

Huh?  crc is u8[4]; that assignment will truncate that le32 to u8 and store it in
the first byte of your 4-element array.  How the hell does sparse *not* complain
on that?

Either make crc __le32 (and turn assignment into crc = cpu_to_le32(...)), or
make that *(__le32 *)crc = ...

> @@ -618,10 +618,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
>  			arc4_setkey(ctx, rc4key, 16);
>  			arc4_crypt(ctx, payload, payload, length);
>  
> -			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
> +			*crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));

Ditto.  Declare crc as __le32 and use
			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
here.

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

* [PATCH v4] rtw_security: fix cast to restricted __le32
  2021-06-18 19:29             ` Al Viro
@ 2021-06-19  7:52               ` Jhih-Ming Huang
  2021-06-21  8:19                 ` [PATCH v5] " Jhih-Ming Huang
  2021-06-19  9:20               ` [PATCH v3] " Jhih-Ming Huang
  1 sibling, 1 reply; 19+ messages in thread
From: Jhih-Ming Huang @ 2021-06-19  7:52 UTC (permalink / raw)
  To: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel
  Cc: Jhih-Ming Huang

This patch fixes the sparse warning of fix cast to restricted __le32.

There was a change for replacing private CRC-32 routines with in kernel
ones.
However, the author used le32_to_cpu to convert crc32_le(), and we
should cpu_to_le32.

Ths commit also fixes the payload checking by memcmp instead of checking element
by element.

Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index a99f439328f1..3a2711e21a0f 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -92,7 +92,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
 void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 {
 	/*  exclude ICV */
-	u8 crc[4];
+	__le32 crc;
 	signed int	length;
 	u32 keylength;
 	u8 *pframe, *payload, *iv, wepkey[16];
@@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 		arc4_crypt(ctx, payload, payload,  length);
 
 		/* calculate icv and compare the icv */
-		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+		crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
 
 	}
 }
@@ -537,7 +537,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 	u32 pnh;
 	u8   rc4key[16];
 	u8   ttkey[16];
-	u8 crc[4];
+	__le32 crc;
 	signed int			length;
 
 	u8 *pframe, *payload, *iv, *prwskey;
@@ -618,10 +618,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 			arc4_setkey(ctx, rc4key, 16);
 			arc4_crypt(ctx, payload, payload, length);
 
-			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
 
-			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
-			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
+			if (memcmp(&crc, payload + length - 4, 4) != 0)
 				res = _FAIL;
 		} else {
 			res = _FAIL;
-- 
2.32.0.288.g62a8d224e6-goog


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

* Re: [PATCH v3] rtw_security: fix cast to restricted __le32
  2021-06-18 19:29             ` Al Viro
  2021-06-19  7:52               ` [PATCH v4] " Jhih-Ming Huang
@ 2021-06-19  9:20               ` Jhih-Ming Huang
  1 sibling, 0 replies; 19+ messages in thread
From: Jhih-Ming Huang @ 2021-06-19  9:20 UTC (permalink / raw)
  To: Al Viro
  Cc: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

On Fri, Jun 18, 2021 at 07:29:16PM +0000, Al Viro wrote:
> On Sat, Jun 19, 2021 at 02:17:51AM +0800, Jhih-Ming Huang wrote:
> > This patch fixes the sparse warning of fix cast to restricted __le32.
> > 
> > There was a change for replacing private CRC-32 routines with in kernel
> > ones.
> > However, the author used le32_to_cpu to convert crc32_le(), and we
> > should cpu_to_le32.
> > 
> > Ths commit also fixes the payload checking by memcmp instead of checking element
> > by element.
> > 
> > Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
> > ---
> >  drivers/staging/rtl8723bs/core/rtw_security.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
> > index a99f439328f1..97a7485f8f58 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
> > @@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
> >  		arc4_crypt(ctx, payload, payload,  length);
> >  
> >  		/* calculate icv and compare the icv */
> > -		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
> > +		*crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
> 
> Huh?  crc is u8[4]; that assignment will truncate that le32 to u8 and store it in
> the first byte of your 4-element array.  How the hell does sparse *not* complain
> on that?
facepalm... fixed in v4 PATCH.

thanks for your help.
> 
> Either make crc __le32 (and turn assignment into crc = cpu_to_le32(...)), or
> make that *(__le32 *)crc = ...
> 
> > @@ -618,10 +618,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
> >  			arc4_setkey(ctx, rc4key, 16);
> >  			arc4_crypt(ctx, payload, payload, length);
> >  
> > -			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
> > +			*crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
> 
> Ditto.  Declare crc as __le32 and use
> 			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
> here.

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

* [PATCH v5] rtw_security: fix cast to restricted __le32
  2021-06-19  7:52               ` [PATCH v4] " Jhih-Ming Huang
@ 2021-06-21  8:19                 ` Jhih-Ming Huang
  2021-06-21 15:48                   ` [PATCH v6] " Jhih-Ming Huang
                                     ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Jhih-Ming Huang @ 2021-06-21  8:19 UTC (permalink / raw)
  To: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel
  Cc: Jhih-Ming Huang

This patch fixes the sparse warning of fix cast to restricted __le32.

There was a change for replacing private CRC-32 routines with in kernel
ones.
However, the author used le32_to_cpu to convert crc32_le(), and we
should cpu_to_le32.

Ths commit also fixes the payload checking by memcmp instead of checking element
by element and removes the unused variable.

Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index a99f439328f1..8dc6a976b487 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -92,7 +92,6 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
 void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 {
 	/*  exclude ICV */
-	u8 crc[4];
 	signed int	length;
 	u32 keylength;
 	u8 *pframe, *payload, *iv, wepkey[16];
@@ -119,10 +118,6 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 		/* decrypt payload include icv */
 		arc4_setkey(ctx, wepkey, 3 + keylength);
 		arc4_crypt(ctx, payload, payload,  length);
-
-		/* calculate icv and compare the icv */
-		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
-
 	}
 }
 
@@ -537,7 +532,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 	u32 pnh;
 	u8   rc4key[16];
 	u8   ttkey[16];
-	u8 crc[4];
+	__le32 crc;
 	signed int			length;
 
 	u8 *pframe, *payload, *iv, *prwskey;
@@ -618,10 +613,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 			arc4_setkey(ctx, rc4key, 16);
 			arc4_crypt(ctx, payload, payload, length);
 
-			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
 
-			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
-			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
+			if (memcmp(&crc, payload + length - 4, 4) != 0)
 				res = _FAIL;
 		} else {
 			res = _FAIL;
-- 
2.32.0


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

* [PATCH v6] rtw_security: fix cast to restricted __le32
  2021-06-21  8:19                 ` [PATCH v5] " Jhih-Ming Huang
@ 2021-06-21 15:48                   ` Jhih-Ming Huang
  2021-06-21 15:51                   ` [PATCH v5] " Jhih-Ming Huang
  2021-06-22  9:31                   ` David Laight
  2 siblings, 0 replies; 19+ messages in thread
From: Jhih-Ming Huang @ 2021-06-21 15:48 UTC (permalink / raw)
  To: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel
  Cc: Jhih-Ming Huang, kernel test robot

This patch fixes the sparse warning of fix cast to restricted __le32.

There was a change for replacing private CRC-32 routines with in kernel
ones.
However, the author used le32_to_cpu to convert crc32_le(), and we
should cpu_to_le32.

Ths commit also fixes the payload checking by memcmp instead of checking element
by element and removes the unused variable.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index a99f439328f1..8dc6a976b487 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -92,7 +92,6 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
 void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 {
 	/*  exclude ICV */
-	u8 crc[4];
 	signed int	length;
 	u32 keylength;
 	u8 *pframe, *payload, *iv, wepkey[16];
@@ -119,10 +118,6 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 		/* decrypt payload include icv */
 		arc4_setkey(ctx, wepkey, 3 + keylength);
 		arc4_crypt(ctx, payload, payload,  length);
-
-		/* calculate icv and compare the icv */
-		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
-
 	}
 }
 
@@ -537,7 +532,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 	u32 pnh;
 	u8   rc4key[16];
 	u8   ttkey[16];
-	u8 crc[4];
+	__le32 crc;
 	signed int			length;
 
 	u8 *pframe, *payload, *iv, *prwskey;
@@ -618,10 +613,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 			arc4_setkey(ctx, rc4key, 16);
 			arc4_crypt(ctx, payload, payload, length);
 
-			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
 
-			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
-			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
+			if (memcmp(&crc, payload + length - 4, 4) != 0)
 				res = _FAIL;
 		} else {
 			res = _FAIL;
-- 
2.32.0


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

* Re: [PATCH v5] rtw_security: fix cast to restricted __le32
  2021-06-21  8:19                 ` [PATCH v5] " Jhih-Ming Huang
  2021-06-21 15:48                   ` [PATCH v6] " Jhih-Ming Huang
@ 2021-06-21 15:51                   ` Jhih-Ming Huang
  2021-06-22  9:31                   ` David Laight
  2 siblings, 0 replies; 19+ messages in thread
From: Jhih-Ming Huang @ 2021-06-21 15:51 UTC (permalink / raw)
  To: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

On Mon, Jun 21, 2021 at 04:19:28PM +0800, Jhih-Ming Huang wrote:
> This patch fixes the sparse warning of fix cast to restricted __le32.
> 
> There was a change for replacing private CRC-32 routines with in kernel
> ones.
> However, the author used le32_to_cpu to convert crc32_le(), and we
> should cpu_to_le32.
> 
> Ths commit also fixes the payload checking by memcmp instead of checking element
> by element and removes the unused variable.
> 
> Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>

I forgot to add the Reported-by tag. 
already added in PATCH v6

thanks.

jmhuang

> ---
>  drivers/staging/rtl8723bs/core/rtw_security.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
> index a99f439328f1..8dc6a976b487 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
> @@ -92,7 +92,6 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
>  void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
>  {
>  	/*  exclude ICV */
> -	u8 crc[4];
>  	signed int	length;
>  	u32 keylength;
>  	u8 *pframe, *payload, *iv, wepkey[16];
> @@ -119,10 +118,6 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
>  		/* decrypt payload include icv */
>  		arc4_setkey(ctx, wepkey, 3 + keylength);
>  		arc4_crypt(ctx, payload, payload,  length);
> -
> -		/* calculate icv and compare the icv */
> -		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
> -
>  	}
>  }
>  
> @@ -537,7 +532,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
>  	u32 pnh;
>  	u8   rc4key[16];
>  	u8   ttkey[16];
> -	u8 crc[4];
> +	__le32 crc;
>  	signed int			length;
>  
>  	u8 *pframe, *payload, *iv, *prwskey;
> @@ -618,10 +613,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
>  			arc4_setkey(ctx, rc4key, 16);
>  			arc4_crypt(ctx, payload, payload, length);
>  
> -			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
> +			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
>  
> -			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
> -			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
> +			if (memcmp(&crc, payload + length - 4, 4) != 0)
>  				res = _FAIL;
>  		} else {
>  			res = _FAIL;
> -- 
> 2.32.0
> 

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

* RE: [PATCH v5] rtw_security: fix cast to restricted __le32
  2021-06-21  8:19                 ` [PATCH v5] " Jhih-Ming Huang
  2021-06-21 15:48                   ` [PATCH v6] " Jhih-Ming Huang
  2021-06-21 15:51                   ` [PATCH v5] " Jhih-Ming Huang
@ 2021-06-22  9:31                   ` David Laight
  2021-07-04 10:31                     ` [PATCH v7] " Jhih-Ming Huang
  2 siblings, 1 reply; 19+ messages in thread
From: David Laight @ 2021-06-22  9:31 UTC (permalink / raw)
  To: 'Jhih-Ming Huang',
	gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel

From: Jhih-Ming Huang
> Sent: 21 June 2021 09:19
> 
> This patch fixes the sparse warning of fix cast to restricted __le32.
> 
> There was a change for replacing private CRC-32 routines with in kernel
> ones.
> However, the author used le32_to_cpu to convert crc32_le(), and we
> should cpu_to_le32.
> 
> Ths commit also fixes the payload checking by memcmp instead of checking element
> by element and removes the unused variable.
...
> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c
> b/drivers/staging/rtl8723bs/core/rtw_security.c
> index a99f439328f1..8dc6a976b487 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
...
> @@ -537,7 +532,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
>  	u32 pnh;
>  	u8   rc4key[16];
>  	u8   ttkey[16];
> -	u8 crc[4];
> +	__le32 crc;
>  	signed int			length;
> 
>  	u8 *pframe, *payload, *iv, *prwskey;
> @@ -618,10 +613,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
>  			arc4_setkey(ctx, rc4key, 16);
>  			arc4_crypt(ctx, payload, payload, length);
> 
> -			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
> +			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
> 
> -			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
> -			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
> +			if (memcmp(&crc, payload + length - 4, 4) != 0)

Shouldn't this be using (IIRC) get_unaligned_le32() ?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


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

* [PATCH v7] rtw_security: fix cast to restricted __le32
  2021-06-22  9:31                   ` David Laight
@ 2021-07-04 10:31                     ` Jhih-Ming Huang
  2021-07-04 19:05                       ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: Jhih-Ming Huang @ 2021-07-04 10:31 UTC (permalink / raw)
  To: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel
  Cc: Jhih-Ming Huang, kernel test robot

This patch fixes the sparse warning of fix cast to restricted __le32.

There was a change for replacing private CRC-32 routines with in kernel
ones.
However, the author used le32_to_cpu to convert crc32_le(), and we
should use cpu_to_le32.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index a99f439328f1..ff79e1aacd1a 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -92,7 +92,6 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
 void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 {
 	/*  exclude ICV */
-	u8 crc[4];
 	signed int	length;
 	u32 keylength;
 	u8 *pframe, *payload, *iv, wepkey[16];
@@ -119,10 +118,6 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 		/* decrypt payload include icv */
 		arc4_setkey(ctx, wepkey, 3 + keylength);
 		arc4_crypt(ctx, payload, payload,  length);
-
-		/* calculate icv and compare the icv */
-		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
-
 	}
 }
 
@@ -537,7 +532,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 	u32 pnh;
 	u8   rc4key[16];
 	u8   ttkey[16];
-	u8 crc[4];
+	__le32 crc;
 	signed int			length;
 
 	u8 *pframe, *payload, *iv, *prwskey;
@@ -618,10 +613,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 			arc4_setkey(ctx, rc4key, 16);
 			arc4_crypt(ctx, payload, payload, length);
 
-			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
 
-			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
-			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
+			if (crc != get_unaligned_le32(payload + length - 4))
 				res = _FAIL;
 		} else {
 			res = _FAIL;
-- 
2.32.0


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

* Re: [PATCH v7] rtw_security: fix cast to restricted __le32
  2021-07-04 10:31                     ` [PATCH v7] " Jhih-Ming Huang
@ 2021-07-04 19:05                       ` Greg KH
  2021-08-01 15:51                         ` Jhih-Ming Huang
  0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2021-07-04 19:05 UTC (permalink / raw)
  To: Jhih-Ming Huang
  Cc: fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel, kernel test robot

On Sun, Jul 04, 2021 at 06:31:12PM +0800, Jhih-Ming Huang wrote:
> This patch fixes the sparse warning of fix cast to restricted __le32.
> 
> There was a change for replacing private CRC-32 routines with in kernel
> ones.
> However, the author used le32_to_cpu to convert crc32_le(), and we
> should use cpu_to_le32.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_security.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
> index a99f439328f1..ff79e1aacd1a 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
> @@ -92,7 +92,6 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
>  void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
>  {
>  	/*  exclude ICV */
> -	u8 crc[4];
>  	signed int	length;
>  	u32 keylength;
>  	u8 *pframe, *payload, *iv, wepkey[16];
> @@ -119,10 +118,6 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
>  		/* decrypt payload include icv */
>  		arc4_setkey(ctx, wepkey, 3 + keylength);
>  		arc4_crypt(ctx, payload, payload,  length);
> -
> -		/* calculate icv and compare the icv */
> -		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
> -
>  	}
>  }
>  
> @@ -537,7 +532,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
>  	u32 pnh;
>  	u8   rc4key[16];
>  	u8   ttkey[16];
> -	u8 crc[4];
> +	__le32 crc;
>  	signed int			length;
>  
>  	u8 *pframe, *payload, *iv, *prwskey;
> @@ -618,10 +613,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
>  			arc4_setkey(ctx, rc4key, 16);
>  			arc4_crypt(ctx, payload, payload, length);
>  
> -			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
> +			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
>  
> -			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
> -			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
> +			if (crc != get_unaligned_le32(payload + length - 4))
>  				res = _FAIL;
>  		} else {
>  			res = _FAIL;
> -- 
> 2.32.0
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/SubmittingPatches for what needs to be done
  here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

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

* [PATCH v7] rtw_security: fix cast to restricted __le32
  2021-07-04 19:05                       ` Greg KH
@ 2021-08-01 15:51                         ` Jhih-Ming Huang
  2021-08-05 11:17                           ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: Jhih-Ming Huang @ 2021-08-01 15:51 UTC (permalink / raw)
  To: gregkh, fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel
  Cc: Jhih-Ming Huang, kernel test robot

This patch fixes the sparse warning of fix cast to restricted __le32.

There was a change for replacing private CRC-32 routines with in kernel
ones.
However, the author used le32_to_cpu to convert crc32_le(), and we
should use cpu_to_le32.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
---
 changes from v6:
   using get_unaligned_le32 to get the value and comparing it with crc,
   instead of using memcmp with raw pointers.

 drivers/staging/rtl8723bs/core/rtw_security.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index a99f439328f1..ff79e1aacd1a 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -92,7 +92,6 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
 void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 {
 	/*  exclude ICV */
-	u8 crc[4];
 	signed int	length;
 	u32 keylength;
 	u8 *pframe, *payload, *iv, wepkey[16];
@@ -119,10 +118,6 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 		/* decrypt payload include icv */
 		arc4_setkey(ctx, wepkey, 3 + keylength);
 		arc4_crypt(ctx, payload, payload,  length);
-
-		/* calculate icv and compare the icv */
-		*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
-
 	}
 }
 
@@ -537,7 +532,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 	u32 pnh;
 	u8   rc4key[16];
 	u8   ttkey[16];
-	u8 crc[4];
+	__le32 crc;
 	signed int			length;
 
 	u8 *pframe, *payload, *iv, *prwskey;
@@ -618,10 +613,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe)
 			arc4_setkey(ctx, rc4key, 16);
 			arc4_crypt(ctx, payload, payload, length);
 
-			*((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4));
+			crc = cpu_to_le32(~crc32_le(~0, payload, length - 4));
 
-			if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] ||
-			    crc[1] != payload[length - 3] || crc[0] != payload[length - 4])
+			if (crc != get_unaligned_le32(payload + length - 4))
 				res = _FAIL;
 		} else {
 			res = _FAIL;
-- 
2.32.0


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

* Re: [PATCH v7] rtw_security: fix cast to restricted __le32
  2021-08-01 15:51                         ` Jhih-Ming Huang
@ 2021-08-05 11:17                           ` Greg KH
  0 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2021-08-05 11:17 UTC (permalink / raw)
  To: Jhih-Ming Huang
  Cc: fabioaiuto83, ross.schm.dev, maqianga, marcocesati,
	linux-staging, linux-kernel, kernel test robot

On Sun, Aug 01, 2021 at 11:51:52PM +0800, Jhih-Ming Huang wrote:
> This patch fixes the sparse warning of fix cast to restricted __le32.
> 
> There was a change for replacing private CRC-32 routines with in kernel
> ones.
> However, the author used le32_to_cpu to convert crc32_le(), and we
> should use cpu_to_le32.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Jhih-Ming Huang <fbihjmeric@gmail.com>
> ---
>  changes from v6:
>    using get_unaligned_le32 to get the value and comparing it with crc,
>    instead of using memcmp with raw pointers.
> 
>  drivers/staging/rtl8723bs/core/rtw_security.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)

Does not apply to the tree :(

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

end of thread, other threads:[~2021-08-05 11:17 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-13 12:28 [PATCH v2] rtw_security: fix cast to restricted __le32 Jhih-Ming Huang
2021-06-13 12:34 ` Greg KH
2021-06-13 16:40   ` Jhih Ming Huang
2021-06-14 14:14     ` Al Viro
2021-06-14 15:27       ` Jhih Ming Huang
2021-06-14 17:03         ` Al Viro
2021-06-18 18:17           ` [PATCH v3] " Jhih-Ming Huang
2021-06-18 19:29             ` Al Viro
2021-06-19  7:52               ` [PATCH v4] " Jhih-Ming Huang
2021-06-21  8:19                 ` [PATCH v5] " Jhih-Ming Huang
2021-06-21 15:48                   ` [PATCH v6] " Jhih-Ming Huang
2021-06-21 15:51                   ` [PATCH v5] " Jhih-Ming Huang
2021-06-22  9:31                   ` David Laight
2021-07-04 10:31                     ` [PATCH v7] " Jhih-Ming Huang
2021-07-04 19:05                       ` Greg KH
2021-08-01 15:51                         ` Jhih-Ming Huang
2021-08-05 11:17                           ` Greg KH
2021-06-19  9:20               ` [PATCH v3] " Jhih-Ming Huang
2021-06-18 18:28           ` [PATCH v2] " Jhih Ming Huang

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox