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

[Laurent Pinchart]

On Fri, May 05, 2023 at 09:50:37AM +0100, Kieran Bingham via libcamera-devel wrote:
> 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.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > 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) {

-- 
Regards,

Laurent Pinchart