[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug-diffutils] bug#33965: handling of closed file descriptors

From: Paul Eggert
Subject: [bug-diffutils] bug#33965: handling of closed file descriptors
Date: Sat, 5 Jan 2019 20:06:51 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

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
> ...
         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.

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

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

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

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

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

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

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]