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

From: Malcolm Priestley
Date: Sun Jul 27 2014 - 14:44:19 EST

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->, wrq->;

Here the extra parameter is the last one, wrq->

I was led to believe that wrq-> 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

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.


greg k-h

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at