Re: usb: Patch for USBDEVFS_IOCTL from 32-bit programs

[Mikael Pettersson]

Arjan van de Ven writes:
 > On Tue, 2005-10-18 at 11:49 -0700, Pete Zaitcev wrote:
 > 
 > > 
 > > The problem here is that compat_ptr does NOT turn user data pointer
 > > into a kernel pointer. It's still a user pointer, only sized
 > > differently. So, when you do set_fs(KERNEL_DS), this pointer
 > > is invalid (miraclously, it does work on AMD64, so Dell's tests
 > > pass on their new Xeons).
 > > 
 > > So, you cannot simply to have a small shim. Instead, you have to allocate
 > > the buffer, do copy_from_user(), and then call the ioctl. But then,
 > > it would be a double-copy, when the ioctl allocates the buffer again.
 > > 
 > > I tweaked this in various ways, and the patch I posted looks like
 > > the cleanest solution. But please tell me if I miss something obvious.
 > 
 > 
 > there is one more option; allocate on the user stack space for a 64 bit
 > struct, then copy_in_user() the fields to that and then pass the new
 > pointer to the 64 bit struct to the ioctl.....

Doesn't work and it would break user-space.
Case 1 is when the user stack pointer is at or very near the limit
of the available address space for the user stack, and the next page
is not available for expanding the stack.
Case 2 is when user-space manages its own threads and stacks, using
sigaltstack() to ensure that signal handlers execute elsewhere. Assume
the user stack pointer is at or near the limit of the currently allocated
stack. Having the kernel write to memory beyond the user's stack pointer
can then easily clobber some unrelated user data structure.

So please don't do this.

/Mikael
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/