[libcamera-devel] [PATCH] libcamera: Add missing O_CLOEXEC flag

[Kieran Bingham]

Hi Elias,

I would set the $SUBJECT as :

libcamera: v4l2_videodevice: Use O_CLOEXEC when exporting DMA buffers

Quoting Elias Naur via libcamera-devel (2023-05-02 00:47:34)
> Missed in commit 436b38fd89fe4324d2342084ef477a7a8809a001.

It's still good to explain why and what the commit is doing.

We can replicate most of the message from 436b38fd89fe:

I'll propose this:

"""
Files opened internally in libcamera without the O_CLOEXEC file will
remain open upon a call to one of the exec(3) functions. As exec()
doesn't destroy local or global objects, this can lead to various side
effects. Avoid this by opening file descriptors with O_CLOEXEC for all
internal files.

The O_CLOEXEC flag should also be set when obtaining file handles from
the V4L2 VIDIOC_EXPBUF operation, but was missed during the updates to
both d942bdc913c5 ("libcamera: v4l2_device: openat(2) with O_CLOEXEC to
cleanup after exec(3)") and 436b38fd89fe ("libcamera: Open files with
O_CLOEXEC").

Set the O_CLOEXEC flag on calls to ioctl(VIDIOC_EXPBUF).

Fixes: 436b38fd89fe ("libcamera: Open files with O_CLOEXEC")
Signed-off-by: Elias Naur <mail at eliasnaur.com>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
"""

I can update as required when applying if you (and at least one more
reviewer) is happy with that.

--
Kieran


> 
> Signed-off-by: Elias Naur <mail at eliasnaur.com>
> ---
>  src/libcamera/v4l2_videodevice.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 1051997e..a72ef64d 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1471,7 +1471,7 @@ UniqueFD V4L2VideoDevice::exportDmabufFd(unsigned int index,
>         expbuf.type = bufferType_;
>         expbuf.index = index;
>         expbuf.plane = plane;
> -       expbuf.flags = O_RDWR;
> +       expbuf.flags = O_CLOEXEC | O_RDWR;
>  
>         ret = ioctl(VIDIOC_EXPBUF, &expbuf);
>         if (ret < 0) {
> -- 
> 2.39.2
>