Re: [PATCH 2/3] staging: rtl8723bs: Use existing arc4 implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 12/04/2021 à 11:34, Greg KH a écrit :
On Sat, Apr 10, 2021 at 03:35:52PM +0200, Christophe JAILLET wrote:
Use functions provided by <crypto/arc4.h> instead of hand writing them.

The implementations are slightly different, but are equivalent. It has
been checked with a test program which compares the output of the 2 sets of
functions.

Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
---
  drivers/staging/rtl8723bs/core/rtw_security.c | 101 ++++--------------
  1 file changed, 21 insertions(+), 80 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index 9587d89a6b24..86949a65e96f 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -6,6 +6,7 @@
   ******************************************************************************/
  #define  _RTW_SECURITY_C_
+#include <crypto/arc4.h>
  #include <linux/crc32poly.h>
  #include <drv_types.h>
  #include <rtw_debug.h>
@@ -31,66 +32,6 @@ const char *security_type_str(u8 value)
/* WEP related ===== */ -struct arc4context {
-	u32 x;
-	u32 y;
-	u8 state[256];
-};
-
-
-static void arcfour_init(struct arc4context	*parc4ctx, u8 *key, u32 key_len)
-{
-	u32 t, u;
-	u32 keyindex;
-	u32 stateindex;
-	u8 *state;
-	u32 counter;
-
-	state = parc4ctx->state;
-	parc4ctx->x = 0;
-	parc4ctx->y = 0;
-	for (counter = 0; counter < 256; counter++)
-		state[counter] = (u8)counter;
-	keyindex = 0;
-	stateindex = 0;
-	for (counter = 0; counter < 256; counter++) {
-		t = state[counter];
-		stateindex = (stateindex + key[keyindex] + t) & 0xff;
-		u = state[stateindex];
-		state[stateindex] = (u8)t;
-		state[counter] = (u8)u;
-		if (++keyindex >= key_len)
-			keyindex = 0;
-	}
-}
-
-static u32 arcfour_byte(struct arc4context	*parc4ctx)
-{
-	u32 x;
-	u32 y;
-	u32 sx, sy;
-	u8 *state;
-
-	state = parc4ctx->state;
-	x = (parc4ctx->x + 1) & 0xff;
-	sx = state[x];
-	y = (sx + parc4ctx->y) & 0xff;
-	sy = state[y];
-	parc4ctx->x = x;
-	parc4ctx->y = y;
-	state[y] = (u8)sx;
-	state[x] = (u8)sy;
-	return state[(sx + sy) & 0xff];
-}
-
-static void arcfour_encrypt(struct arc4context *parc4ctx, u8 *dest, u8 *src, u32 len)
-{
-	u32 i;
-
-	for (i = 0; i < len; i++)
-		dest[i] = src[i] ^ (unsigned char)arcfour_byte(parc4ctx);
-}
-
  static signed int bcrc32initialized;
  static u32 crc32_table[256];
@@ -150,7 +91,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
  {																	/*  exclude ICV */
unsigned char crc[4];
-	struct arc4context	 mycontext;
+	struct arc4_ctx mycontext;

Are you sure you can declare that much space on the stack?  Is that what
other users of this api do?  In looking at the in-kernel users, they do
not :(


In fact arc4context was a u8[256] but arc4_ctx uses a u32[256].

Maybe arc4 function should be modified to use u8? Or at least when
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is defined?



Concerning the other uses of the arc4 API, in fact there is only few of them. Most uses are buried into lib80211_crypt_tkip.c and lib80211_crypt_wep.c which is the way to go, IMHO. In fact, I think that most, if not all 'rtl871x_security.c' could be removed.

However, it looks like a big step to me. And without any hardware to test, I'm pretty sure to break something.


Any hints on the best way to axe some duplicated code without to much risks to break everything?

CJ

Can you fix up this series to not take up so much stack memory?

I guess that _adapter could be used to store this arc4_ctx, but I'm not convinced that it is the way to go.

CJ


thanks,

greg k-h






[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux