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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 27 Apr 2014 19:47:53 +0200
From:	Levente Kurusa <levex@...ux.com>
To:	Dominique van den Broeck <domdevlin@...e.fr>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] staging/rtl8192e: userspace ptr deref + incorrect
 declarations

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@...e.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@...ltek.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

Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)