Re: [PATCH 5/7] FUSE: implement ioctl support

[Tejun Heo <tj@kernel.org>]

Hello, Eric.

Eric W. Biederman wrote:
> Implementation wise it is not too bad.
> 
>            FUSE ----------------
> pid = get_pid(task_tid(current))
>             ^                   |
>             |                   |        kernel
>                           pid_vnr(pid)
>    ------ ioctl ----------- /dev/fuse ------------
>             |                   |       userland
>             |                   v
>      ---------------      -------------
>     | caller        |    | FUSE server |---> reads and writes
>     | with tid CTID |    |             |    /proc/PID/task/TID/mem
>      ---------------      -------------

Ah hah.. thanks for the info.

> However it is a largely an insane idea.
> - Write is not implemented for /proc/PID/task/TID/mem
> - It would be better if the kernel handed you back a file descriptor to the
>   other process memory rather than you having to generate one.

Yeah, that would at least ensure we're not meddling with the wrong
address space.

> - To access /proc/PID/task/TID/mem you need to have CAP_PTRACE.
> - This seems to allow for random ioctls.  With the compat_ioctl
>   thing we have largely stomped on that idea.  So you should only
>   need to deal with well defined ioctls.  At which point why do you
>   need to directly access the memory of another process.

What do you mean by "compat_ioctl" thing?  We still allow random
memory accesses to ioctls.

> So why not just only support well defined ioctls and serialize them
> in the kernel and allow the receiving process to deserialize them?

Yeap, that certainly is an option.

> That would allow all of this to happen with a non-privileged server which
> makes the functionality much more useful.

When !allow_others, there's no reason to prevent one user's FUSE
server to corruption the user's processes.  Everyone has the innate
right to shoot him/herself in the foot.

> Given the pain it is to maintain ioctls I would be very surprised if
> we wanted to open up that pandoras box even wider by allowing
> arbitrary user space processes to support random ioctls.  How would
> you do 32/64bit support and the like?

32/64bit is not really much problem tho.  The kernel just flags the
request as-such and let the userland deal with it.

Miklos, I think Eric's reply pretty much blew the direct access idea
out of water, which leaves us with three options.

1. Support only the proper ioctls.  This would be the simplest but
   someone might run into its limitations in the future.  Given that
   the most probable use cases for CUSE are replacing in-kernel legacy
   code or faking some proprietary interfaces which are likely to be
   somewhat weird, the chance isn't too low IMHO.

2. Do what hpa suggested, that is a simple language to describe the
   needed data regions.  To me, it seems like an overkill.  If there's
   already such infrastructure in kernel, I would be happy to
   piggyback on it but separate serialization language for FUSE ioctl
   support seems extreme.

3. And my favorite (obviously), keep the current implementation.  Ugly
   it may look but given the horror of the problem at hand I think it
   actually hits a pretty good balance between interface peculiarness
   and complexity that a more elegant solution would require.  It's
   only ~250 lines of code which can support any ioctl.  Complex ones
   will have to go through a few duplicate copy operations but I don't
   really think those are gonna hurt anyone when the whole thing is
   being relayed to userland.

So, what do you think?

Thanks.

-- 
tejun