[libcamera-devel] [PATCH] libcamera: v4l2_device: openat(2) with O_CLOEXEC to cleanup after exec(3)

[Elias Naur]

Thank you for your reviews. See updated patch[0] that correct the
style issue and elaborates
the problem O_CLOEXEC solves for me.

[0] https://lists.libcamera.org/pipermail/libcamera-devel/2023-March/037100.html

On Mon, 13 Mar 2023 at 02:29, Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Hi Elias, Laurent
>
> On Sun, Mar 12, 2023 at 11:39:30AM +0200, Laurent Pinchart via libcamera-devel wrote:
> > Hi Elias,
> >
> > Thank you for the patch.
> >
> > On Sat, Mar 11, 2023 at 01:05:25PM -0600, Elias Naur via libcamera-devel wrote:
> > > It's generally a good idea to openat(2) with O_CLOEXEC, but this patch
> > > also fixes a problem where the v4l2 devices would report -EBUSY after
> > > an exec(3).
> >
> > Could you elaborate a bit on this -EBUSY problem ?
> >
>
> I also wonder if you got into a real bug or this is just potentially
> dangerous. In example I know "libcamerify" execs, did you get issues
> with the file descriptor being kept open across it ?
>

It's a weird corner-case: a self-updating executable for an appliance-style
deployment of a buildroot-like environment. I hope I explained the problem
better in the v2 patch.

>
> > > Signed-off-by: Elias Naur <mail at eliasnaur.com>
> > > ---
> > >  src/libcamera/v4l2_device.cpp | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git src/libcamera/v4l2_device.cpp src/libcamera/v4l2_device.cpp
> > > index 57a88d96..ad9d1e37 100644
> > > --- src/libcamera/v4l2_device.cpp
> > > +++ src/libcamera/v4l2_device.cpp
> > > @@ -86,7 +86,7 @@ int V4L2Device::open(unsigned int flags)
> > >             return -EBUSY;
> > >     }
> > >
> > > -   UniqueFD fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), flags));
> > > +   UniqueFD fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), flags|O_CLOEXEC));
> >
> > There should be spaces around the '|'. The utils/checkstyle.py script
> > should have reported that. You can automate running checkstyle.py as a
> > git post-commit (or pre-commit, if desired) hook by copying
> > utils/hooks/post-commit to .git/hooks/.
> >
> > There's no need to submit a v2 of this patch just for this, I can fix it
> > locally.
> >
> Anyway, I think this is generally a good idea and with Laurent's
> comment on style addressed at apply time
>

Addressed in v2.