All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
@ 2020-04-13  3:01 ` Camylla Goncalves Cantanheide
  0 siblings, 0 replies; 18+ messages in thread
From: Camylla Goncalves Cantanheide @ 2020-04-13  3:01 UTC (permalink / raw)
  To: gregkh, navid.emamdoost, sylphrenadin, nishkadg.linux, stephen,
	devel, linux-kernel, lkcamp

Changes of the local variable value and
modification in the seletive repetition structure.

Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_core.c | 52 ++++++++++++--------------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 9b8d85a4855d..87c02aee3854 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
 void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
 	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
 {
-	u32 target_command = 0;
+	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
 	u32 target_content = 0;
 	u16 us_config = 0;
 	u8 i;
@@ -4890,39 +4890,35 @@ void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
 
 	RT_TRACE(COMP_SEC,
 		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
-        	 __func__, dev, entryno, keyindex, keytype, macaddr);
+		 __func__, dev, entryno, keyindex, keytype, macaddr);
 
 	if (defaultkey)
 		us_config |= BIT(15) | (keytype << 2);
 	else
 		us_config |= BIT(15) | (keytype << 2) | keyindex;
 
-	for (i = 0; i < CAM_CONTENT_COUNT; i++) {
-		target_command  = i + CAM_CONTENT_COUNT * entryno;
-		target_command |= BIT(31) | BIT(16);
-
-		if (i == 0) { /* MAC|Config */
-			target_content = (u32)(*(macaddr + 0)) << 16 |
-					(u32)(*(macaddr + 1)) << 24 |
-					(u32)us_config;
-
-			write_nic_dword(dev, WCAMI, target_content);
-			write_nic_dword(dev, RWCAM, target_command);
-		} else if (i == 1) { /* MAC */
-			target_content = (u32)(*(macaddr + 2))	 |
-					(u32)(*(macaddr + 3)) <<  8 |
-					(u32)(*(macaddr + 4)) << 16 |
-					(u32)(*(macaddr + 5)) << 24;
-			write_nic_dword(dev, WCAMI, target_content);
-			write_nic_dword(dev, RWCAM, target_command);
-		} else {
-			/* Key Material */
-			if (keycontent) {
-				write_nic_dword(dev, WCAMI,
-						*(keycontent + i - 2));
-				write_nic_dword(dev, RWCAM, target_command);
-                	}
-		}
+	target_content = macaddr[0] << 16 |
+			 macaddr[0] << 24 |
+			(u32)us_config;
+
+	write_nic_dword(dev, WCAMI, target_content);
+	write_nic_dword(dev, RWCAM, target_command++);
+
+	/* MAC */
+	target_content = macaddr[2]	  |
+			 macaddr[3] <<  8 |
+			 macaddr[4] << 16 |
+			 macaddr[5] << 24;
+	write_nic_dword(dev, WCAMI, target_content);
+	write_nic_dword(dev, RWCAM, target_command++);
+
+	/* Key Material */
+	if (!keycontent)
+		return;
+
+	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
+		write_nic_dword(dev, WCAMI, *keycontent++);
+		write_nic_dword(dev, RWCAM, target_command++);
 	}
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
@ 2020-04-13  3:01 ` Camylla Goncalves Cantanheide
  0 siblings, 0 replies; 18+ messages in thread
From: Camylla Goncalves Cantanheide @ 2020-04-13  3:01 UTC (permalink / raw)
  To: gregkh, navid.emamdoost, sylphrenadin, nishkadg.linux, stephen,
	devel, linux-kernel, lkcamp

Changes of the local variable value and
modification in the seletive repetition structure.

Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_core.c | 52 ++++++++++++--------------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 9b8d85a4855d..87c02aee3854 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
 void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
 	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
 {
-	u32 target_command = 0;
+	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
 	u32 target_content = 0;
 	u16 us_config = 0;
 	u8 i;
@@ -4890,39 +4890,35 @@ void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
 
 	RT_TRACE(COMP_SEC,
 		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
-        	 __func__, dev, entryno, keyindex, keytype, macaddr);
+		 __func__, dev, entryno, keyindex, keytype, macaddr);
 
 	if (defaultkey)
 		us_config |= BIT(15) | (keytype << 2);
 	else
 		us_config |= BIT(15) | (keytype << 2) | keyindex;
 
-	for (i = 0; i < CAM_CONTENT_COUNT; i++) {
-		target_command  = i + CAM_CONTENT_COUNT * entryno;
-		target_command |= BIT(31) | BIT(16);
-
-		if (i == 0) { /* MAC|Config */
-			target_content = (u32)(*(macaddr + 0)) << 16 |
-					(u32)(*(macaddr + 1)) << 24 |
-					(u32)us_config;
-
-			write_nic_dword(dev, WCAMI, target_content);
-			write_nic_dword(dev, RWCAM, target_command);
-		} else if (i == 1) { /* MAC */
-			target_content = (u32)(*(macaddr + 2))	 |
-					(u32)(*(macaddr + 3)) <<  8 |
-					(u32)(*(macaddr + 4)) << 16 |
-					(u32)(*(macaddr + 5)) << 24;
-			write_nic_dword(dev, WCAMI, target_content);
-			write_nic_dword(dev, RWCAM, target_command);
-		} else {
-			/* Key Material */
-			if (keycontent) {
-				write_nic_dword(dev, WCAMI,
-						*(keycontent + i - 2));
-				write_nic_dword(dev, RWCAM, target_command);
-                	}
-		}
+	target_content = macaddr[0] << 16 |
+			 macaddr[0] << 24 |
+			(u32)us_config;
+
+	write_nic_dword(dev, WCAMI, target_content);
+	write_nic_dword(dev, RWCAM, target_command++);
+
+	/* MAC */
+	target_content = macaddr[2]	  |
+			 macaddr[3] <<  8 |
+			 macaddr[4] << 16 |
+			 macaddr[5] << 24;
+	write_nic_dword(dev, WCAMI, target_content);
+	write_nic_dword(dev, RWCAM, target_command++);
+
+	/* Key Material */
+	if (!keycontent)
+		return;
+
+	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
+		write_nic_dword(dev, WCAMI, *keycontent++);
+		write_nic_dword(dev, RWCAM, target_command++);
 	}
 }
 
