[PATCH 1/2] staging/rtl8192e: Erroneous return codes (types and values)

[PATCH 1/2] staging/rtl8192e: Erroneous return codes (types and values)

[Dominique van den Broeck]

This function returns a bool, that is supposed to be false when something
goes wrong. It's assumed this way by its lone calling function (which is
SetRFPowerState8190(), line 1445 of rtl8192e/rtl8192e/r8192E_phy.c)

Despite of this, this procedure returns non-null enumerations values or
negative codes instead. This patch fixes this.

Signed-off-by: Dominique van den Broeck <domdevlin@free.fr>
---
I submit this patch as a result for Task #16 of the Eudyptula Challenge.

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index c01abc2..a6f2b2b 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -3030,7 +3030,7 @@ bool NicIFEnableNIC(struct net_device *dev)
 		RT_TRACE(COMP_ERR, "ERR!!! %s(): Driver is already down!\n",
 			 __func__);
 		priv->bdisable_nic = false;
-		return RT_STATUS_FAILURE;
+		return false;
 	}
 
 	RT_TRACE(COMP_PS, "===========>%s()\n", __func__);
@@ -3040,7 +3040,7 @@ bool NicIFEnableNIC(struct net_device *dev)
 		RT_TRACE(COMP_ERR, "ERR!!! %s(): initialization is failed!\n",
 			 __func__);
 		priv->bdisable_nic = false;
-		return -1;
+		return false;
 	}
 	RT_TRACE(COMP_INIT, "start adapter finished\n");
 	RT_CLEAR_PS_LEVEL(pPSC, RT_RF_OFF_LEVL_HALT_NIC);

[PATCH 2/2] staging/rtl8192e: userspace ptr deref + incorrect declarations

[Dominique van den Broeck]

. userspace pointer dereference ;
. missing inclusions of needed header files ;
. unrequired static function declaration (confusing another *.c file).

Signed-off-by: Dominique van den Broeck <domdevlin@free.fr>
---
I submit this patch as a result for Task #16 of the Eudyptula Challenge.

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
index 498995d..d87cdfa 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
@@ -17,8 +17,10 @@
  * wlanfae <wlanfae@realtek.com>
 ******************************************************************************/
 
+#include <linux/uaccess.h>
 #include <linux/string.h>
 #include "rtl_core.h"
+#include "rtl_wx.h"
 
 #define RATE_COUNT 12
 static u32 rtl8192_rates[] = {
@@ -1130,11 +1132,18 @@ static int r8192_wx_set_PromiscuousMode(struct net_device *dev,
 	struct r8192_priv *priv = rtllib_priv(dev);
 	struct rtllib_device *ieee = priv->rtllib;
 
-	u32 *info_buf = (u32 *)(wrqu->data.pointer);
+	u32 info_buf[3];
 
-	u32 oid = info_buf[0];
-	u32 bPromiscuousOn = info_buf[1];
-	u32 bFilterSourceStationFrame = info_buf[2];
+	u32 oid;
+	u32 bPromiscuousOn;
+	u32 bFilterSourceStationFrame;
+
+	if (copy_from_user(info_buf, wrqu->data.pointer, sizeof(info_buf)))
+		return -EFAULT;
+
+	oid = info_buf[0];
+	bPromiscuousOn = info_buf[1];
+	bFilterSourceStationFrame = info_buf[2];
 
 	if (OID_RT_INTEL_PROMISCUOUS_MODE == oid) {
 		ieee->IntelPromiscuousModeInfo.bPromiscuousOn =
diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.h b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.h
index 6a51a25..6437664 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.h
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.h
@@ -25,7 +25,6 @@ struct iw_handler_def;
 struct iw_statistics;
 
 extern struct iw_handler_def r8192_wx_handlers_def;
-struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev);
 u16 rtl8192_11n_user_show_rates(struct net_device *dev);
 
 #endif

Re: [PATCH 2/2] staging/rtl8192e: userspace ptr deref + incorrect declarations

[Levente Kurusa]

[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]

Hi,

On Sun, Apr 27, 2014 at 07:11:16PM +0200, Dominique van den Broeck wrote:
> . userspace pointer dereference ;
> . missing inclusions of needed header files ;
> . unrequired static function declaration (confusing another *.c file).
> 
> Signed-off-by: Dominique van den Broeck <domdevlin@free.fr>
> ---
> I submit this patch as a result for Task #16 of the Eudyptula Challenge.
> 
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> index 498995d..d87cdfa 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> @@ -17,8 +17,10 @@
>   * wlanfae <wlanfae@realtek.com>
>  ******************************************************************************/
>  
> +#include <linux/uaccess.h>
>  #include <linux/string.h>
>  #include "rtl_core.h"
> +#include "rtl_wx.h"
>  
>  #define RATE_COUNT 12
>  static u32 rtl8192_rates[] = {
> @@ -1130,11 +1132,18 @@ static int r8192_wx_set_PromiscuousMode(struct net_device *dev,
>  	struct r8192_priv *priv = rtllib_priv(dev);
>  	struct rtllib_device *ieee = priv->rtllib;
>  
> -	u32 *info_buf = (u32 *)(wrqu->data.pointer);
> +	u32 info_buf[3];
>  
> -	u32 oid = info_buf[0];
> -	u32 bPromiscuousOn = info_buf[1];
> -	u32 bFilterSourceStationFrame = info_buf[2];
> +	u32 oid;
> +	u32 bPromiscuousOn;
> +	u32 bFilterSourceStationFrame;
> +
> +	if (copy_from_user(info_buf, wrqu->data.pointer, sizeof(info_buf)))
> +		return -EFAULT;
> +
> +	oid = info_buf[0];
> +	bPromiscuousOn = info_buf[1];
> +	bFilterSourceStationFrame = info_buf[2];

I guess it would be better to have defines for those instead of
hard-coding the offsets. Also the size of the info_buf array
might change depending on the size of wrqu->data.pointer, right?
Maybe create a new define for that as well?

Let's just be safe and create new defines to prevent headaches in
the future, if not for futher expansion then for the sake of
legibility.

Thanks,
Levente Kurusa

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 2/2] staging/rtl8192e: userspace ptr deref + incorrect declarations

[Dominique van den Broeck]

Hello,
Thanks for your comments and reviewing.

> I guess it would be better to have defines for those instead of
> hard-coding the offsets. Also the size of the info_buf array
> might change depending on the size of wrqu->data.pointer, right?
> Maybe create a new define for that as well?
> Let's just be safe and create new defines to prevent headaches in
> the future, if not for futher expansion then for the sake of
> legibility.

As regards those hardcoded indexes, I totally agree.

Still, for this particular patch only, I tried to stick to what
was signaled by sparse. I kept the rest as it was. "info_buf",
directly pointing userspace, and its index numbers are declared
this way by the original code...

I remain totally willing to make it better. :-)

Cheers.

Re: [PATCH 2/2] staging/rtl8192e: userspace ptr deref + incorrect declarations

[Greg Kroah-Hartman]

On Sun, Apr 27, 2014 at 07:11:16PM +0200, Dominique van den Broeck wrote:
> . userspace pointer dereference ;
> . missing inclusions of needed header files ;
> . unrequired static function declaration (confusing another *.c file).
> 
> Signed-off-by: Dominique van den Broeck <domdevlin@free.fr>
> ---

This patch doesn't apply properly for some reason, can you refresh it
and resend?

thanks,

greg k-h