Re: [PATCH] open: introduce O_NOSTD

[Eric Blake <ebb9@byu.net>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Florian Weimer on 8/27/2009 8:35 AM:
> * Eric Blake:
> 
>> int open_safer (const char *name, int flags, int mode)
>> {
>>   int fd = open (name, flags | O_CLOEXEC, mode);
>>   if (0 <= fd && fd <= 2)
>>     {
>>       int dup = fcntl (fd, ((flags & O_CLOEXEC)
>>                             ? F_DUPFD_CLOEXEC : F_DUPFD), 3);
>>       int saved_errno = errno;
>>       close (fd);
>>       errno = saved_errno;
>>       fd = dup;
>>     }
>>   else if (!(flags & O_CLOEXEC))
>>     {
>>       if ((flags = fcntl (fd, F_GETFD)) < 0
>>           || fcntl (fd, F_SETFD, flags & ~FD_CLOEXEC) == -1)
>>         {
>>           int saved_errno = errno;
>>           close (fd);
>>           fd = -1;
>>           errno = saved_errno;
>>         }
>>     }
>>   return fd;
>> }
> 
>> This solves the fd leak,
> 
> It's still buggy.

In what manner?  Are you stating that F_DUPFD_CLOEXEC is not atomic?

>  You need something like this:
> 
> int open_safer(const char *name, int flags, int mode)
> {
>   int opened_fd[3] = {0, 0, 0};
>   int fd, i, errno_saved;
>   while (1) {
>     fd = open(name, flags | O_CLOEXEC, mode);
>     if (fd < 0 || fd > 2) {
>       break;
>     }
>     opened_fd[fd] = 1;
>   }
>   for (int i = 0; i <= 2; ++i) {
>     if (opened_fd[i]) {
>       errno_saved = errno;
>       close(i);
>       errno = errno_saved;
>     }
>   }
>   return fd;
> }
> 
> It's untested, so it's probably still buggy.

Your version fails to clear the cloexec bit of the final fd if the
original caller didn't request O_CLOEXEC.  If the caller requested
O_CLOEXEC, then your version takes 3, 5, or 7 syscalls depending on how
many std fds were closed, while my version takes 3 syscalls regardless of
how many std fds were closed.  Also, your suggestion has a definite race
in that you are calling open() multiple times rather than cloning an
existing fd after the first open(), such that another process could alter
which file is visited between your first and last open().

- --
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@byu.net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkqX0W0ACgkQ84KuGfSFAYDYdwCeOB8dt0Rx2QYJkfIsfP452AYj
V7QAoL1FACwnRPhhQ2aTh2EB38i+d34o
=ouPs
-----END PGP SIGNATURE-----