On Fri, Jul 17, 2009 at 11:54:33PM +0200, Michael Buesch wrote:
> I was wondering whether the following patch could make sense.
> The compat ioctl code uses get_user() to fetch all integers, but
> uses __get_user() to fetch the pointer. But I don't see who checked access_ok()
> on the pointer member of the compat struct in userspace.
>
> The native IOCTL does check access_ok() on the pointer (via copy_from_user() on
> the whole struct usbdevfs_ioctl)
>
> What happens to __get_user() if access is not OK? Does it crash? Does it silently return
> and leave udata uninitialized (= initialized with stack junk). Both would be pretty bad.
I'm pretty sure that is up to the compat_ptr() call, right? That
happens right below the __copy_user().
Pete would know best, as he added this code way back in 2005 in commit
c36fc889.
Pete, any thoughts on the patch below?
thanks,
greg k-h
>
> Signed-off-by: Michael Buesch <mb@xxxxxxxxx>
>
> ---
> drivers/usb/core/devio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-2.6.orig/drivers/usb/core/devio.c
> +++ linux-2.6/drivers/usb/core/devio.c
> @@ -1531,21 +1531,21 @@ static int proc_ioctl_default(struct dev
> #ifdef CONFIG_COMPAT
> static int proc_ioctl_compat(struct dev_state *ps, compat_uptr_t arg)
> {
> struct usbdevfs_ioctl32 __user *uioc;
> struct usbdevfs_ioctl ctrl;
> u32 udata;
>
> uioc = compat_ptr((long)arg);
> if (get_user(ctrl.ifno, &uioc->ifno) ||
> get_user(ctrl.ioctl_code, &uioc->ioctl_code) ||
> - __get_user(udata, &uioc->data))
> + get_user(udata, &uioc->data))
> return -EFAULT;
> ctrl.data = compat_ptr(udata);
>
> return proc_ioctl(ps, &ctrl);
> }
> #endif
>
> /*
> * NOTE: All requests here that have interface numbers as parameters
> * are assuming that somehow the configuration has been prevented from
>
> --
> Greetings, Michael.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
