Re: bug#33965: handling of closed file descriptors

[Paul Eggert]

Bruno Haible wrote:

> > The feature is not portable to POSIX
> > platforms. Besides, I doubt whether anybody is using the feature anyway.
>
> OK, if you question the "feature" as a whole,

Sorry, I was referring only to the never-used feature of diffutils, where "-" in 
a command-line argument stands for a missing file if stdin is closed (only in 
some cases, e.g., -N). I think nobody uses this feature or will miss it if it's 
removed, and anyway it can't work on some POSIX-conforming platforms. I put the 
feature into diffutils in many years ago, but it's now clear that the test case 
that is exercising it is a mistake and that the feature itself was mistaken.

But to broaden this to the more-general issue of whether kernels should force 
stdin/stdout/stderr to be open after an exec:

>    I predict that this "simple change to the kernel" will not make it into
>    the majority of the operating systems in the next 10 years. (But you
>    could start lobbying for it among the OpenBSD people. They would be the
>    most likely to adopt it, I guess.)

OpenBSD has already adopted it for setuid and setgid executables, as has 
FreeBSD, NetBSD, and SELinux. I imagine this feature is reasonably common 
elsewhere, at least for setuid (I haven't checked carefully). As far as I know 
only HP-UX does it for non-setuid/gid executables.

>    In the mean time, the remaining options I can see are:
>
>      (a) Keep using the *-safer Gnulib modules.
>
>      (b) Introduce a sanity_check_file_descriptors() function
> ...
>          void
>          sanity_check_file_descriptors (void)
>          {
>            int fd;
>            for (fd = 0; fd <= 2; fd++)
>              if (fcntl (fd, F_GETFD, NULL) < 0 && errno == EBADF)
>                exit (125);
>          }
> ... 
>    (b) surely is simpler than (a).

I suggest instead a third option (c) that was formerly used in Coreutils, which 
is that the program makes sure that stdin/stdout/stderr are open before doing 
anything that creates a file descriptor. As I understand it Coreutils moved away 
from that approach primarily because it was redundant if the (earlier) *-safer 
modules are also used. However, in hindsight perhaps Coreutils should have 
stopped using the *-safer modules and just used method (c); it's a lot simpler 
and less intrusive.

I resurrected and refreshed that old Coreutils code and put it into a new Gnulib 
module stdopen, adjusted Gnulib's xfreopen module in a couple of minor ways 
(only diffutils uses xfreopen as far as I know) so that it no longer requires 
freopen-safer, and installed the first four attached patches into Gnulib. I then 
propagated this into diffutils and removed the unportable feature by installing 
the last three attached patches into diffutils.

As a result of these changes, diffutils no longer uses the *-safer modules (it 
uses the new stdopen module instead), it no longer supports the unportable 
feature in question, and it no longer mishandles some obscure situations where 
stdin, stdout, or stderr are closed.

0001-stdopen-copy-from-last-use-in-coreutils.patch
Description: Text Data

0002-stdopen-modernize-and-simplify.patch
Description: Text Data

0003-xfreopen-need-not-depend-on-freopen-safer.patch
Description: Text Data

0004-xfreopen-need-not-include-stdio-.h.patch
Description: Text Data

0001-build-update-gnulib-submodule-to-latest.patch
Description: Text Data

0002-diff-remove-unportable-diff-N-f-feature.patch
Description: Text Data

0003-diff-fix-cmp-diff3-sdiff-with-stdin-closed.patch
Description: Text Data