On 12/06/15 11:29 AM, Emil Velikov wrote:
> Hi Derek,
Hi, thanks for looking at this. :)
> On 1 May 2015 at 18:34, Derek Foreman <der...@osg.samsung.com> wrote:
>> These fds can propagate to child processes if we don't set CLOEXEC,
>> so make a best effort to do that.
>>
>> Signed-off-by: Derek Foreman <der...@osg.samsung.com>
>> ---
>> Noticed this when fixing up similar problems in weston - weston's
>> drm fd gets dup()ed here and loses CLOEXEC and ends up in every child
>> process the shell launches.
>>
>> src/egl/drivers/dri2/platform_drm.c | 14 +++++++++++---
>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_drm.c
>> b/src/egl/drivers/dri2/platform_drm.c
>> index 486b003..c326d6c 100644
>> --- a/src/egl/drivers/dri2/platform_drm.c
>> +++ b/src/egl/drivers/dri2/platform_drm.c
>> @@ -596,7 +596,11 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
>> struct dri2_egl_display *dri2_dpy;
>> struct gbm_device *gbm;
>> int fd = -1;
>> - int i;
>> + int i, flags = O_RDWR;
>> +
>> +#ifdef O_CLOEXEC
>> + flags |= O_CLOEXEC;
>> +#endif
>>
>> loader_set_logger(_eglLog);
>>
>> @@ -611,9 +615,9 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
>> char buf[64];
>> int n = snprintf(buf, sizeof(buf), DRM_DEV_NAME, DRM_DIR_NAME, 0);
>> if (n != -1 && n < sizeof(buf))
>> - fd = open(buf, O_RDWR);
>> + fd = open(buf, flags);
>> if (fd < 0)
>> - fd = open("/dev/dri/card0", O_RDWR);
>> + fd = open("/dev/dri/card0", flags);
>> dri2_dpy->own_device = 1;
>> gbm = gbm_create_device(fd);
>> if (gbm == NULL)
>> @@ -639,6 +643,10 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
>> }
>> }
>>
>> + flags = fcntl(dri2_dpy->fd, F_GETFD);
>> + if (flags >= 0 && !(flags & FD_CLOEXEC))
>> + fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
>> +
> In other places in mesa we explicitly check for EINVAL if open()
> fails, and use that as indication that O_CLOEXEC is not supported
> (note that we only use the O_RDWR and O_CLOEXEC flags). Curious which
> would be the better approach to use through mesa ?
Oops - I think that way is better.
There's a simple function in loader.c called drm_open_device() that does
the open that way, could we just rename that to loader_open_device() and
make it non-static and call it from dri2_initialize_drm()?
That said...
Unfortunately, it appears my previous testing was insufficient and
there's still a leaked fd - dup() creates fds without CLOEXEC set.
I've sent another couple of patches to this effect...
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
