| Message ID | 20230501234734.43869-1-mail@eliasnaur.com |
|---|---|
| State | Accepted |
| Commit | 4c71ec00c2ea5c6acc75e436bf6d24e6861cb66f |
| Headers | show |
| Series |
|
| Related | show |
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@eliasnaur.com> Reviewed-by: Kieran Bingham <kieran.bingham@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@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 >
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@eliasnaur.com> > Reviewed-by: Kieran Bingham <kieran.bingham@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@ideasonboard.com> > > Signed-off-by: Elias Naur <mail@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) {
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) {
Missed in commit 436b38fd89fe4324d2342084ef477a7a8809a001. Signed-off-by: Elias Naur <mail@eliasnaur.com> --- src/libcamera/v4l2_videodevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)