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

From: Levente Kurusa
Date: Sun Apr 27 2014 - 13:48:48 EST


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@xxxxxxx>
> ---
> 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@xxxxxxxxxxx>
> ******************************************************************************/
>
> +#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: signature.asc
Description: Digital signature