* [PATCH] Ensure standard file descriptors are open on start
@ 2020-08-18 17:50 Arsen Arsenović
2020-08-18 20:11 ` Paul Eggert
0 siblings, 1 reply; 5+ messages in thread
From: Arsen Arsenović @ 2020-08-18 17:50 UTC (permalink / raw)
To: libc-alpha
ISO C requires that standard input, output and error are always open on
program startup.
---
Prior to these changes, a program could be launched that has no standard input,
output or error stream. This is in conflict with the C and POSIX standards, and
causes some programs (such as autoconf configure scripts) to fail. This behavior
can be easily replicated using a "wrapper" program such as:
#include <unistd.h>
int main(int argc, char **argv) {
close(0);
execvp(argv[1], argv + 1);
}
Launching cat via ./close_stdin cat will, without these changes, result in an
EBADF, but will silently exit (as it is reading /dev/null) with these changes.
The latter behavior imitates what the C standard requires.
csu/check_fds.c | 10 +++++-----
csu/libc-start.c | 9 +++------
elf/dl-sysdep.c | 7 ++-----
3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/csu/check_fds.c b/csu/check_fds.c
index 30634b81..f0f88268 100644
--- a/csu/check_fds.c
+++ b/csu/check_fds.c
@@ -58,8 +58,8 @@ check_one_fd (int fd, int mode)
}
/* Something is wrong with this descriptor, it's probably not
- opened. Open /dev/null so that the SUID program we are
- about to start does not accidentally use this descriptor. */
+ opened. Open /dev/null so that the program we are about to
+ start does not accidentally use this descriptor. */
int nullfd = __open_nocancel (name, mode, 0);
/* We are very paranoid here. With all means we try to ensure
@@ -90,7 +90,7 @@ __libc_check_standard_fds (void)
is really paranoid but some people actually are. If /dev/null
should happen to be a symlink to somewhere else and not the
device commonly known as "/dev/null" we bail out. */
- check_one_fd (STDIN_FILENO, O_WRONLY | O_NOFOLLOW);
- check_one_fd (STDOUT_FILENO, O_RDONLY | O_NOFOLLOW);
- check_one_fd (STDERR_FILENO, O_RDONLY | O_NOFOLLOW);
+ check_one_fd (STDIN_FILENO, O_RDONLY | O_NOFOLLOW);
+ check_one_fd (STDOUT_FILENO, O_WRONLY | O_NOFOLLOW);
+ check_one_fd (STDERR_FILENO, O_WRONLY | O_NOFOLLOW);
}
diff --git a/csu/libc-start.c b/csu/libc-start.c
index 4005caf8..f99efda0 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -253,12 +253,9 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
if (fini)
__cxa_atexit ((void (*) (void *)) fini, NULL, NULL);
- /* Some security at this point. Prevent starting a SUID binary where
- the standard file descriptors are not opened. We have to do this
- only for statically linked applications since otherwise the dynamic
- loader did the work already. */
- if (__builtin_expect (__libc_enable_secure, 0))
- __libc_check_standard_fds ();
+ /* Ensure the standard streams are opened, as required by POSIX and C. For
+ dynamic programs this is already handled in the dynamic loader. */
+ __libc_check_standard_fds ();
#endif
/* Call the initializer of the program, if any. */
diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
index 85457082..83070413 100644
--- a/elf/dl-sysdep.c
+++ b/elf/dl-sysdep.c
@@ -243,11 +243,8 @@ _dl_sysdep_start (void **start_argptr,
__sbrk (GLRO(dl_pagesize)
- ((_end - (char *) 0) & (GLRO(dl_pagesize) - 1)));
- /* If this is a SUID program we make sure that FDs 0, 1, and 2 are
- allocated. If necessary we are doing it ourself. If it is not
- possible we stop the program. */
- if (__builtin_expect (__libc_enable_secure, 0))
- __libc_check_standard_fds ();
+ /* Ensure all the standard streams are open (C and POSIX require this) */
+ __libc_check_standard_fds ();
(*dl_main) (phdr, phnum, &user_entry, GLRO(dl_auxv));
return user_entry;
--
2.26.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Ensure standard file descriptors are open on start
2020-08-18 17:50 [PATCH] Ensure standard file descriptors are open on start Arsen Arsenović
@ 2020-08-18 20:11 ` Paul Eggert
2020-08-18 22:31 ` Arsen Arsenović
0 siblings, 1 reply; 5+ messages in thread
From: Paul Eggert @ 2020-08-18 20:11 UTC (permalink / raw)
To: Arsen Arsenović; +Cc: libc-alpha
On 8/18/20 10:50 AM, Arsen Arsenović via Libc-alpha wrote:
> - check_one_fd (STDIN_FILENO, O_WRONLY | O_NOFOLLOW);
> - check_one_fd (STDOUT_FILENO, O_RDONLY | O_NOFOLLOW);
> - check_one_fd (STDERR_FILENO, O_RDONLY | O_NOFOLLOW);
> + check_one_fd (STDIN_FILENO, O_RDONLY | O_NOFOLLOW);
> + check_one_fd (STDOUT_FILENO, O_WRONLY | O_NOFOLLOW);
> + check_one_fd (STDERR_FILENO, O_WRONLY | O_NOFOLLOW);
Why is this change needed? Even if ISO C requires that standard streams be open,
it doesn't require that one can do I/O successfully with the streams.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Ensure standard file descriptors are open on start
2020-08-18 20:11 ` Paul Eggert
@ 2020-08-18 22:31 ` Arsen Arsenović
2020-08-18 22:37 ` Paul Eggert
0 siblings, 1 reply; 5+ messages in thread
From: Arsen Arsenović @ 2020-08-18 22:31 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]
> > - check_one_fd (STDIN_FILENO, O_WRONLY | O_NOFOLLOW);
> > - check_one_fd (STDOUT_FILENO, O_RDONLY | O_NOFOLLOW);
> > - check_one_fd (STDERR_FILENO, O_RDONLY | O_NOFOLLOW);
> > + check_one_fd (STDIN_FILENO, O_RDONLY | O_NOFOLLOW);
> > + check_one_fd (STDOUT_FILENO, O_WRONLY | O_NOFOLLOW);
> > + check_one_fd (STDERR_FILENO, O_WRONLY | O_NOFOLLOW);
>
> Why is this change needed? Even if ISO C requires that standard streams be
> open, it doesn't require that one can do I/O successfully with the streams.
It seems to me that the sanest option would be making all three file descriptors
read/wrote only (as appropriate), by opening /dev/null. This makes all reads
EOFs and all writes successful, which would be what, at least, I expect when
closing one of the standard streams.
Regardless, with and without this change, the configure script I mentioned still
runs fine (since the standard streams it rightfully assumes exist indeed do
exist), so I can remove it, if that's a better solution.
--
Arsen Arsenović
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Ensure standard file descriptors are open on start
2020-08-18 22:31 ` Arsen Arsenović
@ 2020-08-18 22:37 ` Paul Eggert
2020-08-18 23:28 ` Arsen Arsenović
0 siblings, 1 reply; 5+ messages in thread
From: Paul Eggert @ 2020-08-18 22:37 UTC (permalink / raw)
To: Arsen Arsenović; +Cc: libc-alpha
On 8/18/20 3:31 PM, Arsen Arsenović wrote:
> This makes all reads EOFs and all writes successful, which would be what,
> at least, I expect when closing one of the standard streams.
That's not what I expect. There is a benefit for a program like 'cat' to
report an error (instead of silently succeeding) if its invoker closed stdin
or stdout. That's why glibc behaves the way it does, and it seems a behavior
worth keeping.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Ensure standard file descriptors are open on start
2020-08-18 22:37 ` Paul Eggert
@ 2020-08-18 23:28 ` Arsen Arsenović
0 siblings, 0 replies; 5+ messages in thread
From: Arsen Arsenović @ 2020-08-18 23:28 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 411 bytes --]
> That's not what I expect. There is a benefit for a program like 'cat' to
> report an error (instead of silently succeeding) if its invoker closed stdin
> or stdout. That's why glibc behaves the way it does, and it seems a behavior
> worth keeping.
Fair enough. It is, however, still not correct to have any of those missing on
startup. I can resubmit the patch with this fixed.
--
Arsen Arsenović
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-18 23:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-18 17:50 [PATCH] Ensure standard file descriptors are open on start Arsen Arsenović
2020-08-18 20:11 ` Paul Eggert
2020-08-18 22:31 ` Arsen Arsenović
2020-08-18 22:37 ` Paul Eggert
2020-08-18 23:28 ` Arsen Arsenović
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).