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

2014-07-27 Thread Malcolm Priestley 

On 27/07/14 19:21, Greg Kroah-Hartman wrote:

On Sat, Jul 26, 2014 at 11:44:57AM +0100, Malcolm Priestley wrote:

On 26/07/14 10:18, Guillaume CLÉMENT wrote:

On Sat, Jul 26, 2014 at 10:24:30AM +0200, Guillaume CLÉMENT wrote:

Hi Malcolm,

On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:

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 right.

This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
an example, in device_main.c, we have this code:

rc = iwctl_siwgenie(dev, NULL, &(wrq->u.data), wrq->u.data.pointer);

Here the extra parameter is the last one, wrq->u.data.pointer.

I was led to believe that wrq->u.data.pointer is in userspace (this was
reported by sparse actually) because the pointer field in data is
actually defined as __user.



By the way, the original code (before my patch) reads:

if ((wrq->length < 2) || (extra[1]+2 != wrq->length)) {
ret = -EINVAL;
goto out;
}
if (wrq->length > MAX_WPA_IE_LEN) {
ret = -ENOMEM;
goto out;
}
memset(pMgmt->abyWPAIE, 0, MAX_WPA_IE_LEN);
if (copy_from_user(pMgmt->abyWPAIE, extra, wrq->length)) {
ret = -EFAULT;
goto out;
}

Note extra[1] and later copy_from_user(x, extra, y).

If extra is not in userspace, we should not call copy_from_user, and if
it is, we should not dereference it. There is definitely something fishy
here.



I got it wrong when the iw_handler is not present a standard ioctl is called
extra is in userspace.

Sorry for the noise.


So this patch is acceptable?


Yes, this patch is okay.



confused,

greg k-h



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

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

2014-07-27 Thread Greg Kroah-Hartman 
On Sat, Jul 26, 2014 at 11:44:57AM +0100, Malcolm Priestley wrote:
> On 26/07/14 10:18, Guillaume CLÉMENT wrote:
> >On Sat, Jul 26, 2014 at 10:24:30AM +0200, Guillaume CLÉMENT wrote:
> >>Hi Malcolm,
> >>
> >>On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:
> >>>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 right.
> >>
> >>This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
> >>an example, in device_main.c, we have this code:
> >>
> >>rc = iwctl_siwgenie(dev, NULL, &(wrq->u.data), wrq->u.data.pointer);
> >>
> >>Here the extra parameter is the last one, wrq->u.data.pointer.
> >>
> >>I was led to believe that wrq->u.data.pointer is in userspace (this was
> >>reported by sparse actually) because the pointer field in data is
> >>actually defined as __user.
> >>
> >>
> >By the way, the original code (before my patch) reads:
> >
> > if ((wrq->length < 2) || (extra[1]+2 != wrq->length)) {
> > ret = -EINVAL;
> > goto out;
> > }
> > if (wrq->length > MAX_WPA_IE_LEN) {
> > ret = -ENOMEM;
> > goto out;
> > }
> > memset(pMgmt->abyWPAIE, 0, MAX_WPA_IE_LEN);
> > if (copy_from_user(pMgmt->abyWPAIE, extra, wrq->length)) {
> > ret = -EFAULT;
> > goto out;
> > }
> >
> >Note extra[1] and later copy_from_user(x, extra, y).
> >
> >If extra is not in userspace, we should not call copy_from_user, and if
> >it is, we should not dereference it. There is definitely something fishy
> >here.
> >
> 
> I got it wrong when the iw_handler is not present a standard ioctl is called
> extra is in userspace.
> 
> Sorry for the noise.

So this patch is acceptable?

confused,

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

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

