Greg KH <[EMAIL PROTECTED]> wrote:
>
> On Fri, Jun 02, 2006 at 03:46:52PM -0700, Andrew Morton wrote:
> > Greg KH <[EMAIL PROTECTED]> wrote:
> > >
> > > On Thu, Jun 01, 2006 at 08:48:46PM -0700, [EMAIL PROTECTED] wrote:
> > > >
> > > > From: Philippe Retornaz <[EMAIL PROTECTED]>
> > > >
> > > > See http://bugzilla.kernel.org/show_bug.cgi?id=6617.
> > > >
> > > > This function dereference a __user pointer.
> > > >
> > > > (akpm: this code is deeply fishy. Are the types correct?)
> > > >
> > > > Signed-off-by: Philippe Retornaz <[EMAIL PROTECTED]>
> > > > Cc: Greg KH <[EMAIL PROTECTED]>
> > > > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
> > > > ---
> > > >
> > > > drivers/usb/core/devio.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff -puN
> > > > drivers/usb/core/devio.c~drivers-usb-core-devioc-dereference-userspace-pointer
> > > > drivers/usb/core/devio.c
> > > > ---
> > > > devel/drivers/usb/core/devio.c~drivers-usb-core-devioc-dereference-userspace-pointer
> > > > 2006-06-01 20:48:09.000000000 -0700
> > > > +++ devel-akpm/drivers/usb/core/devio.c 2006-06-01 20:48:09.000000000
> > > > -0700
> > > > @@ -1079,7 +1079,9 @@ static int proc_submiturb(struct dev_sta
> > > > if (copy_from_user(&uurb, arg, sizeof(uurb)))
> > > > return -EFAULT;
> > > >
> > > > - return proc_do_submiturb(ps, &uurb, (((struct usbdevfs_urb
> > > > __user *)arg)->iso_frame_desc), arg);
> > > > + return proc_do_submiturb(ps, &uurb,
> > > > + (struct usbdevfs_iso_packet_desc __user
> > > > *)uurb.iso_frame_desc,
> > > > + arg);
> > > > }
> > >
> > > This doesn't do anything, or solve any problem, so I'm going to drop it.
> > >
> >
> > Are you sure? `arg' is a userspace pointer and this:
> >
> > proc_do_submiturb(ps, &uurb, (((struct usbdevfs_urb __user
> > *)arg)->iso_frame_desc), arg);
> >
> > directly dereferences it, whereas this:
> >
> >
> > (struct usbdevfs_iso_packet_desc __user *)uurb.iso_frame_desc,
> >
> > instead uses the copy which we obtained via copy_from_user.
>
> Either way it's still a userspace pointer (the data itself didn't change
> in the copy_from_user), and we are using that data to pass to that
> function.
Yes, it's a userspace pointer. But in the present code, we're reading that
*pointer*'s value direct from userspace.
But after the patch, we're reading that pointer-to-userspace from kernel
memory.
It's the difference between:
reference((struct foo __user *)arg);
and
copy_from_user(tmp, arg, sizeof(arg));
reference((struct foo *)tmp);
After doing that, we pass this userspace pointer into proc_do_submiturb(),
which handles it.
I think. ;)
> Unless on some arches we can't walk into a userspace structure?
That's OK, as long as we use the appropriate uaccess functions.
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
