[PATCH] dma-buf: document fd flags and O_CLOEXEC requirement

[PATCH] dma-buf: document fd flags and O_CLOEXEC requirement

[Rob Clark]

From: Rob Clark <rob@ti.com>

Otherwise subsystems will get this wrong and end up with a second
export ioctl with the flag and O_CLOEXEC support added.

Signed-off-by: Rob Clark <rob@ti.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
Updated version of Daniel's original documentation patch with (hopefully)
improved wording, and a better description of the motivation.

 Documentation/dma-buf-sharing.txt |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt
index 225f96d..3b51134 100644
--- a/Documentation/dma-buf-sharing.txt
+++ b/Documentation/dma-buf-sharing.txt
@@ -223,6 +223,24 @@ Miscellaneous notes:
 - Any exporters or users of the dma-buf buffer sharing framework must have
   a 'select DMA_SHARED_BUFFER' in their respective Kconfigs.
 
+- In order to avoid fd leaks on exec, the FD_CLOEXEC flag must be set
+  on the file descriptor.  This is not just a resource leak, but a
+  potential security hole.  It could give the newly exec'd application
+  access to buffers, via the leaked fd, to which it should otherwise
+  not be permitted access.
+
+  The problem with doing this via a separate fcntl() call, versus doing it
+  atomically when the fd is created, is that this is inherently racy in a
+  multi-threaded app[3].  The issue is made worse when it is library code
+  opening/creating the file descriptor, as the application may not even be
+  aware of the fd's.
+
+  To avoid this problem, userspace must have a way to request O_CLOEXEC
+  flag be set when the dma-buf fd is created.  So any API provided by
+  the exporting driver to create a dmabuf fd must provide a way to let
+  userspace control setting of O_CLOEXEC flag passed in to dma_buf_fd().
+
 References:
 [1] struct dma_buf_ops in include/linux/dma-buf.h
 [2] All interfaces mentioned above defined in include/linux/dma-buf.h
+[3] https://lwn.net/Articles/236486/
-- 
1.7.5.4

Re: [PATCH] dma-buf: document fd flags and O_CLOEXEC requirement

[Sumit Semwal]

On 20 March 2012 03:12, Rob Clark <rob.clark@linaro.org> wrote:
> From: Rob Clark <rob@ti.com>
>
> Otherwise subsystems will get this wrong and end up with a second
> export ioctl with the flag and O_CLOEXEC support added.
>
> Signed-off-by: Rob Clark <rob@ti.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> Updated version of Daniel's original documentation patch with (hopefully)
> improved wording, and a better description of the motivation.
Thanks; applied this in place of Daniel's to for-next.
>
BR,
~Sumit.

Re: [Linaro-mm-sig] [PATCH 4/4] dma-buf: document fd flags and O_CLOEXEC requirement

[Dave Airlie]

On Sun, Mar 18, 2012 at 11:34 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Otherwise subsystems will get this wrong and end up with and second
> export ioctl with the flag and O_CLOEXEC support added.

Its not actually dma_buf_export that takes the O_CLOEXEC flag its dma_buf_fd

I'm not sure how blindly we should be passing flags in from userspace
to these, like O_NONBLOCK or perms flags.

Dave.

[PATCH] dma-buf: document fd flags and O_CLOEXEC requirement

[Daniel Vetter]

Otherwise subsystems will get this wrong and end up with a second
export ioctl with the flag and O_CLOEXEC support added.

v2: Fixup the function name and caution exporters to limit the flags
to only O_CLOEXEC. Noted by Dave Airlie.

Cc: Dave Airlie <airlied@gmail.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/dma-buf-sharing.txt |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt
index 9f3aeef..a6d4c37 100644
--- a/Documentation/dma-buf-sharing.txt
+++ b/Documentation/dma-buf-sharing.txt
@@ -319,6 +319,12 @@ Miscellaneous notes
 - Any exporters or users of the dma-buf buffer sharing framework must have
   a 'select DMA_SHARED_BUFFER' in their respective Kconfigs.
 
+- To avoid the wrath of userspace library writers exporting subsystems must have
+  a flag parameter in the ioctl that creates the dma-buf fd which needs to
+  support at least the O_CLOEXEC fd flag. This needs to be passed in the flag
+  parameter of dma_buf_fd. Without any other reasons applying it is recommended
+  that exporters limit the flags passed to dma_buf_fd to only O_CLOEXEC.
+
 References:
 [1] struct dma_buf_ops in include/linux/dma-buf.h
 [2] All interfaces mentioned above defined in include/linux/dma-buf.h
-- 
1.7.7.5

Re: [PATCH] dma-buf: document fd flags and O_CLOEXEC requirement

[Ville Syrjälä]

On Mon, Mar 19, 2012 at 04:41:55PM +0100, Daniel Vetter wrote:
> Otherwise subsystems will get this wrong and end up with a second
> export ioctl with the flag and O_CLOEXEC support added.
> 
> v2: Fixup the function name and caution exporters to limit the flags
> to only O_CLOEXEC. Noted by Dave Airlie.
> 
> Cc: Dave Airlie <airlied@gmail.com>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/dma-buf-sharing.txt |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt
> index 9f3aeef..a6d4c37 100644
> --- a/Documentation/dma-buf-sharing.txt
> +++ b/Documentation/dma-buf-sharing.txt
> @@ -319,6 +319,12 @@ Miscellaneous notes
>  - Any exporters or users of the dma-buf buffer sharing framework must have
>    a 'select DMA_SHARED_BUFFER' in their respective Kconfigs.
>  
> +- To avoid the wrath of userspace library writers exporting subsystems must have
> +  a flag parameter in the ioctl that creates the dma-buf fd which needs to
> +  support at least the O_CLOEXEC fd flag. This needs to be passed in the flag
> +  parameter of dma_buf_fd. Without any other reasons applying it is recommended
> +  that exporters limit the flags passed to dma_buf_fd to only O_CLOEXEC.

Difficult to parse. Needs more punctuation.

-- 
Ville Syrjälä
Intel OTC