All of lore.kernel.org
 help / color / mirror / Atom feed
From: Malcolm Priestley <tvboxspy@gmail.com>
To: Guillaume Clement <gclement@baobob.org>,
	Forest Bond <forest@alittletooquiet.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH] staging: vt6655: fix direct dereferencing of user pointer
Date: Sat, 26 Jul 2014 00:09:49 +0100	[thread overview]
Message-ID: <53D2E3BD.1000208@gmail.com> (raw)
In-Reply-To: <1406292443-11734-1-git-send-email-gclement@baobob.org>

Hi Guillaume

On 25/07/14 13:47, Guillaume Clement wrote:
> Sparse reported that the data from tagSCmdRequest is given by
> userspace, so it should be tagged as such.
extra is not in user space

All Wireless Extensions ioctl extra calls originate from 
ioctl_standard_iw_point in wext-core.

Either through ioctl or iw_handler

All these functions should have been converted to iw_handler.

Regards


Malcolm

>
> Later, we were memcomparing and dereferencing it without first copying
> it, fix that as well.
>
> Signed-off-by: Guillaume Clement <gclement@baobob.org>
> ---
>   drivers/staging/vt6655/iocmd.h |  2 +-
>   drivers/staging/vt6655/iwctl.c | 32 ++++++++++++++++++++++----------
>   drivers/staging/vt6655/iwctl.h |  6 +++---
>   3 files changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/vt6655/iocmd.h b/drivers/staging/vt6655/iocmd.h
> index e499f1b..dd12498 100644
> --- a/drivers/staging/vt6655/iocmd.h
> +++ b/drivers/staging/vt6655/iocmd.h
> @@ -100,7 +100,7 @@ typedef enum tagWZONETYPE {
>   #pragma pack(1)
>   typedef struct tagSCmdRequest {
>   	u8	    name[16];
> -	void	*data;
> +	void __user *data;
>   	u16	    wResult;
>   	u16     wCmdCode;
>   } SCmdRequest, *PSCmdRequest;
> diff --git a/drivers/staging/vt6655/iwctl.c b/drivers/staging/vt6655/iwctl.c
> index 501cd64..7ce23b5 100644
> --- a/drivers/staging/vt6655/iwctl.c
> +++ b/drivers/staging/vt6655/iwctl.c
> @@ -1621,17 +1621,24 @@ int iwctl_giwauth(struct net_device *dev,
>   int iwctl_siwgenie(struct net_device *dev,
>   		   struct iw_request_info *info,
>   		   struct iw_point *wrq,
> -		   char *extra)
> +		   char __user *extra)
>   {
>   	PSDevice			pDevice = (PSDevice)netdev_priv(dev);
>   	PSMgmtObject	pMgmt = &(pDevice->sMgmtObj);
>   	int ret = 0;
> +	char length;
>
>   	if (wrq->length) {
> -		if ((wrq->length < 2) || (extra[1]+2 != wrq->length)) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> +		if (wrq->length < 2)
> +			return -EINVAL;
> +
> +		ret = get_user(length, extra + 1);
> +		if (ret)
> +			return ret;
> +
> +		if (length + 2 != wrq->length)
> +			return -EINVAL;
> +
>   		if (wrq->length > MAX_WPA_IE_LEN) {
>   			ret = -ENOMEM;
>   			goto out;
> @@ -1654,7 +1661,7 @@ out://not completely ...not necessary in wpa_supplicant 0.5.8
>   int iwctl_giwgenie(struct net_device *dev,
>   		   struct iw_request_info *info,
>   		   struct iw_point *wrq,
> -		   char *extra)
> +		   char __user *extra)
>   {
>   	PSDevice			pDevice = (PSDevice)netdev_priv(dev);
>   	PSMgmtObject	pMgmt = &(pDevice->sMgmtObj);
> @@ -1801,18 +1808,23 @@ int iwctl_giwencodeext(struct net_device *dev,
>   int iwctl_siwmlme(struct net_device *dev,
>   		  struct iw_request_info *info,
>   		  struct iw_point *wrq,
> -		  char *extra)
> +		  char __user *extra)
>   {
>   	PSDevice			pDevice = (PSDevice)netdev_priv(dev);
>   	PSMgmtObject	pMgmt = &(pDevice->sMgmtObj);
> -	struct iw_mlme *mlme = (struct iw_mlme *)extra;
> +	struct iw_mlme mime;
> +
>   	int ret = 0;
>
> -	if (memcmp(pMgmt->abyCurrBSSID, mlme->addr.sa_data, ETH_ALEN)) {
> +	ret = copy_from_user(&mime, extra, sizeof(mime));
> +	if (ret)
> +		return -EFAULT;
> +
> +	if (memcmp(pMgmt->abyCurrBSSID, mime.addr.sa_data, ETH_ALEN)) {
>   		ret = -EINVAL;
>   		return ret;
>   	}
> -	switch (mlme->cmd) {
> +	switch (mime.cmd) {
>   	case IW_MLME_DEAUTH:
>   		//this command seems to be not complete,please test it --einsnliu
>   		//bScheduleCommand((void *) pDevice, WLAN_CMD_DEAUTH, (unsigned char *)&reason);
> diff --git a/drivers/staging/vt6655/iwctl.h b/drivers/staging/vt6655/iwctl.h
> index de0a337..7dd6310 100644
> --- a/drivers/staging/vt6655/iwctl.h
> +++ b/drivers/staging/vt6655/iwctl.h
> @@ -176,12 +176,12 @@ int iwctl_giwauth(struct net_device *dev,
>   int iwctl_siwgenie(struct net_device *dev,
>   		   struct iw_request_info *info,
>   		   struct iw_point *wrq,
> -		   char *extra);
> +		   char __user *extra);
>
>   int iwctl_giwgenie(struct net_device *dev,
>   		   struct iw_request_info *info,
>   		   struct iw_point *wrq,
> -		   char *extra);
> +		   char __user *extra);
>
>   int iwctl_siwencodeext(struct net_device *dev,
>   		       struct iw_request_info *info,
> @@ -196,7 +196,7 @@ int iwctl_giwencodeext(struct net_device *dev,
>   int iwctl_siwmlme(struct net_device *dev,
>   		  struct iw_request_info *info,
>   		  struct iw_point *wrq,
> -		  char *extra);
> +		  char __user *extra);
>   #endif // #ifdef WPA_SUPPLICANT_DRIVER_WEXT_SUPPORT
>   //End Add -- //2008-0409-07, <Add> by Einsn Liu
>
>


  reply	other threads:[~2014-07-25 23:10 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-24 23:07 [PATCH] staging: vt6655: tag data as __user in struct tagSCmdRequest Guillaume Clement
2014-07-25 11:52 ` Dan Carpenter
2014-07-25 12:25   ` Guillaume CLÉMENT
2014-07-25 12:33     ` Dan Carpenter
2014-07-25 12:47       ` [PATCH] staging: vt6655: fix direct dereferencing of user pointer Guillaume Clement
2014-07-25 23:09         ` Malcolm Priestley [this message]
2014-07-26  8:24           ` Guillaume CLÉMENT
2014-07-26  9:18             ` Guillaume CLÉMENT
2014-07-26 10:44               ` Malcolm Priestley
2014-07-27 18:21                 ` Greg Kroah-Hartman
2014-07-27 18:44                   ` Malcolm Priestley
2014-07-25 23:16         ` Malcolm Priestley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53D2E3BD.1000208@gmail.com \
    --to=tvboxspy@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=forest@alittletooquiet.net \
    --cc=gclement@baobob.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.