-- 
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 2/2] staging: rtl8192u: Renames variables in setKey function
  2020-04-13  3:01 ` Camylla Goncalves Cantanheide
@ 2020-04-13  3:01   ` Camylla Goncalves Cantanheide
  -1 siblings, 0 replies; 18+ messages in thread
From: Camylla Goncalves Cantanheide @ 2020-04-13  3:01 UTC (permalink / raw)
  To: gregkh, navid.emamdoost, sylphrenadin, nishkadg.linux, stephen,
	devel, linux-kernel, lkcamp

Renames the local variables of the setKey
function, making them explicit.

Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_core.c | 48 +++++++++++++-------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 87c02aee3854..cc02c3b1eb91 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -4877,48 +4877,48 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
 	write_nic_byte(dev, SECR,  SECR_value);
 }
 
-void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
-	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
+void setKey(struct net_device *dev, u8 entry_no, u8 key_idx, u16 key_type,
+	    u8 *mac_addr, u8 default_key, u32 *key_content)
 {
-	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
-	u32 target_content = 0;
-	u16 us_config = 0;
+	u32 cmd = CAM_CONTENT_COUNT * entry_no |  BIT(31) | BIT(16);
+	u32 content = 0;
+	u16 config = 0;
 	u8 i;
 
-	if (entryno >= TOTAL_CAM_ENTRY)
+	if (entry_no >= TOTAL_CAM_ENTRY)
 		RT_TRACE(COMP_ERR, "cam entry exceeds in %s\n", __func__);
 
 	RT_TRACE(COMP_SEC,
 		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
-		 __func__, dev, entryno, keyindex, keytype, macaddr);
+		 __func__, dev, entry_no, key_idx, key_type, mac_addr);
 
-	if (defaultkey)
-		us_config |= BIT(15) | (keytype << 2);
+	if (default_key)
+		config |= BIT(15) | (key_type << 2);
 	else
-		us_config |= BIT(15) | (keytype << 2) | keyindex;
+		config |= BIT(15) | (key_type << 2) | key_idx;
 
-	target_content = macaddr[0] << 16 |
-			 macaddr[0] << 24 |
-			(u32)us_config;
+	content = mac_addr[0] << 16 |
+		  mac_addr[0] << 24 |
+		 (u32)config;
 
-	write_nic_dword(dev, WCAMI, target_content);
-	write_nic_dword(dev, RWCAM, target_command++);
+	write_nic_dword(dev, WCAMI, content);
+	write_nic_dword(dev, RWCAM, cmd++);
 
 	/* MAC */
-	target_content = macaddr[2]	  |
-			 macaddr[3] <<  8 |
-			 macaddr[4] << 16 |
-			 macaddr[5] << 24;
-	write_nic_dword(dev, WCAMI, target_content);
-	write_nic_dword(dev, RWCAM, target_command++);
+	content = mac_addr[2]	    |
+		  mac_addr[3] <<  8 |
+		  mac_addr[4] << 16 |
+		  mac_addr[5] << 24;
+	write_nic_dword(dev, WCAMI, content);
+	write_nic_dword(dev, RWCAM, cmd++);
 
 	/* Key Material */
-	if (!keycontent)
+	if (!key_content)
 		return;
 
 	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
-		write_nic_dword(dev, WCAMI, *keycontent++);
-		write_nic_dword(dev, RWCAM, target_command++);
+		write_nic_dword(dev, WCAMI, *key_content++);
+		write_nic_dword(dev, RWCAM, cmd++);
 	}
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 2/2] staging: rtl8192u: Renames variables in setKey function
@ 2020-04-13  3:01   ` Camylla Goncalves Cantanheide
  0 siblings, 0 replies; 18+ messages in thread
From: Camylla Goncalves Cantanheide @ 2020-04-13  3:01 UTC (permalink / raw)
  To: gregkh, navid.emamdoost, sylphrenadin, nishkadg.linux, stephen,
	devel, linux-kernel, lkcamp

Renames the local variables of the setKey
function, making them explicit.

Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_core.c | 48 +++++++++++++-------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 87c02aee3854..cc02c3b1eb91 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -4877,48 +4877,48 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
 	write_nic_byte(dev, SECR,  SECR_value);
 }
 
-void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
-	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
+void setKey(struct net_device *dev, u8 entry_no, u8 key_idx, u16 key_type,
+	    u8 *mac_addr, u8 default_key, u32 *key_content)
 {
-	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
-	u32 target_content = 0;
-	u16 us_config = 0;
+	u32 cmd = CAM_CONTENT_COUNT * entry_no |  BIT(31) | BIT(16);
+	u32 content = 0;
+	u16 config = 0;
 	u8 i;
 
-	if (entryno >= TOTAL_CAM_ENTRY)
+	if (entry_no >= TOTAL_CAM_ENTRY)
 		RT_TRACE(COMP_ERR, "cam entry exceeds in %s\n", __func__);
 
 	RT_TRACE(COMP_SEC,
 		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
-		 __func__, dev, entryno, keyindex, keytype, macaddr);
+		 __func__, dev, entry_no, key_idx, key_type, mac_addr);
 
-	if (defaultkey)
-		us_config |= BIT(15) | (keytype << 2);
+	if (default_key)
+		config |= BIT(15) | (key_type << 2);
 	else
-		us_config |= BIT(15) | (keytype << 2) | keyindex;
+		config |= BIT(15) | (key_type << 2) | key_idx;
 
-	target_content = macaddr[0] << 16 |
-			 macaddr[0] << 24 |
-			(u32)us_config;
+	content = mac_addr[0] << 16 |
+		  mac_addr[0] << 24 |
+		 (u32)config;
 
-	write_nic_dword(dev, WCAMI, target_content);
-	write_nic_dword(dev, RWCAM, target_command++);
+	write_nic_dword(dev, WCAMI, content);
+	write_nic_dword(dev, RWCAM, cmd++);
 
 	/* MAC */
-	target_content = macaddr[2]	  |
-			 macaddr[3] <<  8 |
-			 macaddr[4] << 16 |
-			 macaddr[5] << 24;
-	write_nic_dword(dev, WCAMI, target_content);
-	write_nic_dword(dev, RWCAM, target_command++);
+	content = mac_addr[2]	    |
+		  mac_addr[3] <<  8 |
+		  mac_addr[4] << 16 |
+		  mac_addr[5] << 24;
+	write_nic_dword(dev, WCAMI, content);
+	write_nic_dword(dev, RWCAM, cmd++);
 
 	/* Key Material */
-	if (!keycontent)
+	if (!key_content)
 		return;
 
 	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
-		write_nic_dword(dev, WCAMI, *keycontent++);
-		write_nic_dword(dev, RWCAM, target_command++);
+		write_nic_dword(dev, WCAMI, *key_content++);
+		write_nic_dword(dev, RWCAM, cmd++);
 	}
 }
 
-- 
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
  2020-04-13  3:01 ` Camylla Goncalves Cantanheide
