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.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