2014-07-27 Thread Greg Kroah-Hartman 
On Sat, Jul 26, 2014 at 11:44:57AM +0100, Malcolm Priestley wrote:
 On 26/07/14 10:18, Guillaume CLÉMENT wrote:
 On Sat, Jul 26, 2014 at 10:24:30AM +0200, Guillaume CLÉMENT wrote:
 Hi Malcolm,
 
 On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:
 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 right.
 
 This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
 an example, in device_main.c, we have this code:
 
 rc = iwctl_siwgenie(dev, NULL, (wrq-u.data), wrq-u.data.pointer);
 
 Here the extra parameter is the last one, wrq-u.data.pointer.
 
 I was led to believe that wrq-u.data.pointer is in userspace (this was
 reported by sparse actually) because the pointer field in data is
 actually defined as __user.
 
 
 By the way, the original code (before my patch) reads:
 
  if ((wrq-length  2) || (extra[1]+2 != wrq-length)) {
  ret = -EINVAL;
  goto out;
  }
  if (wrq-length  MAX_WPA_IE_LEN) {
  ret = -ENOMEM;
  goto out;
  }
  memset(pMgmt-abyWPAIE, 0, MAX_WPA_IE_LEN);
  if (copy_from_user(pMgmt-abyWPAIE, extra, wrq-length)) {
  ret = -EFAULT;
  goto out;
  }
 
 Note extra[1] and later copy_from_user(x, extra, y).
 
 If extra is not in userspace, we should not call copy_from_user, and if
 it is, we should not dereference it. There is definitely something fishy
 here.
 
 
 I got it wrong when the iw_handler is not present a standard ioctl is called
 extra is in userspace.
 
 Sorry for the noise.

So this patch is acceptable?

confused,

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

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

2014-07-27 Thread Malcolm Priestley 

On 27/07/14 19:21, Greg Kroah-Hartman wrote:

On Sat, Jul 26, 2014 at 11:44:57AM +0100, Malcolm Priestley wrote:

On 26/07/14 10:18, Guillaume CLÉMENT wrote:

On Sat, Jul 26, 2014 at 10:24:30AM +0200, Guillaume CLÉMENT wrote:

Hi Malcolm,

On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:

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 right.

This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
an example, in device_main.c, we have this code:

rc = iwctl_siwgenie(dev, NULL, (wrq-u.data), wrq-u.data.pointer);

Here the extra parameter is the last one, wrq-u.data.pointer.

I was led to believe that wrq-u.data.pointer is in userspace (this was
reported by sparse actually) because the pointer field in data is
actually defined as __user.



By the way, the original code (before my patch) reads:

if ((wrq-length  2) || (extra[1]+2 != wrq-length)) {
ret = -EINVAL;
goto out;
}
if (wrq-length  MAX_WPA_IE_LEN) {
ret = -ENOMEM;
goto out;
}
memset(pMgmt-abyWPAIE, 0, MAX_WPA_IE_LEN);
if (copy_from_user(pMgmt-abyWPAIE, extra, wrq-length)) {
ret = -EFAULT;
goto out;
}

Note extra[1] and later copy_from_user(x, extra, y).

If extra is not in userspace, we should not call copy_from_user, and if
it is, we should not dereference it. There is definitely something fishy
here.



I got it wrong when the iw_handler is not present a standard ioctl is called
extra is in userspace.

Sorry for the noise.


So this patch is acceptable?


Yes, this patch is okay.



confused,

greg k-h



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

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

2014-07-26 Thread Malcolm Priestley 

On 26/07/14 10:18, Guillaume CLÉMENT wrote:

On Sat, Jul 26, 2014 at 10:24:30AM +0200, Guillaume CLÉMENT wrote:

Hi Malcolm,

On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:

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 right.

This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
an example, in device_main.c, we have this code:

rc = iwctl_siwgenie(dev, NULL, &(wrq->u.data), wrq->u.data.pointer);

Here the extra parameter is the last one, wrq->u.data.pointer.

I was led to believe that wrq->u.data.pointer is in userspace (this was
reported by sparse actually) because the pointer field in data is
actually defined as __user.



By the way, the original code (before my patch) reads:

if ((wrq->length < 2) || (extra[1]+2 != wrq->length)) {
ret = -EINVAL;
goto out;
}
if (wrq->length > MAX_WPA_IE_LEN) {
ret = -ENOMEM;
goto out;
}
memset(pMgmt->abyWPAIE, 0, MAX_WPA_IE_LEN);
if (copy_from_user(pMgmt->abyWPAIE, extra, wrq->length)) {
ret = -EFAULT;
goto out;
}

Note extra[1] and later copy_from_user(x, extra, y).

If extra is not in userspace, we should not call copy_from_user, and if
it is, we should not dereference it. There is definitely something fishy
here.



I got it wrong when the iw_handler is not present a standard ioctl is 
called extra is in userspace.


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

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