@ 2020-04-13 12:50   ` Greg KH
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2020-04-13 12:50 UTC (permalink / raw)
  To: Camylla Goncalves Cantanheide
  Cc: navid.emamdoost, sylphrenadin, nishkadg.linux, stephen, devel,
	linux-kernel, lkcamp

On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> Changes of the local variable value and
> modification in the seletive repetition structure.
> 
> Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
> ---
>  drivers/staging/rtl8192u/r8192U_core.c | 52 ++++++++++++--------------
>  1 file changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index 9b8d85a4855d..87c02aee3854 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
>  void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
>  	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
>  {
> -	u32 target_command = 0;
> +	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
>  	u32 target_content = 0;
>  	u16 us_config = 0;
>  	u8 i;
> @@ -4890,39 +4890,35 @@ void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
>  
>  	RT_TRACE(COMP_SEC,
>  		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
> -        	 __func__, dev, entryno, keyindex, keytype, macaddr);
> +		 __func__, dev, entryno, keyindex, keytype, macaddr);
>  
>  	if (defaultkey)
>  		us_config |= BIT(15) | (keytype << 2);
>  	else
>  		us_config |= BIT(15) | (keytype << 2) | keyindex;
>  
> -	for (i = 0; i < CAM_CONTENT_COUNT; i++) {
> -		target_command  = i + CAM_CONTENT_COUNT * entryno;
> -		target_command |= BIT(31) | BIT(16);
> -
> -		if (i == 0) { /* MAC|Config */
> -			target_content = (u32)(*(macaddr + 0)) << 16 |
> -					(u32)(*(macaddr + 1)) << 24 |
> -					(u32)us_config;
> -
> -			write_nic_dword(dev, WCAMI, target_content);
> -			write_nic_dword(dev, RWCAM, target_command);
> -		} else if (i == 1) { /* MAC */
> -			target_content = (u32)(*(macaddr + 2))	 |
> -					(u32)(*(macaddr + 3)) <<  8 |
> -					(u32)(*(macaddr + 4)) << 16 |
> -					(u32)(*(macaddr + 5)) << 24;
> -			write_nic_dword(dev, WCAMI, target_content);
> -			write_nic_dword(dev, RWCAM, target_command);
> -		} else {
> -			/* Key Material */
> -			if (keycontent) {
> -				write_nic_dword(dev, WCAMI,
> -						*(keycontent + i - 2));
> -				write_nic_dword(dev, RWCAM, target_command);
> -                	}
> -		}
> +	target_content = macaddr[0] << 16 |
> +			 macaddr[0] << 24 |
> +			(u32)us_config;
> +
> +	write_nic_dword(dev, WCAMI, target_content);
> +	write_nic_dword(dev, RWCAM, target_command++);
> +
> +	/* MAC */
> +	target_content = macaddr[2]	  |
> +			 macaddr[3] <<  8 |
> +			 macaddr[4] << 16 |
> +			 macaddr[5] << 24;
> +	write_nic_dword(dev, WCAMI, target_content);
> +	write_nic_dword(dev, RWCAM, target_command++);
> +
> +	/* Key Material */
> +	if (!keycontent)
> +		return;
> +
> +	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
> +		write_nic_dword(dev, WCAMI, *keycontent++);
> +		write_nic_dword(dev, RWCAM, target_command++);
>  	}
>  }
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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:

- Your patch did many different things all at once, making it difficult
  to review.  All Linux kernel patches need to only do one thing at a
  time.  If you need to do multiple things (such as clean up all coding
  style issues in a file/driver), do it in a sequence of patches, each
  one doing only one thing.  This will make it easier to review the
  patches to ensure that they are correct, and to help alleviate any
  merge issues that larger patches can cause.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

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] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
@ 2020-04-13 12:50   ` Greg KH
  0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2020-04-13 12:50 UTC (permalink / raw)
  To: Camylla Goncalves Cantanheide
  Cc: devel, linux-kernel, lkcamp, nishkadg.linux, navid.emamdoost

On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> Changes of the local variable value and
> modification in the seletive repetition structure.
> 
> Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
> ---
>  drivers/staging/rtl8192u/r8192U_core.c | 52 ++++++++++++--------------
>  1 file changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index 9b8d85a4855d..87c02aee3854 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
>  void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
>  	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
>  {
> -	u32 target_command = 0;
> +	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
>  	u32 target_content = 0;
>  	u16 us_config = 0;
>  	u8 i;
> @@ -4890,39 +4890,35 @@ void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
>  
>  	RT_TRACE(COMP_SEC,
>  		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
> -        	 __func__, dev, entryno, keyindex, keytype, macaddr);
> +		 __func__, dev, entryno, keyindex, keytype, macaddr);
>  
>  	if (defaultkey)
>  		us_config |= BIT(15) | (keytype << 2);
>  	else
>  		us_config |= BIT(15) | (keytype << 2) | keyindex;
>  
> -	for (i = 0; i < CAM_CONTENT_COUNT; i++) {
> -		target_command  = i + CAM_CONTENT_COUNT * entryno;
> -		target_command |= BIT(31) | BIT(16);
> -
> -		if (i == 0) { /* MAC|Config */
> -			target_content = (u32)(*(macaddr + 0)) << 16 |
> -					(u32)(*(macaddr + 1)) << 24 |
> -					(u32)us_config;
> -
> -			write_nic_dword(dev, WCAMI, target_content);
> -			write_nic_dword(dev, RWCAM, target_command);
> -		} else if (i == 1) { /* MAC */
> -			target_content = (u32)(*(macaddr + 2))	 |
> -					(u32)(*(macaddr + 3)) <<  8 |
> -					(u32)(*(macaddr + 4)) << 16 |
> -					(u32)(*(macaddr + 5)) << 24;
> -			write_nic_dword(dev, WCAMI, target_content);
> -			write_nic_dword(dev, RWCAM, target_command);
> -		} else {
> -			/* Key Material */
> -			if (keycontent) {
> -				write_nic_dword(dev, WCAMI,
> -						*(keycontent + i - 2));
> -				write_nic_dword(dev, RWCAM, target_command);
> -                	}
> -		}
> +	target_content = macaddr[0] << 16 |
> +			 macaddr[0] << 24 |
> +			(u32)us_config;
> +
> +	write_nic_dword(dev, WCAMI, target_content);
> +	write_nic_dword(dev, RWCAM, target_command++);
> +
> +	/* MAC */
> +	target_content = macaddr[2]	  |
> +			 macaddr[3] <<  8 |
> +			 macaddr[4] << 16 |
> +			 macaddr[5] << 24;
> +	write_nic_dword(dev, WCAMI, target_content);
> +	write_nic_dword(dev, RWCAM, target_command++);
> +
> +	/* Key Material */
> +	if (!keycontent)
> +		return;
> +
> +	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
> +		write_nic_dword(dev, WCAMI, *keycontent++);
> +		write_nic_dword(dev, RWCAM, target_command++);
>  	}
>  }
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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:

- Your patch did many different things all at once, making it difficult
  to review.  All Linux kernel patches need to only do one thing at a
  time.  If you need to do multiple things (such as clean up all coding
  style issues in a file/driver), do it in a sequence of patches, each
  one doing only one thing.  This will make it easier to review the
  patches to ensure that they are correct, and to help alleviate any
  merge issues that larger patches can cause.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] staging: rtl8192u: Renames variables in setKey function
  2020-04-13  3:01   ` Camylla Goncalves Cantanheide
@ 2020-04-13 12:50     ` Greg KH
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2020-04-13 12:50 UTC (permalink / raw)
  To: Camylla Goncalves Cantanheide
  Cc: navid.emamdoost, sylphrenadin, nishkadg.linux, stephen, devel,
	linux-kernel, lkcamp

On Mon, Apr 13, 2020 at 03:01:29AM +0000, Camylla Goncalves Cantanheide wrote:
> Renames the local variables of the setKey
> function, making them explicit.

Why do this?

> 
> Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
> ---
>  drivers/staging/rtl8192u/r8192U_core.c | 48 +++++++++++++-------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index 87c02aee3854..cc02c3b1eb91 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -4877,48 +4877,48 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
>  	write_nic_byte(dev, SECR,  SECR_value);
>  }
>  
> -void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
> -	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
> +void setKey(struct net_device *dev, u8 entry_no, u8 key_idx, u16 key_type,
> +	    u8 *mac_addr, u8 default_key, u32 *key_content)

What was wrong with the original names?  Why add a '_' character for no
good reason?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] staging: rtl8192u: Renames variables in setKey function
@ 2020-04-13 12:50     ` Greg KH
  0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2020-04-13 12:50 UTC (permalink / raw)
  To: Camylla Goncalves Cantanheide
  Cc: devel, linux-kernel, lkcamp, nishkadg.linux, navid.emamdoost

On Mon, Apr 13, 2020 at 03:01:29AM +0000, Camylla Goncalves Cantanheide wrote:
> Renames the local variables of the setKey
> function, making them explicit.

Why do this?

> 
> Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
> ---
>  drivers/staging/rtl8192u/r8192U_core.c | 48 +++++++++++++-------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index 87c02aee3854..cc02c3b1eb91 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -4877,48 +4877,48 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
>  	write_nic_byte(dev, SECR,  SECR_value);
>  }
>  
> -void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
> -	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
> +void setKey(struct net_device *dev, u8 entry_no, u8 key_idx, u16 key_type,
> +	    u8 *mac_addr, u8 default_key, u32 *key_content)

What was wrong with the original names?  Why add a '_' character for no
good reason?

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
  2020-04-13 12:50   ` Greg KH
@ 2020-04-13 15:39     ` Joe Perches
  -1 siblings, 0 replies; 18+ messages in thread
From: Joe Perches @ 2020-04-13 15:39 UTC (permalink / raw)
  To: Greg KH, Camylla Goncalves Cantanheide
  Cc: navid.emamdoost, sylphrenadin, nishkadg.linux, stephen, devel,
	linux-kernel, lkcamp

On Mon, 2020-04-13 at 14:50 +0200, Greg KH wrote:
> On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> > Changes of the local variable value and
> > modification in the seletive repetition structure.
[]
> > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
[]
> You are receiving this message because of the following common error(s)
> as indicated below:
[]
> greg k-h's patch email bot

Hey Greg.

I think I wrote most (all?) of this as a suggestion
to Camylla.

It's a refactoring patch which would be difficult
or impossible to separate into multiple patches.



^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
@ 2020-04-13 15:39     ` Joe Perches
  0 siblings, 0 replies; 18+ messages in thread
From: Joe Perches @ 2020-04-13 15:39 UTC (permalink / raw)
  To: Greg KH, Camylla Goncalves Cantanheide
  Cc: devel, linux-kernel, lkcamp, nishkadg.linux, navid.emamdoost

On Mon, 2020-04-13 at 14:50 +0200, Greg KH wrote:
> On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> > Changes of the local variable value and
> > modification in the seletive repetition structure.
[]
> > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
[]
> You are receiving this message because of the following common error(s)
> as indicated below:
[]
> greg k-h's patch email bot

Hey Greg.

I think I wrote most (all?) of this as a suggestion
to Camylla.

