For the almost 4-year anniversary: O_CLOEXEC again

For the almost 4-year anniversary: O_CLOEXEC again

[Ulrich Drepper]

The last time I commented about this was almost exactly 4 years ago.
And the problem is still not resolved.

The situation is this:

       thread 1                           thread 2

       fd = open(...)

                                          fork()

       fcntl(fd, F_SETFD, FD_CLOEXEC)

                                          execve()


Multi-threaded apps can use fork().  Requiring theese use of fork() to
syncronize with every other thread and every piece of code which they
use wrt to opening files is impossible.  Likewise impossible is it to
change the default to FD_CLOEXEC by default.

Some (vocal) people requested to always use separate processes when
security sensitive files are opened, processes which then can be single
threaded.  I find this hardly acceptable especially since it just
introduces additional problems.  Just think about opendir() which just
opens the directory using open().

The proposed solution is simple and already implemented on some systems
(QNX, BeOS, maybe more).  Beside the definition of O_CLOEXEC the
untested patch below should be all that's needed.  It does *not* solve
the related problem of socket descriptors etc.  I've no good idea for
those situations.  It cannot be done through a per-thread switch which
decides whether newly allocated descriptors are CLOEXEC until further
notice since open() is a signal-safe interface.  The signal handler code
might not be aware of this mode and create descriptors which are not
inherited.  At least this little change can handle open().

I sincerely hope that this is not again answered with curses on POSIX
and its authors.  POSIX is here to stay and we should do whatever is
possible to fix it.


--- fs/open.c   2004-03-13 21:16:11.000000000 -0800
+++ fs/open.c-ud        2004-03-28 15:43:23.000000000 -0800
@@ -938,11 +938,14 @@ asmlinkage long sys_open(const char __us
        if (!IS_ERR(tmp)) {
                fd = get_unused_fd();
                if (fd >= 0) {
-                       struct file *f = filp_open(tmp, flags, mode);
+                       struct file *f = filp_open(tmp, flags & ~O_CLOEXEC,
+                                                  mode);
                        error = PTR_ERR(f);
                        if (IS_ERR(f))
                                goto out_error;
                        fd_install(fd, f);
+                       if (flags & O_CLOEXEC)
+                               set_close_on_exec(fd, 1);
                }
 out:
                putname(tmp);


-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

Re: For the almost 4-year anniversary: O_CLOEXEC again

[Davide Libenzi]

On Sun, 28 Mar 2004, Ulrich Drepper wrote:

> The proposed solution is simple and already implemented on some systems
> (QNX, BeOS, maybe more).  Beside the definition of O_CLOEXEC the
> untested patch below should be all that's needed.

What does prevent a fork() to hit you right before set_close_on_exec()?
Shouldn't we have an install-and-set?

(Personally I manually handle the fork child's code in my MT apps, when 
I'm going to exec untrusted apps. Manually here mean close all open fds > 2)



- Davide

Re: For the almost 4-year anniversary: O_CLOEXEC again

[dean gaudet]

On Sun, 28 Mar 2004, Ulrich Drepper wrote:

> The proposed solution is simple and already implemented on some systems
> (QNX, BeOS, maybe more).  Beside the definition of O_CLOEXEC the
> untested patch below should be all that's needed.  It does *not* solve
> the related problem of socket descriptors etc.  I've no good idea for

we could create new socket, socketpair, accept, and pipe, system calls
with an extra flags argument.  plus some similar solution to account for
dup/dup2/F_DUPFD as well.

how many other fd creating syscalls did i miss? :)

-dean

Re: For the almost 4-year anniversary: O_CLOEXEC again

[Jamie Lokier]

Ulrich Drepper wrote:
> It cannot be done through a per-thread switch which decides whether
> newly allocated descriptors are CLOEXEC until further notice since
> open() is a signal-safe interface.  The signal handler code might
> not be aware of this mode and create descriptors which are not
> inherited.

Since O_CLOEXEC is non-portable, why not implement the per-thread
switch?

Signal handlers that call open() will have to be aware of it, but
that's ok: an application will only set the switch if it knows what
that implies.

For POSIX compatibility the switch should be off initially, and only
turned on at the application's request.

Portable code which assumes POSIX interfaces has to have another way
to work around the close-on-exec problem anyway.

Overall I like the O_CLOEXEC flag.  It really should have always been
an O_* flag, and F_SETFL should be able to change it.

The only problem is there are so many other ways to get file
descriptors which it doesn't help with.  Most especially sockets,
which are heavily used in the kinds of programs which use threads and
and forking.

Btw, is open() the only signal-safe interface where this matters?
Perhaps a per-thread flag could be used for all the _other_ file
descriptor generating interfaces, but not for open() where only
O_CLOEXEC would work?

-- Jamie

Re: For the almost 4-year anniversary: O_CLOEXEC again

[Ulrich Drepper]

Jamie Lokier wrote:

> Since O_CLOEXEC is non-portable, why not implement the per-thread
> switch?
> 
> Signal handlers that call open() will have to be aware of it, but
> that's ok: an application will only set the switch if it knows what
> that implies.

It's not OK since library functions can install signal handlers and they
need not be aware of everything the application does.

Any globally visible change in the behavior can have negative effects.

Yes, there are other calls but open() is far more critical since it is
now promoted not only to an interface to actually do work.  We are
supported to read kernel and system informatino from /proc which
requires open().

And no, oen() is not the only signal-safe function.  If you don't have
the POSIX specs handy, look at a recent signal(2) man page which lists
the signal-safe functions.  accept(), socket() are on the list among others.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