2014-07-26 Thread Guillaume CLÉMENT 
On Sat, Jul 26, 2014 at 10:24:30AM +0200, Guillaume CLÉMENT wrote:
> Hi Malcolm,
>
> On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:
> > 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 right.
>
> This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
> an example, in device_main.c, we have this code:
>
> rc = iwctl_siwgenie(dev, NULL, &(wrq->u.data), wrq->u.data.pointer);
>
> Here the extra parameter is the last one, wrq->u.data.pointer.
>
> I was led to believe that wrq->u.data.pointer is in userspace (this was
> reported by sparse actually) because the pointer field in data is
> actually defined as __user.
>
>
By the way, the original code (before my patch) reads:

if ((wrq->length < 2) || (extra[1]+2 != wrq->length)) {
ret = -EINVAL;
goto out;
}
if (wrq->length > MAX_WPA_IE_LEN) {
ret = -ENOMEM;
goto out;
}
memset(pMgmt->abyWPAIE, 0, MAX_WPA_IE_LEN);
if (copy_from_user(pMgmt->abyWPAIE, extra, wrq->length)) {
ret = -EFAULT;
goto out;
}

Note extra[1] and later copy_from_user(x, extra, y).

If extra is not in userspace, we should not call copy_from_user, and if
it is, we should not dereference it. There is definitely something fishy
here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-07-26 Thread Guillaume CLÉMENT 
Hi Malcolm,

On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:
> 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 right.

This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
an example, in device_main.c, we have this code:

rc = iwctl_siwgenie(dev, NULL, &(wrq->u.data), wrq->u.data.pointer);

Here the extra parameter is the last one, wrq->u.data.pointer.

I was led to believe that wrq->u.data.pointer is in userspace (this was
reported by sparse actually) because the pointer field in data is
actually defined as __user.


I can understand it's not though, if it was pre-processed by another
function. Or if that pointer was never given by userspace in the first
place (if so, why would it be inside a __user pointer) ?


> All Wireless Extensions ioctl extra calls originate from
> ioctl_standard_iw_point in wext-core.
>
> Either through ioctl or iw_handler

After digging into the code a bit more, I think that there may still be
an issue. Are we really going through ioctl_standard_iw_point in this
specific case ?

In the iwctl_handler definition, we have no handler because of:

   (iw_handler) NULL,   // SIOCSIWGENIE


In the wireless_process_ioctl function, the returned handler should be
NULL if I understand correctly. I believe we're going through the "old
API" part with ndo_do_ioctl being called, which is consistent with the
fact that the code I showed earlier comes from that big switch in
device_ioctl in device_main.c.

This means we don't go to ioctl_standard_call, that would have called
ioctl_standard_iw_point, the function that should have done the
copy_from_user.

I didn't see a copy_from_user of the pointer field in the paths that I
think lead to device_ioctl in device_main.c.



Maybe the proper fix should have been to copy the content of
wrq->u.data.pointer to some buffer before calling iwctl_siwgenie, and
let this function not taking __user data?

This way, the driver is still ready to be converted to iw_handler
because it keeps the proper signature.


> All these functions should have been converted to iw_handler.

Yes certainly, but with no access to the real hardware for testing, I'd
rather not make deep changes like this for now.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-07-26 Thread Guillaume CLÉMENT 
Hi Malcolm,

On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:
 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 right.

This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
an example, in device_main.c, we have this code:

rc = iwctl_siwgenie(dev, NULL, (wrq-u.data), wrq-u.data.pointer);

Here the extra parameter is the last one, wrq-u.data.pointer.

I was led to believe that wrq-u.data.pointer is in userspace (this was
reported by sparse actually) because the pointer field in data is
actually defined as __user.


I can understand it's not though, if it was pre-processed by another
function. Or if that pointer was never given by userspace in the first
place (if so, why would it be inside a __user pointer) ?


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

 Either through ioctl or iw_handler

After digging into the code a bit more, I think that there may still be
an issue. Are we really going through ioctl_standard_iw_point in this
specific case ?

In the iwctl_handler definition, we have no handler because of:

   (iw_handler) NULL,   // SIOCSIWGENIE


In the wireless_process_ioctl function, the returned handler should be
NULL if I understand correctly. I believe we're going through the old
API part with ndo_do_ioctl being called, which is consistent with the
fact that the code I showed earlier comes from that big switch in
device_ioctl in device_main.c.

This means we don't go to ioctl_standard_call, that would have called
ioctl_standard_iw_point, the function that should have done the
copy_from_user.

I didn't see a copy_from_user of the pointer field in the paths that I
think lead to device_ioctl in device_main.c.



Maybe the proper fix should have been to copy the content of
wrq-u.data.pointer to some buffer before calling iwctl_siwgenie, and
let this function not taking __user data?

