Thread (6 messages) 6 messages, 2 authors, 2021-07-24

Re: [PATCH v3 1/2] staging: rtl8188eu: Replace a custom function with crc32_le()

From: Fabio M. De Francesco <hidden>
Date: 2021-07-23 17:40:41
Also in: lkml

Hi David,

This driver is going to be replaced by a "better" version, so I'm not sure 
whether or not this patch is still needed.

However, I see that we have similar problems in rtl8723bs and perhaps also in 
other drivers. Therefore, I'd like to solve this problem, whatever will happen 
to the "worse" rti8188eu, and change the code where else it needs to be 
changed.

Now I have a few questions... 

On Thursday, July 22, 2021 5:30:08 PM CEST David Laight wrote:
From: Fabio M. De Francesco
quoted
Sent: 21 July 2021 12:01

Use crc32_le() in place of the custom getcrc32().
...
quoted
@@ -609,14 +595,15 @@ u32	rtw_tkip_encrypt(struct adapter *padapter, 
struct xmit_frame
quoted
*pxmitframe)

 				if ((curfragnum + 1) == pattrib-
nr_frags) {	/* 4 the last fragment */
quoted
 				
 					length = pattrib-
last_txcmdsz - pattrib->hdrlen - pattrib->iv_len -
quoted
pattrib->icv_len;
-					*((__le32 *)crc) = 
getcrc32(payload, length);/* modified by Amy*/
quoted
+					*((__le32 *)crc) = 
cpu_to_le32(~crc32_le(~0, payload, length));
quoted
 					
arcfour_init(&mycontext, rc4key, 16);
quoted
 					
arcfour_encrypt(&mycontext, payload, payload, length);
quoted
 					
arcfour_encrypt(&mycontext, payload + length, crc, 4);
quoted
 				
 				} else {
 				
 					length = pxmitpriv-
frag_len - pattrib->hdrlen - pattrib->iv_len -
quoted
pattrib->icv_len;
-					*((__le32 *)crc) = 
getcrc32(payload, length);/* modified by Amy*/
quoted
+					*((__le32 *)crc) = 
cpu_to_le32(~crc32_le(~0, payload, length));
quoted
+

 					
arcfour_init(&mycontext, rc4key, 16);
quoted
 					
arcfour_encrypt(&mycontext, payload, payload, length);
quoted
 					
arcfour_encrypt(&mycontext, payload + length, crc, 4);
Change crc to be __le32, kill the casts and pass &crc in the last call.
I could do it, but the last call (that to arcfour_encrypt() takes a pointer to 
u8 type as the third parameter. How can I use a __le32 for that?

I think that one possible solution is to change crc to be an union and use two 
fields in the following lines:

union {
	__le32 f0; /* More descriptive name? */
	u8 f1[4]; /* More descriptive name? */
} crc;

[...]

crc.f0 = cpu_to_le32(~crc32_le(~0, payload, length - 4));

[...]

arcfour_encrypt(&mycontext, payload + length, crc.f1, 4);

Please, tell me... What about the solution above?
quoted
@@ -682,7 +669,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, struct 
recv_frame
quoted
*precvframe)> 
 			arcfour_init(&mycontext, rc4key, 16);
 			arcfour_encrypt(&mycontext, payload, 
payload, length);
quoted
-			*((__le32 *)crc) = getcrc32(payload, length 
- 4);
quoted
+			*((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, 
payload, length - 4));
quoted
 			if (crc[3] != payload[length - 1] ||
 			
 			    crc[2] != payload[length - 2] ||
You could to the same here, or make crc u32, remove the cpu_to_le32()
and use get_unaligned_u32(payload + length - 4) (or whatever it is called).
Sorry, I can't understand this line. Can you please elaborate it a bit more?
But it is much better to do:
	crc = crc32_le(~0, payload, length);
	if (crc != VALID_CRC32)
		res = _FAIL;
Why "crc = crc32_le(~0, payload, length);"? Shouldn't it be "crc = 
cpu_to_le32(~crc32_le(~0, patload, length);"? 

Why did you drop both the cpu_to_le32() call and the '~' operator?

Thanks in advance,

Fabio
You can lookup VALID_CRC32, but it is crc32_le(0, "\xff\xff\xff\xff", 4);

	David

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


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help