Re: [PATCH v6] mmc: Add mmc CMD+ACMD passthrough ioctl

[Arnd Bergmann <arnd@arndb.de>]

On Wednesday 20 April 2011 21:46:04 Michał Mirosław wrote:
> 2011/4/20 Arnd Bergmann <arnd@arndb.de>:
> > No, please don't try to invent random new ways of doing this.
> > Your example relies on the assumption that the task is calling
> > the entry point for its native word size. Some architectures
> > intentionally allow calling the 32 bit entry point from 64 bit
> > tasks and vice versa, e.g. for user space emulators converting
> > to a different ABI, and in that case is_compat_task() produces
> > the wrong result. Don't ever rely on that.
> 
> This doesn't make sense to me. If you call 32-bit entry point from
> 64-bit process, you can't reliably pass pointers through the call
> (unless you limit 64-bit process to 32-bit address space).
> 
> Do you know a working example of something using this kind of cross-call?

There are people that use 32 bit programs on x86_64 in 64 bit mode
and switch on the ADDR_LIMIT_32BIT personality, IIRC.
This gives you more registers and lets you do 64 bit arithmetic
while not using any more memory to store long pointers.
There are a few problems with this, and the new x32 ABI will make it
cleaner.

I believe qemu also does this to run foreign user binaries. You
can use qemu-user to emulate user space with a different instruction
set, but when you call into the kernel, you have to use the native
data structures that the host understands.

> >> I'm okay with the anon union + ``compat_ptr(*(u32 *))`` part of your
> >> solution.  If everyone else thinks it is reasonable, I'll submit a v7
> >> with it.
> > No need for a union or a ptr_size member in the struct. Just use
> > a single __u64 and let the user cast the pointer to that. This
> > will work on all architectures.
> 
> Union is just hiding this cast (it will be done in kernel) and allows
> cleaner code for userspace (there's a single kernel and possibly
> multiple applications that will implement this call).

As I explained, it doesn't work. Please read my earlier mails.

> >> However, I still think it should be implemented in compat_ioctl()
> >> because compat_blkdev_ioctl() expects it.  Either that, or I add to the
> >> big switch in compat_blkdev_driver_ioctl(), and spreading this change
> >> out to block/compat_ioctl.c does not seem like The Right Thing to me.
> > Yes, you definitely need to fill the .compat_ioctl member. We don't want
> > new entries in the switch statement, in particular none that are specific
> > to a single driver.
> 
> Hmm, you're right. fs/compat_ioctl.c falls back to plain .ioctl if
> .compat_ioctl == NULL.

No, it doesn't.


	Arnd