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

[Jacopo Mondi]

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 ?


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

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

> >  	if (!fd.isValid()) {
> >  		int ret = -errno;
> >  		LOG(V4L2, Error) << "Failed to open V4L2 device '"
>
> --
> Regards,
>
> Laurent Pinchart