On 04/21/2011 08:43 PM, Rebecca Schultz Zavin wrote:
On Thu, Apr 21, 2011 at 5:55 AM, Marcus Lorentzon <marcus.xm.lorentzon@stericsson.com> wrote:
On 04/21/2011 02:15 PM, Arnd Bergmann wrote:
On Wednesday 20 April 2011, Marcus Lorentzon wrote:
 
On 04/20/2011 06:19 PM, Arnd Bergmann wrote:

* Established ways to pass them around

     
What could be easier than passing an int? I just don't like the
"feature" of passing fds where they are dup-ed without driver knowing
so. If you want to store process specific info associated with the fd
you have to use a list since you don't know if the app sends the fd to
another process. Probably not a big deal, but I don't like it ;)
   
The problem with passing an integer is that it doesn't have any
concept of ownership or lifetime rules.
 

The idea of passing global ids should not affect lifetime. These ints are still lifetime controlled by the "fd" device that created them. So I think they do have a defined lifetime, that of whatever device this int is registered in. And if the process is shut down, all buffers registered in the drm/v4l2 device fd will be released and even freed if it was the last ref.
And if you mean lifetime while process is still alive, even fds has to be closed, and ints has to be closed/unregistered using ioctl. And this is not something that is used by applications either, these refs and allocs are handled by the user space drivers, like libEGL / libGL / X-drivers etc, so ioctl vs. close should not matter.

The problem isn't managing their lifetime in the side that created the buffer, it's managing it while they are in flight.  What happens if process 1 passes a buffer to process 2 and before process 2 takes a reference to it, process 1 crashes?  Some central clearing house has to handle that.  I'm guessing that's the X server in the X case.  In my proposal that's handled by the extra reference being held by the passed fd itself (ie the kernel has a reference as long as the file struct exists in either processes file descriptor table).  

I see no advantage in process 2 getting a reference to the buffer before process 1 crash. Process 1 could crash just be fore it sent it too, giving same issue. Doesn't seem to solve any real problem. And if process 1 & 2 are communicating, one crashes, then this is probably the least important problem. And the case with global ids I was trying to propose doesn't have any issue in this case either. The global id doesn't hold a reference. Only the device local ids hold references, and the global id is valid until all local handles are released. So the creator can control lifetime of global id even after it is sent, or until another process has imported the global id, creating its own local handle/ref. Since the local handles are device local, they will be automatically released upon device close (or process crash).

But, not that I'm not saying we Can't do fds. I'm just saying it just a few lines of extra code to have both. I know fds are easy in Android due to Binder. But for all other IPC use cases handling buffer ids specially is not that simple (having to use spacial pipe functions for example, which might not be available in all IPC protocols).
So, if you have device local ids, exportable to Either fd or global id, then both models will be supported (see HWMEM for example, supporting DRM & Android).

Another case hard to solve using fd security model is Media process -> Application -> "Flinger" passing of DRM (Digital Rights Management) buffers. For example if you have to extend the media framework to playback buffers applications can "see". Then fds will still give full rights to any process the buffer passes through (Unless you start using EGL streams ;). But global ids with explicit buffer rights (Read / Write / Import) will allow buffers to be passed through "unsecure" processes. Even the media process could be considered unsecure if media decoder is in kernel (accepting global ids as buffer references at decode, resolving these in kernel).

 

If you allow any process access to all integers, a malicious process
might be able to guess it unless you use long cryptographic random
numbers.

 
That's why you put a security model on top. Like GEM auth (which only have access all or nothing) or something like HWMEM where each buffer/id has read/write/import rights per process. This is also easier to trace/debug security since driver is notified when a buffer is transfered to another process. You never get this info from binder/pipe (dup-ed).

It's totally trivial to have debug info on what buffers are currently mapped into what processes.  The kernel knows where all the memory manager's file descriptors have gone.  This is already implemented in the proposal I posted.  From userspace security becomes really simple, a process owns all the buffers it's created and any that have been shared with it.  If it doesn't want to share a buffer with another process, it doesn't pass it to it. 

Unless you don't want to give access to intermediate process as in the use case above.
 

When you have a file descriptor, you can assume that the object
is still alive until you close it. With an integer passed by some
other application, that is less clear.

 
That's why I prefer the register/import global id step with device. It gives the driver a chance to store meta data and prepare to use this buffer. If this has to be done for every device ioctl call, you loose efficiency and all device APIs would have to be updated with cloned ops for fds. Register/import an fd/globalid and then use device local handles is much more efficient and don't require API changes, only additions.

That's exactly what I'm proposing, you import an fd that's been passed to you.

And that is fine, as long as you support global ids too for those without Binder ;). Or at least start out with an attempt to map your requirements on GEM or something else already solving most of the issues. Even if Android decide to take their own route again, Linaro has as main target to upstream everything. And not making an attempt to merge with "GEM" or what already exist upstream will probably only make that job even harder than merge the ARM world where every one currently have their own "pmem".

/BR
/Marcus