From: Jean Tourrilhes (jt_at_bougret.hpl.hp.com)
Date: 2002-06-21 21:31:20 UTC
On Fri, Jun 21, 2002 at 09:21:44AM +0300, Jouni Malinen wrote:
> On Thu, Jun 20, 2002 at 11:53:45AM -0700, Jean Tourrilhes wrote:
>
> > Is anybody using successfully the following ioctls :
> > wds_add
> > wds_del
> > addmac
> > delmac
> > kickmac
> > The way they are currently implemented, they expect to receive
> > 18 bytes in a memory location containing only 16 bytes. If they work,
> > I'm curious to know how.
>
> Yes, they are used and yes, they seem to work.. Why they are working, is a
> good question.. I don't know.. ;-)
>
> The code came from someone else for *mac ioctls and sinced it worked I did
> not check the buffer size etc. more closely and just
> copied for the wds_* ioctls..
Found the bug. The problem is not a buffer space, the problem is the lack of copy_from_user (== pointer mixup).
wrq->u.data.pointer is a user space pointer, which is valid only in the context of the user level program. You can't use that in kernel space, because it's a different context. Actually, on i386 you can get away with it, but it's bad practice and not portable.
You need to move explicitely the data from user space to kernel space. It should look like :
case PRISM2_IOCTL_WDS_ADD:
if (!capable(CAP_NET_ADMIN)) ret = -EPERM;
else if (wrq->u.data.pointer) {
char addrbuf[18];
if (copy_from_user(addrbuf, wrq->u.data.pointer, 18)) {
ret = -EFAULT;
break;
}
ret = prism2_ioctl_priv_wds(dev, 1, addrbuf);
}
break;
--------------------------------------------
Of course, the new driver API deal with this stuff internally,
which prevent this kind of bug.
You may want to forward this to whoever submitted the patch in
the first place so that he doesn't propagate this error to other
drivers ;-)
Have fun...
Jean