It's a refactoring patch which would be difficult
or impossible to separate into multiple patches.


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
  2020-04-13  3:01 ` Camylla Goncalves Cantanheide
@ 2020-04-14 12:33   ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2020-04-14 12:33 UTC (permalink / raw)
  To: Camylla Goncalves Cantanheide
  Cc: gregkh, navid.emamdoost, sylphrenadin, nishkadg.linux, stephen,
	devel, linux-kernel, lkcamp

On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> Changes of the local variable value and
> modification in the seletive repetition structure.
> 

This changelog isn't totally clear why you're doing this.  Just say:
"I am refactorying setKey() to make it more clear.  I have unrolled the
first two iterations through the loop.  This patch will not change
runtime."

So long as it's clear what you're trying to do and why, that's the
important thing with a commit message.

> Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
> ---
>  drivers/staging/rtl8192u/r8192U_core.c | 52 ++++++++++++--------------
>  1 file changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index 9b8d85a4855d..87c02aee3854 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
>  void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
>  	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
>  {
> -	u32 target_command = 0;
> +	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
>  	u32 target_content = 0;
>  	u16 us_config = 0;
>  	u8 i;
> @@ -4890,39 +4890,35 @@ void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
>  
>  	RT_TRACE(COMP_SEC,
>  		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
> -        	 __func__, dev, entryno, keyindex, keytype, macaddr);
> +		 __func__, dev, entryno, keyindex, keytype, macaddr);

Do this white space change in a separate patch.

>  
>  	if (defaultkey)
>  		us_config |= BIT(15) | (keytype << 2);
>  	else
>  		us_config |= BIT(15) | (keytype << 2) | keyindex;
>  
> -	for (i = 0; i < CAM_CONTENT_COUNT; i++) {
> -		target_command  = i + CAM_CONTENT_COUNT * entryno;
> -		target_command |= BIT(31) | BIT(16);
> -
> -		if (i == 0) { /* MAC|Config */
> -			target_content = (u32)(*(macaddr + 0)) << 16 |
> -					(u32)(*(macaddr + 1)) << 24 |
> -					(u32)us_config;
> -
> -			write_nic_dword(dev, WCAMI, target_content);
> -			write_nic_dword(dev, RWCAM, target_command);
> -		} else if (i == 1) { /* MAC */
> -			target_content = (u32)(*(macaddr + 2))	 |
> -					(u32)(*(macaddr + 3)) <<  8 |
> -					(u32)(*(macaddr + 4)) << 16 |
> -					(u32)(*(macaddr + 5)) << 24;
> -			write_nic_dword(dev, WCAMI, target_content);
> -			write_nic_dword(dev, RWCAM, target_command);
> -		} else {
> -			/* Key Material */
> -			if (keycontent) {
> -				write_nic_dword(dev, WCAMI,
> -						*(keycontent + i - 2));
> -				write_nic_dword(dev, RWCAM, target_command);
> -                	}
> -		}
> +	target_content = macaddr[0] << 16 |
> +			 macaddr[0] << 24 |
> +			(u32)us_config;
> +
> +	write_nic_dword(dev, WCAMI, target_content);
> +	write_nic_dword(dev, RWCAM, target_command++);
> +
> +	/* MAC */
> +	target_content = macaddr[2]	  |
> +			 macaddr[3] <<  8 |
> +			 macaddr[4] << 16 |
> +			 macaddr[5] << 24;
> +	write_nic_dword(dev, WCAMI, target_content);
> +	write_nic_dword(dev, RWCAM, target_command++);
> +
> +	/* Key Material */
> +	if (!keycontent)
> +		return;
> +
> +	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
> +		write_nic_dword(dev, WCAMI, *keycontent++);

This code was wrong in the original as well, but now that I see the bug
let's fix it.  CAM_CONTENT_COUNT is 8.  8 - 2 = 6.  We are writing 6
u32 variables to write_nic_dword().  But the *keycontent buffer only has
4 u32 variables so it is a buffer overflow.

> +		write_nic_dword(dev, RWCAM, target_command++);
>  	}
>  }

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
@ 2020-04-14 12:33   ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2020-04-14 12:33 UTC (permalink / raw)
  To: Camylla Goncalves Cantanheide
  Cc: devel, gregkh, linux-kernel, lkcamp, nishkadg.linux, navid.emamdoost

On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> Changes of the local variable value and
> modification in the seletive repetition structure.
> 

This changelog isn't totally clear why you're doing this.  Just say:
"I am refactorying setKey() to make it more clear.  I have unrolled the
first two iterations through the loop.  This patch will not change
runtime."

So long as it's clear what you're trying to do and why, that's the
important thing with a commit message.

> Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
> ---
>  drivers/staging/rtl8192u/r8192U_core.c | 52 ++++++++++++--------------
>  1 file changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index 9b8d85a4855d..87c02aee3854 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
>  void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
>  	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
>  {
> -	u32 target_command = 0;
> +	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
>  	u32 target_content = 0;
>  	u16 us_config = 0;
>  	u8 i;
> @@ -4890,39 +4890,35 @@ void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
>  
>  	RT_TRACE(COMP_SEC,
>  		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
> -        	 __func__, dev, entryno, keyindex, keytype, macaddr);
> +		 __func__, dev, entryno, keyindex, keytype, macaddr);

Do this white space change in a separate patch.

>  
>  	if (defaultkey)
>  		us_config |= BIT(15) | (keytype << 2);
>  	else
>  		us_config |= BIT(15) | (keytype << 2) | keyindex;
>  
> -	for (i = 0; i < CAM_CONTENT_COUNT; i++) {
> -		target_command  = i + CAM_CONTENT_COUNT * entryno;
> -		target_command |= BIT(31) | BIT(16);
> -
> -		if (i == 0) { /* MAC|Config */
> -			target_content = (u32)(*(macaddr + 0)) << 16 |
> -					(u32)(*(macaddr + 1)) << 24 |
> -					(u32)us_config;
> -
> -			write_nic_dword(dev, WCAMI, target_content);
> -			write_nic_dword(dev, RWCAM, target_command);
> -		} else if (i == 1) { /* MAC */
> -			target_content = (u32)(*(macaddr + 2))	 |
> -					(u32)(*(macaddr + 3)) <<  8 |
> -					(u32)(*(macaddr + 4)) << 16 |
> -					(u32)(*(macaddr + 5)) << 24;
> -			write_nic_dword(dev, WCAMI, target_content);
> -			write_nic_dword(dev, RWCAM, target_command);
> -		} else {
> -			/* Key Material */
> -			if (keycontent) {
> -				write_nic_dword(dev, WCAMI,
> -						*(keycontent + i - 2));
> -				write_nic_dword(dev, RWCAM, target_command);
> -                	}
> -		}
> +	target_content = macaddr[0] << 16 |
> +			 macaddr[0] << 24 |
> +			(u32)us_config;
> +
> +	write_nic_dword(dev, WCAMI, target_content);
> +	write_nic_dword(dev, RWCAM, target_command++);
> +
> +	/* MAC */
> +	target_content = macaddr[2]	  |
> +			 macaddr[3] <<  8 |
> +			 macaddr[4] << 16 |
> +			 macaddr[5] << 24;
> +	write_nic_dword(dev, WCAMI, target_content);
> +	write_nic_dword(dev, RWCAM, target_command++);
> +
> +	/* Key Material */
> +	if (!keycontent)
> +		return;
> +
> +	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
> +		write_nic_dword(dev, WCAMI, *keycontent++);

This code was wrong in the original as well, but now that I see the bug
let's fix it.  CAM_CONTENT_COUNT is 8.  8 - 2 = 6.  We are writing 6
u32 variables to write_nic_dword().  But the *keycontent buffer only has
4 u32 variables so it is a buffer overflow.

> +		write_nic_dword(dev, RWCAM, target_command++);
>  	}
>  }

regards,
dan carpenter


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
  2020-04-14 12:33   ` Dan Carpenter