This way, the driver is still ready to be converted to iw_handler
because it keeps the proper signature.


 All these functions should have been converted to iw_handler.

Yes certainly, but with no access to the real hardware for testing, I'd
rather not make deep changes like this for now.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-07-26 Thread Guillaume CLÉMENT 
On Sat, Jul 26, 2014 at 10:24:30AM +0200, Guillaume CLÉMENT wrote:
 Hi Malcolm,

 On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:
  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 right.

 This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
 an example, in device_main.c, we have this code:

 rc = iwctl_siwgenie(dev, NULL, (wrq-u.data), wrq-u.data.pointer);

 Here the extra parameter is the last one, wrq-u.data.pointer.

 I was led to believe that wrq-u.data.pointer is in userspace (this was
 reported by sparse actually) because the pointer field in data is
 actually defined as __user.


By the way, the original code (before my patch) reads:

if ((wrq-length  2) || (extra[1]+2 != wrq-length)) {
ret = -EINVAL;
goto out;
}
if (wrq-length  MAX_WPA_IE_LEN) {
ret = -ENOMEM;
goto out;
}
memset(pMgmt-abyWPAIE, 0, MAX_WPA_IE_LEN);
if (copy_from_user(pMgmt-abyWPAIE, extra, wrq-length)) {
ret = -EFAULT;
goto out;
}

Note extra[1] and later copy_from_user(x, extra, y).

If extra is not in userspace, we should not call copy_from_user, and if
it is, we should not dereference it. There is definitely something fishy
here.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-07-26 Thread Malcolm Priestley 

On 26/07/14 10:18, Guillaume CLÉMENT wrote:

On Sat, Jul 26, 2014 at 10:24:30AM +0200, Guillaume CLÉMENT wrote:

Hi Malcolm,

On Sat, Jul 26, 2014 at 12:09:49AM +0100, Malcolm Priestley wrote:

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 right.

This is still confusing to me because, taking the SIOCSIWGENIE ioctl as
an example, in device_main.c, we have this code:

rc = iwctl_siwgenie(dev, NULL, (wrq-u.data), wrq-u.data.pointer);

Here the extra parameter is the last one, wrq-u.data.pointer.

I was led to believe that wrq-u.data.pointer is in userspace (this was
reported by sparse actually) because the pointer field in data is
actually defined as __user.



By the way, the original code (before my patch) reads:

if ((wrq-length  2) || (extra[1]+2 != wrq-length)) {
ret = -EINVAL;
goto out;
}
if (wrq-length  MAX_WPA_IE_LEN) {
ret = -ENOMEM;
goto out;
}
memset(pMgmt-abyWPAIE, 0, MAX_WPA_IE_LEN);
if (copy_from_user(pMgmt-abyWPAIE, extra, wrq-length)) {
ret = -EFAULT;
goto out;
}

Note extra[1] and later copy_from_user(x, extra, y).

If extra is not in userspace, we should not call copy_from_user, and if
it is, we should not dereference it. There is definitely something fishy
here.



I got it wrong when the iw_handler is not present a standard ioctl is 
called extra is in userspace.


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

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

2014-07-25 Thread 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 
---
  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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = &(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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = &(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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = &(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(, 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 *));
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

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

2014-07-25 Thread 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 
---
  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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = &(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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = &(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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = &(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(, 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 *));
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

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

2014-07-25 Thread Guillaume Clement 
Sparse reported that the data from tagSCmdRequest is given by
userspace, so it should be tagged as such.

Later, we were memcomparing and dereferencing it without first copying
it, fix that as well.

Signed-off-by: Guillaume Clement 
---
 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)
 {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = &(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)
 {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = &(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)
 {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = &(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(, 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 *));
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,  by Einsn Liu
 
-- 
1.8.5.5

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

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

2014-07-25 Thread Guillaume Clement 
Sparse reported that the data from tagSCmdRequest is given by
userspace, so it should be tagged as such.

Later, we were memcomparing and dereferencing it without first copying
it, fix that as well.

Signed-off-by: Guillaume Clement gclem...@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)
 {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = (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)
 {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = (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)
 {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = (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
 
-- 
1.8.5.5

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please

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

2014-07-25 Thread 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 gclem...@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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = (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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = (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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = (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);

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

2014-07-25 Thread 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 gclem...@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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = (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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = (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)
  {
PSDevicepDevice = (PSDevice)netdev_priv(dev);
PSMgmtObjectpMgmt = (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);