From 6793dd51d8e7d4f3408c183c6f7dc86acd7f1330 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: Fri, 19 Feb 2016 14:49:38 +0100 Subject: [PATCH] Improve file descriptor checks for posix_spawn actions [BZ #19505] --- ChangeLog | 17 ++++ posix/Makefile | 5 +- posix/spawn_faction_addclose.c | 4 +- posix/spawn_faction_adddup2.c | 4 +- posix/spawn_faction_addopen.c | 5 +- posix/spawn_int.h | 30 ++++++ posix/spawn_valid_fd.c | 31 +++++++ posix/tst-posix_spawn-fd.c | 165 +++++++++++++++++++++++++++++++++ 8 files changed, 249 insertions(+), 12 deletions(-) create mode 100644 posix/spawn_valid_fd.c create mode 100644 posix/tst-posix_spawn-fd.c diff --git a/ChangeLog b/ChangeLog index bc7c5d374b..8d5406870b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2016-02-19 Florian Weimer + + [BZ #19505] + * posix/spawn_int.h: Add headers and include guard. + (__spawn_valid_fd): New function. + * posix/spawn_faction_addopen.c + (posix_spawn_file_actions_addopen): Use __spawn_valid_fd. + * posix/spawn_faction_addclose.c + (posix_spawn_file_actions_addclose): Likewise. + * posix/spawn_faction_adddup2.c + (posix_spawn_file_actions_adddup2): Likewise. Add check for + second file descriptor. + * posix/spawn_valid_fd.c: New file. + * posix/tst-posix_spawn-fd.c: New file. + * posix/Makefile (routines): Add spawn_valid_fd. + (tests): Add tst-posix_spawn-fd. + 2016-02-19 Florian Weimer * malloc/tst-malloc-thread-exit.c: Include test-skeleton.c early. diff --git a/posix/Makefile b/posix/Makefile index f94e023c60..4e90a9540a 100644 --- a/posix/Makefile +++ b/posix/Makefile @@ -51,7 +51,7 @@ routines := \ getaddrinfo gai_strerror wordexp \ pread pwrite pread64 pwrite64 \ spawn_faction_init spawn_faction_destroy spawn_faction_addclose \ - spawn_faction_addopen spawn_faction_adddup2 \ + spawn_faction_addopen spawn_faction_adddup2 spawn_valid_fd \ spawnattr_init spawnattr_destroy \ spawnattr_getdefault spawnattr_setdefault \ spawnattr_getflags spawnattr_setflags \ @@ -87,7 +87,8 @@ tests := tstgetopt testfnm runtests runptests \ bug-getopt1 bug-getopt2 bug-getopt3 bug-getopt4 \ bug-getopt5 tst-getopt_long1 bug-regex34 bug-regex35 \ tst-pathconf tst-getaddrinfo4 tst-rxspencer-no-utf8 \ - tst-fnmatch3 bug-regex36 tst-getaddrinfo5 + tst-fnmatch3 bug-regex36 tst-getaddrinfo5 \ + tst-posix_spawn-fd xtests := bug-ga2 ifeq (yes,$(build-shared)) test-srcs := globtest diff --git a/posix/spawn_faction_addclose.c b/posix/spawn_faction_addclose.c index 1a4a2f2457..946454e646 100644 --- a/posix/spawn_faction_addclose.c +++ b/posix/spawn_faction_addclose.c @@ -27,11 +27,9 @@ int posix_spawn_file_actions_addclose (posix_spawn_file_actions_t *file_actions, int fd) { - int maxfd = __sysconf (_SC_OPEN_MAX); struct __spawn_action *rec; - /* Test for the validity of the file descriptor. */ - if (fd < 0 || fd >= maxfd) + if (!__spawn_valid_fd (fd)) return EBADF; /* Allocate more memory if needed. */ diff --git a/posix/spawn_faction_adddup2.c b/posix/spawn_faction_adddup2.c index 8beee1099b..a04fc521c3 100644 --- a/posix/spawn_faction_adddup2.c +++ b/posix/spawn_faction_adddup2.c @@ -27,11 +27,9 @@ int posix_spawn_file_actions_adddup2 (posix_spawn_file_actions_t *file_actions, int fd, int newfd) { - int maxfd = __sysconf (_SC_OPEN_MAX); struct __spawn_action *rec; - /* Test for the validity of the file descriptor. */ - if (fd < 0 || newfd < 0 || fd >= maxfd || newfd >= maxfd) + if (!__spawn_valid_fd (fd) || !__spawn_valid_fd (newfd)) return EBADF; /* Allocate more memory if needed. */ diff --git a/posix/spawn_faction_addopen.c b/posix/spawn_faction_addopen.c index 36cde06fb2..4f37d0b847 100644 --- a/posix/spawn_faction_addopen.c +++ b/posix/spawn_faction_addopen.c @@ -16,7 +16,6 @@ . */ #include -#include #include #include #include @@ -30,11 +29,9 @@ posix_spawn_file_actions_addopen (posix_spawn_file_actions_t *file_actions, int fd, const char *path, int oflag, mode_t mode) { - int maxfd = __sysconf (_SC_OPEN_MAX); struct __spawn_action *rec; - /* Test for the validity of the file descriptor. */ - if (fd < 0 || fd >= maxfd) + if (!__spawn_valid_fd (fd)) return EBADF; char *path_copy = strdup (path); diff --git a/posix/spawn_int.h b/posix/spawn_int.h index 861e3b47bb..b7da3b8abe 100644 --- a/posix/spawn_int.h +++ b/posix/spawn_int.h @@ -1,3 +1,27 @@ +/* Internal definitions for posix_spawn functionality. + Copyright (C) 2000-2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#ifndef _SPAWN_INT_H +#define _SPAWN_INT_H + +#include +#include + /* Data structure to contain the action information. */ struct __spawn_action { @@ -39,3 +63,9 @@ extern int __spawni (pid_t *pid, const char *path, const posix_spawn_file_actions_t *file_actions, const posix_spawnattr_t *attrp, char *const argv[], char *const envp[], int xflags); + +/* Return true if FD falls into the range valid for file descriptors. + The check in this form is mandated by POSIX. */ +bool __spawn_valid_fd (int fd) internal_function attribute_hidden; + +#endif /* _SPAWN_INT_H */ diff --git a/posix/spawn_valid_fd.c b/posix/spawn_valid_fd.c new file mode 100644 index 0000000000..c199750557 --- /dev/null +++ b/posix/spawn_valid_fd.c @@ -0,0 +1,31 @@ +/* File descriptor validity check for posix_spawn file actions. + Copyright (C) 2000-2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include "spawn_int.h" + +#include + +bool +internal_function +__spawn_valid_fd (int fd) +{ + long maxfd = __sysconf (_SC_OPEN_MAX); + return __glibc_likely (fd >= 0) + && (__glibc_unlikely (maxfd < 0) /* No limit set. */ + || __glibc_likely (fd < maxfd)); +} diff --git a/posix/tst-posix_spawn-fd.c b/posix/tst-posix_spawn-fd.c new file mode 100644 index 0000000000..cde5bdccd5 --- /dev/null +++ b/posix/tst-posix_spawn-fd.c @@ -0,0 +1,165 @@ +/* Test that spawn file action functions work without file limit. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include +#include + +/* _SC_OPEN_MAX value. */ +static long maxfd; + +/* A positive but unused file descriptor, used for testing + purposes. */ +static int invalid_fd; + +/* Indicate that errors have been encountered. */ +static bool errors; + +static posix_spawn_file_actions_t actions; + +static void +one_test (const char *name, int (*func) (int), int fd, + bool expect_success) +{ + int ret = func (fd); + if (expect_success) + { + if (ret != 0) + { + errno = ret; + printf ("error: posix_spawn_file_actions_%s (%d): %m\n", name, fd); + errors = true; + } + } + else if (ret != EBADF) + { + if (ret == 0) + printf ("error: posix_spawn_file_actions_%s (%d):" + " unexpected success\n", name, fd); + else + { + errno = ret; + printf ("error: posix_spawn_file_actions_%s (%d): %m\n", name, fd); + } + errors = true; + } +} + +static void +all_tests (const char *name, int (*func) (int)) +{ + one_test (name, func, 0, true); + one_test (name, func, invalid_fd, true); + one_test (name, func, -1, false); + one_test (name, func, -2, false); + if (maxfd >= 0) + one_test (name, func, maxfd, false); +} + +static int +addopen (int fd) +{ + return posix_spawn_file_actions_addopen + (&actions, fd, "/dev/null", O_RDONLY, 0); +} + +static int +adddup2 (int fd) +{ + return posix_spawn_file_actions_adddup2 (&actions, fd, 1); +} + +static int +adddup2_reverse (int fd) +{ + return posix_spawn_file_actions_adddup2 (&actions, 1, fd); +} + +static int +addclose (int fd) +{ + return posix_spawn_file_actions_addclose (&actions, fd); +} + +static void +all_functions (void) +{ + all_tests ("addopen", addopen); + all_tests ("adddup2", adddup2); + all_tests ("adddup2", adddup2_reverse); + all_tests ("adddup2", addclose); +} + +static int +do_test (void) +{ + /* Try to eliminate the file descriptor limit. */ + { + struct rlimit limit; + if (getrlimit (RLIMIT_NOFILE, &limit) < 0) + { + printf ("error: getrlimit: %m\n"); + return 1; + } + limit.rlim_cur = RLIM_INFINITY; + if (setrlimit (RLIMIT_NOFILE, &limit) < 0) + printf ("warning: setrlimit: %m\n"); + } + + maxfd = sysconf (_SC_OPEN_MAX); + printf ("info: _SC_OPEN_MAX: %ld\n", maxfd); + + invalid_fd = dup (0); + if (invalid_fd < 0) + { + printf ("error: dup: %m\n"); + return 1; + } + if (close (invalid_fd) < 0) + { + printf ("error: close: %m\n"); + return 1; + } + + int ret = posix_spawn_file_actions_init (&actions); + if (ret != 0) + { + errno = ret; + printf ("error: posix_spawn_file_actions_init: %m\n"); + return 1; + } + + all_functions (); + + ret = posix_spawn_file_actions_destroy (&actions); + if (ret != 0) + { + errno = ret; + printf ("error: posix_spawn_file_actions_destroy: %m\n"); + return 1; + } + + return errors; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c"