@ 2020-04-14 16:01     ` Joe Perches
  -1 siblings, 0 replies; 18+ messages in thread
From: Joe Perches @ 2020-04-14 16:01 UTC (permalink / raw)
  To: Dan Carpenter, Camylla Goncalves Cantanheide
  Cc: gregkh, navid.emamdoost, sylphrenadin, nishkadg.linux, stephen,
	devel, linux-kernel, lkcamp

On Tue, 2020-04-14 at 15:33 +0300, Dan Carpenter wrote:
> On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> > Changes of the local variable value and
> > modification in the seletive repetition structure.
> > 
> 
> This changelog isn't totally clear why you're doing this.  Just say:
> "I am refactorying setKey() to make it more clear.  I have unrolled the
> first two iterations through the loop.  This patch will not change
> runtime."
> 
> So long as it's clear what you're trying to do and why, that's the
> important thing with a commit message.
> 
> > Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
> > ---
> >  drivers/staging/rtl8192u/r8192U_core.c | 52 ++++++++++++--------------
> >  1 file changed, 24 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> > index 9b8d85a4855d..87c02aee3854 100644
> > --- a/drivers/staging/rtl8192u/r8192U_core.c
> > +++ b/drivers/staging/rtl8192u/r8192U_core.c
> > @@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
> >  void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
> >  	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
> >  {
> > -	u32 target_command = 0;
> > +	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
> >  	u32 target_content = 0;
> >  	u16 us_config = 0;
> >  	u8 i;
> > @@ -4890,39 +4890,35 @@ void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
> >  
> >  	RT_TRACE(COMP_SEC,
> >  		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
> > -        	 __func__, dev, entryno, keyindex, keytype, macaddr);
> > +		 __func__, dev, entryno, keyindex, keytype, macaddr);
> 
> Do this white space change in a separate patch.
> 
> >  
> >  	if (defaultkey)
> >  		us_config |= BIT(15) | (keytype << 2);
> >  	else
> >  		us_config |= BIT(15) | (keytype << 2) | keyindex;
> >  
> > -	for (i = 0; i < CAM_CONTENT_COUNT; i++) {
> > -		target_command  = i + CAM_CONTENT_COUNT * entryno;
> > -		target_command |= BIT(31) | BIT(16);
> > -
> > -		if (i == 0) { /* MAC|Config */
> > -			target_content = (u32)(*(macaddr + 0)) << 16 |
> > -					(u32)(*(macaddr + 1)) << 24 |
> > -					(u32)us_config;
> > -
> > -			write_nic_dword(dev, WCAMI, target_content);
> > -			write_nic_dword(dev, RWCAM, target_command);
> > -		} else if (i == 1) { /* MAC */
> > -			target_content = (u32)(*(macaddr + 2))	 |
> > -					(u32)(*(macaddr + 3)) <<  8 |
> > -					(u32)(*(macaddr + 4)) << 16 |
> > -					(u32)(*(macaddr + 5)) << 24;
> > -			write_nic_dword(dev, WCAMI, target_content);
> > -			write_nic_dword(dev, RWCAM, target_command);
> > -		} else {
> > -			/* Key Material */
> > -			if (keycontent) {
> > -				write_nic_dword(dev, WCAMI,
> > -						*(keycontent + i - 2));
> > -				write_nic_dword(dev, RWCAM, target_command);
> > -                	}
> > -		}
> > +	target_content = macaddr[0] << 16 |
> > +			 macaddr[0] << 24 |
> > +			(u32)us_config;
> > +
> > +	write_nic_dword(dev, WCAMI, target_content);
> > +	write_nic_dword(dev, RWCAM, target_command++);
> > +
> > +	/* MAC */
> > +	target_content = macaddr[2]	  |
> > +			 macaddr[3] <<  8 |
> > +			 macaddr[4] << 16 |
> > +			 macaddr[5] << 24;
> > +	write_nic_dword(dev, WCAMI, target_content);
> > +	write_nic_dword(dev, RWCAM, target_command++);
> > +
> > +	/* Key Material */
> > +	if (!keycontent)
> > +		return;
> > +
> > +	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
> > +		write_nic_dword(dev, WCAMI, *keycontent++);
> 
> This code was wrong in the original as well, but now that I see the bug
> let's fix it.  CAM_CONTENT_COUNT is 8.  8 - 2 = 6.  We are writing 6
> u32 variables to write_nic_dword().  But the *keycontent buffer only has
> 4 u32 variables so it is a buffer overflow.

Did you find the overflow with smatch?

Perhaps this but it'll write 0's for the
last couple cam entries instead of random
address values.

Dunno what effect that'd have.  Likely none.

---
 drivers/staging/rtl8192u/r8192U_wx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/r8192U_wx.c b/drivers/staging/rtl8192u/r8192U_wx.c
index 100532..923c0c 100644
--- a/drivers/staging/rtl8192u/r8192U_wx.c
+++ b/drivers/staging/rtl8192u/r8192U_wx.c
@@ -733,7 +733,7 @@ static int r8192_wx_set_enc_ext(struct net_device *dev,
 	{
 		u8 broadcast_addr[6] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 		u8 zero[6] = {0};
-		u32 key[4] = {0};
+		u32 key[6] = {0};
 		struct iw_encode_ext *ext = (struct iw_encode_ext *)extra;
 		struct iw_point *encoding = &wrqu->encoding;
 		u8 idx = 0, alg = 0, group = 0;




^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
@ 2020-04-14 16:01     ` Joe Perches
  0 siblings, 0 replies; 18+ messages in thread
From: Joe Perches @ 2020-04-14 16:01 UTC (permalink / raw)
  To: Dan Carpenter, Camylla Goncalves Cantanheide
  Cc: devel, gregkh, linux-kernel, lkcamp, nishkadg.linux, navid.emamdoost

