Re: [PATCH] staging: vt6655: fix direct dereferencing of user pointer

[Malcolm Priestley]

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@xxxxxxxxxx>
> ---
>   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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/