On Tue, 2020-04-14 at 15:33 +0300, Dan Carpenter wrote:
> On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> > Changes of the local variable value and
> > modification in the seletive repetition structure.
> > 
> 
> This changelog isn't totally clear why you're doing this.  Just say:
> "I am refactorying setKey() to make it more clear.  I have unrolled the
> first two iterations through the loop.  This patch will not change
> runtime."
> 
> So long as it's clear what you're trying to do and why, that's the
> important thing with a commit message.
> 
> > Signed-off-by: Camylla Goncalves Cantanheide <c.cantanheide@gmail.com>
> > ---
> >  drivers/staging/rtl8192u/r8192U_core.c | 52 ++++++++++++--------------
> >  1 file changed, 24 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> > index 9b8d85a4855d..87c02aee3854 100644
> > --- a/drivers/staging/rtl8192u/r8192U_core.c
> > +++ b/drivers/staging/rtl8192u/r8192U_core.c
> > @@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
> >  void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
> >  	    u8 *macaddr, u8 defaultkey, u32 *keycontent)
> >  {
> > -	u32 target_command = 0;
> > +	u32 target_command = CAM_CONTENT_COUNT * entryno |  BIT(31) | BIT(16);
> >  	u32 target_content = 0;
> >  	u16 us_config = 0;
> >  	u8 i;
> > @@ -4890,39 +4890,35 @@ void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype,
> >  
> >  	RT_TRACE(COMP_SEC,
> >  		 "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n",
> > -        	 __func__, dev, entryno, keyindex, keytype, macaddr);
> > +		 __func__, dev, entryno, keyindex, keytype, macaddr);
> 
> Do this white space change in a separate patch.
> 
> >  
> >  	if (defaultkey)
> >  		us_config |= BIT(15) | (keytype << 2);
> >  	else
> >  		us_config |= BIT(15) | (keytype << 2) | keyindex;
> >  
> > -	for (i = 0; i < CAM_CONTENT_COUNT; i++) {
> > -		target_command  = i + CAM_CONTENT_COUNT * entryno;
> > -		target_command |= BIT(31) | BIT(16);
> > -
> > -		if (i == 0) { /* MAC|Config */
> > -			target_content = (u32)(*(macaddr + 0)) << 16 |
> > -					(u32)(*(macaddr + 1)) << 24 |
> > -					(u32)us_config;
> > -
> > -			write_nic_dword(dev, WCAMI, target_content);
> > -			write_nic_dword(dev, RWCAM, target_command);
> > -		} else if (i == 1) { /* MAC */
> > -			target_content = (u32)(*(macaddr + 2))	 |
> > -					(u32)(*(macaddr + 3)) <<  8 |
> > -					(u32)(*(macaddr + 4)) << 16 |
> > -					(u32)(*(macaddr + 5)) << 24;
> > -			write_nic_dword(dev, WCAMI, target_content);
> > -			write_nic_dword(dev, RWCAM, target_command);
> > -		} else {
> > -			/* Key Material */
> > -			if (keycontent) {
> > -				write_nic_dword(dev, WCAMI,
> > -						*(keycontent + i - 2));
> > -				write_nic_dword(dev, RWCAM, target_command);
> > -                	}
> > -		}
> > +	target_content = macaddr[0] << 16 |
> > +			 macaddr[0] << 24 |
> > +			(u32)us_config;
> > +
> > +	write_nic_dword(dev, WCAMI, target_content);
> > +	write_nic_dword(dev, RWCAM, target_command++);
> > +
> > +	/* MAC */
> > +	target_content = macaddr[2]	  |
> > +			 macaddr[3] <<  8 |
> > +			 macaddr[4] << 16 |
> > +			 macaddr[5] << 24;
> > +	write_nic_dword(dev, WCAMI, target_content);
> > +	write_nic_dword(dev, RWCAM, target_command++);
> > +
> > +	/* Key Material */
> > +	if (!keycontent)
> > +		return;
> > +
> > +	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
> > +		write_nic_dword(dev, WCAMI, *keycontent++);
> 
> This code was wrong in the original as well, but now that I see the bug
> let's fix it.  CAM_CONTENT_COUNT is 8.  8 - 2 = 6.  We are writing 6
> u32 variables to write_nic_dword().  But the *keycontent buffer only has
> 4 u32 variables so it is a buffer overflow.

Did you find the overflow with smatch?

Perhaps this but it'll write 0's for the
last couple cam entries instead of random
address values.

Dunno what effect that'd have.  Likely none.

---
 drivers/staging/rtl8192u/r8192U_wx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/r8192U_wx.c b/drivers/staging/rtl8192u/r8192U_wx.c
index 100532..923c0c 100644
--- a/drivers/staging/rtl8192u/r8192U_wx.c
+++ b/drivers/staging/rtl8192u/r8192U_wx.c
@@ -733,7 +733,7 @@ static int r8192_wx_set_enc_ext(struct net_device *dev,
 	{
 		u8 broadcast_addr[6] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 		u8 zero[6] = {0};
-		u32 key[4] = {0};
+		u32 key[6] = {0};
 		struct iw_encode_ext *ext = (struct iw_encode_ext *)extra;
 		struct iw_point *encoding = &wrqu->encoding;
 		u8 idx = 0, alg = 0, group = 0;



_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
  2020-04-14 16:01     ` Joe Perches
@ 2020-04-14 18:32       ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2020-04-14 18:32 UTC (permalink / raw)
  To: Joe Perches
  Cc: Camylla Goncalves Cantanheide, gregkh, navid.emamdoost,
	sylphrenadin, nishkadg.linux, stephen, devel, linux-kernel,
	lkcamp

On Tue, Apr 14, 2020 at 09:01:18AM -0700, Joe Perches wrote:
> On Tue, 2020-04-14 at 15:33 +0300, Dan Carpenter wrote:
> > On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> > > +
> > > +	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
> > > +		write_nic_dword(dev, WCAMI, *keycontent++);
> > 
> > This code was wrong in the original as well, but now that I see the bug
> > let's fix it.  CAM_CONTENT_COUNT is 8.  8 - 2 = 6.  We are writing 6
> > u32 variables to write_nic_dword().  But the *keycontent buffer only has
> > 4 u32 variables so it is a buffer overflow.
> 
> Did you find the overflow with smatch?

No.  Smatch isn't smart enough to understand that *(keycontent + i - 2)
is an array overflow.  It thinks *(keycontent + i) is an array overflow
but the "- 2" confuses it.  Also Smatch isn't smart enough to parse the
*keycontent++.  It takes a shortcut when it parses loops.

To be honest, I just didn't like starting the loop from 2 and was trying
to see if there was a better define to use.

I agree that your solution of making the buffer larger is probably the
safest approach given that none of us really know the hardware.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
@ 2020-04-14 18:32       ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2020-04-14 18:32 UTC (permalink / raw)
  To: Joe Perches
  Cc: devel, Camylla Goncalves Cantanheide, gregkh, linux-kernel,
	lkcamp, nishkadg.linux, navid.emamdoost

On Tue, Apr 14, 2020 at 09:01:18AM -0700, Joe Perches wrote:
> On Tue, 2020-04-14 at 15:33 +0300, Dan Carpenter wrote:
> > On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote:
> > > +
> > > +	for (i = 2; i < CAM_CONTENT_COUNT; i++) {
> > > +		write_nic_dword(dev, WCAMI, *keycontent++);
> > 
> > This code was wrong in the original as well, but now that I see the bug
> > let's fix it.  CAM_CONTENT_COUNT is 8.  8 - 2 = 6.  We are writing 6
> > u32 variables to write_nic_dword().  But the *keycontent buffer only has
> > 4 u32 variables so it is a buffer overflow.
> 
> Did you find the overflow with smatch?

No.  Smatch isn't smart enough to understand that *(keycontent + i - 2)
is an array overflow.  It thinks *(keycontent + i) is an array overflow
but the "- 2" confuses it.  Also Smatch isn't smart enough to parse the
*keycontent++.  It takes a shortcut when it parses loops.

To be honest, I just didn't like starting the loop from 2 and was trying
to see if there was a better define to use.

I agree that your solution of making the buffer larger is probably the
safest approach given that none of us really know the hardware.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
       [not found]   ` <CAG3pEr+huVACoP7sTMALYfE46dc+D8DdGPF0ky6EShd4eXD9eg@mail.gmail.com>
@ 2020-04-15  3:09       ` Joe Perches
  0 siblings, 0 replies; 18+ messages in thread
From: Joe Perches @ 2020-04-15  3:09 UTC (permalink / raw)
  To: Camylla Cantanheide, Dan Carpenter
  Cc: Greg Kroah-Hartman, navid.emamdoost, sylphrenadin,
	nishkadg.linux, stephen, devel, LKML, lkcamp

On Tue, 2020-04-14 at 22:18 -0300, Camylla Cantanheide wrote:
> Em ter., 14 de abr. de 2020 às 09:35, Dan Carpenter <
> dan.carpenter@oracle.com> escreveu:
[]
> > > diff --git a/drivers/staging/rtl8192u/r8192U_core.c
[]
> > > +     target_content = macaddr[0] << 16 |
> > > +                      macaddr[0] << 24 |
> > > +                     (u32)us_config;

And Camylla, this statement should be

+     target_content = macaddr[0] << 16 |
+                      macaddr[1] << 24 |
+                     (u32)us_config;

Not a repeated [0]

> > > +
> > > +     write_nic_dword(dev, WCAMI, target_content);
> > > +     write_nic_dword(dev, RWCAM, target_command++);
> > > +
> > > +     /* MAC */
> > > +     target_content = macaddr[2]       |
> > > +                      macaddr[3] <<  8 |
> > > +                      macaddr[4] << 16 |
> > > +                      macaddr[5] << 24;




^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function
@ 2020-04-15  3:09       ` Joe Perches
  0 siblings, 0 replies; 18+ messages in thread
From: Joe Perches @ 2020-04-15  3:09 UTC (permalink / raw)
  To: Camylla Cantanheide, Dan Carpenter
  Cc: devel, Greg Kroah-Hartman, LKML, lkcamp, nishkadg.linux, navid.emamdoost

On Tue, 2020-04-14 at 22:18 -0300, Camylla Cantanheide wrote:
> Em ter., 14 de abr. de 2020 às 09:35, Dan Carpenter <
> dan.carpenter@oracle.com> escreveu:
[]
> > > diff --git a/drivers/staging/rtl8192u/r8192U_core.c
[]
> > > +     target_content = macaddr[0] << 16 |
> > > +                      macaddr[0] << 24 |
> > > +                     (u32)us_config;

And Camylla, this statement should be

+     target_content = macaddr[0] << 16 |
+                      macaddr[1] << 24 |
+                     (u32)us_config;

Not a repeated [0]

> > > +
> > > +     write_nic_dword(dev, WCAMI, target_content);
> > > +     write_nic_dword(dev, RWCAM, target_command++);
> > > +
> > > +     /* MAC */
> > > +     target_content = macaddr[2]       |
> > > +                      macaddr[3] <<  8 |
> > > +                      macaddr[4] << 16 |
> > > +                      macaddr[5] << 24;



_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-04-15  3:17 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-13  3:01 [PATCH 1/2] staging: rtl8192u: Refactoring setKey function Camylla Goncalves Cantanheide
2020-04-13  3:01 ` Camylla Goncalves Cantanheide
2020-04-13  3:01 ` [PATCH 2/2] staging: rtl8192u: Renames variables in " Camylla Goncalves Cantanheide
2020-04-13  3:01   ` Camylla Goncalves Cantanheide
2020-04-13 12:50   ` Greg KH
2020-04-13 12:50     ` Greg KH
2020-04-13 12:50 ` [PATCH 1/2] staging: rtl8192u: Refactoring " Greg KH
2020-04-13 12:50   ` Greg KH
2020-04-13 15:39   ` Joe Perches
2020-04-13 15:39     ` Joe Perches
2020-04-14 12:33 ` Dan Carpenter
2020-04-14 12:33   ` Dan Carpenter
2020-04-14 16:01   ` Joe Perches
2020-04-14 16:01     ` Joe Perches
2020-04-14 18:32     ` Dan Carpenter
2020-04-14 18:32       ` Dan Carpenter
     [not found]   ` <CAG3pEr+huVACoP7sTMALYfE46dc+D8DdGPF0ky6EShd4eXD9eg@mail.gmail.com>
2020-04-15  3:09     ` Joe Perches
2020-04-15  3:09       ` Joe Perches

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.