mirror of
git://sourceware.org/git/glibc.git
synced 2025-01-24 12:25:35 +08:00
posix: Clear close-on-exec for posix_spawn adddup2 (BZ#23640)
Austin Group issue #411 [1] proposes that posix_spawn file action posix_spawn_file_actions_adddup2 resets the close-on-exec when source and destination refer to same file descriptor. It solves the issue on multi-thread applications which uses close-on-exec as default, and want to hand-chose specifically file descriptor to purposefully inherited into a child process. Current approach to achieve this scenario is to use two adddup2 file actions and a temporary file description which do not conflict with any other, coupled with a close file action to avoid leaking the temporary file descriptor. This approach, besides being complex, may fail with EMFILE/ENFILE file descriptor exaustion. This can be more easily accomplished with an in-place removal of FD_CLOEXEC. Although the resulting adddup2 semantic is slight different than dup2 (equal file descriptors should be handled as no-op), the proposed possible solution are either more complex (fcntl action which a limited set of operations) or results in unrequired operations (dup3 which also returns EINVAL for same file descriptor). Checked on aarch64-linux-gnu. [BZ #23640] * posix/tst-spawn.c (do_prepare, handle_restart, do_test): Add posix_spawn_file_actions_adddup2 test to check O_CLOCEXEC reset. * sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Add close-on-exec reset for adddup2 file action. * sysdeps/posix/spawni.c (__spawni_child): Likewise. [1] http://austingroupbugs.net/view.php?id=411
This commit is contained in:
Adhemerval Zanella
parent
03992356e6
commit
805334b26c
@ -1,3 +1,12 @@
|
||||
2019-01-03 Adhemerval Zanella <adhemerval.zanella@linaro.org>
|
||||
|
||||
[BZ #23640]
|
||||
* posix/tst-spawn.c (do_prepare, handle_restart, do_test): Add
|
||||
posix_spawn_file_actions_adddup2 test to check O_CLOCEXEC reset.
|
||||
* sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Add
|
||||
close-on-exec reset for adddup2 file action.
|
||||
* sysdeps/posix/spawni.c (__spawni_child): Likewise.
|
||||
|
||||
2019-01-03 Zack Weinberg <zackw@panix.com>
|
||||
|
||||
* include/features.h (__GLIBC_USE_DEPRECATED_SCANF): New __GLIBC_USE
|
||||
|
||||
@ -44,16 +44,19 @@ static int restart;
|
||||
static char *name1;
|
||||
static char *name2;
|
||||
static char *name3;
|
||||
static char *name5;
|
||||
|
||||
/* Descriptors for the temporary files. */
|
||||
static int temp_fd1 = -1;
|
||||
static int temp_fd2 = -1;
|
||||
static int temp_fd3 = -1;
|
||||
static int temp_fd5 = -1;
|
||||
|
||||
/* The contents of our files. */
|
||||
static const char fd1string[] = "This file should get closed";
|
||||
static const char fd2string[] = "This file should stay opened";
|
||||
static const char fd3string[] = "This file will be opened";
|
||||
static const char fd5string[] = "This file should stay opened (O_CLOEXEC)";
|
||||
|
||||
|
||||
/* We have a preparation function. */
|
||||
@ -67,25 +70,32 @@ do_prepare (int argc, char *argv[])
|
||||
TEST_VERIFY_EXIT ((temp_fd1 = create_temp_file ("spawn", &name1)) != -1);
|
||||
TEST_VERIFY_EXIT ((temp_fd2 = create_temp_file ("spawn", &name2)) != -1);
|
||||
TEST_VERIFY_EXIT ((temp_fd3 = create_temp_file ("spawn", &name3)) != -1);
|
||||
TEST_VERIFY_EXIT ((temp_fd5 = create_temp_file ("spawn", &name5)) != -1);
|
||||
|
||||
int flags;
|
||||
TEST_VERIFY_EXIT ((flags = fcntl (temp_fd5, F_GETFD, &flags)) != -1);
|
||||
TEST_COMPARE (fcntl (temp_fd5, F_SETFD, flags | FD_CLOEXEC), 0);
|
||||
}
|
||||
#define PREPARE do_prepare
|
||||
|
||||
|
||||
static int
|
||||
handle_restart (const char *fd1s, const char *fd2s, const char *fd3s,
|
||||
const char *fd4s, const char *name)
|
||||
const char *fd4s, const char *name, const char *fd5s)
|
||||
{
|
||||
char buf[100];
|
||||
int fd1;
|
||||
int fd2;
|
||||
int fd3;
|
||||
int fd4;
|
||||
int fd5;
|
||||
|
||||
/* First get the descriptors. */
|
||||
fd1 = atol (fd1s);
|
||||
fd2 = atol (fd2s);
|
||||
fd3 = atol (fd3s);
|
||||
fd4 = atol (fd4s);
|
||||
fd5 = atol (fd5s);
|
||||
|
||||
/* Sanity check. */
|
||||
TEST_VERIFY_EXIT (fd1 != fd2);
|
||||
@ -94,6 +104,7 @@ handle_restart (const char *fd1s, const char *fd2s, const char *fd3s,
|
||||
TEST_VERIFY_EXIT (fd2 != fd3);
|
||||
TEST_VERIFY_EXIT (fd2 != fd4);
|
||||
TEST_VERIFY_EXIT (fd3 != fd4);
|
||||
TEST_VERIFY_EXIT (fd4 != fd5);
|
||||
|
||||
/* First the easy part: read from the file descriptor which is
|
||||
supposed to be open. */
|
||||
@ -122,6 +133,10 @@ handle_restart (const char *fd1s, const char *fd2s, const char *fd3s,
|
||||
TEST_COMPARE (read (fd1, buf, sizeof buf), strlen (fd1string));
|
||||
TEST_COMPARE_BLOB (fd1string, strlen (fd1string), buf, strlen (fd1string));
|
||||
|
||||
TEST_COMPARE (xlseek (fd5, 0, SEEK_SET), 0);
|
||||
TEST_COMPARE (read (fd5, buf, sizeof buf), strlen (fd5string));
|
||||
TEST_COMPARE_BLOB (fd5string, strlen (fd5string), buf, strlen (fd5string));
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
@ -137,6 +152,7 @@ do_test (int argc, char *argv[])
|
||||
char fd2name[18];
|
||||
char fd3name[18];
|
||||
char fd4name[18];
|
||||
char fd5name[18];
|
||||
char *name3_copy;
|
||||
char *spargv[12];
|
||||
int i;
|
||||
@ -147,26 +163,30 @@ do_test (int argc, char *argv[])
|
||||
+ "--library-path" optional
|
||||
+ the library path optional
|
||||
+ the application name
|
||||
- five parameters left if called through re-execution
|
||||
- six parameters left if called through re-execution
|
||||
+ file descriptor number which is supposed to be closed
|
||||
+ the open file descriptor
|
||||
+ the newly opened file descriptor
|
||||
+ thhe duped second descriptor
|
||||
+ the duped second descriptor
|
||||
+ the name of the closed descriptor
|
||||
+ the duped fourth dile descriptor which O_CLOEXEC should be
|
||||
remove by adddup2.
|
||||
*/
|
||||
if (argc != (restart ? 6 : 2) && argc != (restart ? 6 : 5))
|
||||
if (argc != (restart ? 7 : 2) && argc != (restart ? 7 : 5))
|
||||
FAIL_EXIT1 ("wrong number of arguments (%d)", argc);
|
||||
|
||||
if (restart)
|
||||
return handle_restart (argv[1], argv[2], argv[3], argv[4], argv[5]);
|
||||
return handle_restart (argv[1], argv[2], argv[3], argv[4], argv[5],
|
||||
argv[6]);
|
||||
|
||||
/* Prepare the test. We are creating two files: one which file descriptor
|
||||
/* Prepare the test. We are creating four files: two which file descriptor
|
||||
will be marked with FD_CLOEXEC, another which is not. */
|
||||
|
||||
/* Write something in the files. */
|
||||
xwrite (temp_fd1, fd1string, strlen (fd1string));
|
||||
xwrite (temp_fd2, fd2string, strlen (fd2string));
|
||||
xwrite (temp_fd3, fd3string, strlen (fd3string));
|
||||
xwrite (temp_fd5, fd5string, strlen (fd5string));
|
||||
|
||||
/* Close the third file. It'll be opened by `spawn'. */
|
||||
xclose (temp_fd3);
|
||||
@ -185,17 +205,24 @@ do_test (int argc, char *argv[])
|
||||
memset (name3_copy, 'X', strlen (name3_copy));
|
||||
|
||||
/* We dup the second descriptor. */
|
||||
fd4 = MAX (2, MAX (temp_fd1, MAX (temp_fd2, temp_fd3))) + 1;
|
||||
fd4 = MAX (2, MAX (temp_fd1, MAX (temp_fd2, MAX (temp_fd3, temp_fd5)))) + 1;
|
||||
TEST_COMPARE (posix_spawn_file_actions_adddup2 (&actions, temp_fd2, fd4),
|
||||
0);
|
||||
|
||||
/* We clear the O_CLOEXEC on fourth descriptor, so it should be
|
||||
stay open on child. */
|
||||
TEST_COMPARE (posix_spawn_file_actions_adddup2 (&actions, temp_fd5,
|
||||
temp_fd5),
|
||||
0);
|
||||
|
||||
/* Now spawn the process. */
|
||||
snprintf (fd1name, sizeof fd1name, "%d", temp_fd1);
|
||||
snprintf (fd2name, sizeof fd2name, "%d", temp_fd2);
|
||||
snprintf (fd3name, sizeof fd3name, "%d", temp_fd3);
|
||||
snprintf (fd4name, sizeof fd4name, "%d", fd4);
|
||||
snprintf (fd5name, sizeof fd5name, "%d", temp_fd5);
|
||||
|
||||
for (i = 0; i < (argc == (restart ? 6 : 5) ? 4 : 1); i++)
|
||||
for (i = 0; i < (argc == (restart ? 7 : 5) ? 4 : 1); i++)
|
||||
spargv[i] = argv[i + 1];
|
||||
spargv[i++] = (char *) "--direct";
|
||||
spargv[i++] = (char *) "--restart";
|
||||
@ -204,6 +231,7 @@ do_test (int argc, char *argv[])
|
||||
spargv[i++] = fd3name;
|
||||
spargv[i++] = fd4name;
|
||||
spargv[i++] = name1;
|
||||
spargv[i++] = fd5name;
|
||||
spargv[i] = NULL;
|
||||
|
||||
TEST_COMPARE (posix_spawn (&pid, argv[1], &actions, NULL, spargv, environ),
|
||||
|
||||
@ -204,9 +204,21 @@ __spawni_child (void *arguments)
|
||||
break;
|
||||
|
||||
case spawn_do_dup2:
|
||||
if (__dup2 (action->action.dup2_action.fd,
|
||||
action->action.dup2_action.newfd)
|
||||
!= action->action.dup2_action.newfd)
|
||||
/* Austin Group issue #411 requires adddup2 action with source
|
||||
and destination being equal to remove close-on-exec flag. */
|
||||
if (action->action.dup2_action.fd
|
||||
== action->action.dup2_action.newfd)
|
||||
{
|
||||
int fd = action->action.dup2_action.newfd;
|
||||
int flags = __fcntl (fd, F_GETFD, 0);
|
||||
if (flags == -1)
|
||||
goto fail;
|
||||
if (__fcntl (fd, F_SETFD, flags & ~FD_CLOEXEC) == -1)
|
||||
goto fail;
|
||||
}
|
||||
else if (__dup2 (action->action.dup2_action.fd,
|
||||
action->action.dup2_action.newfd)
|
||||
!= action->action.dup2_action.newfd)
|
||||
goto fail;
|
||||
break;
|
||||
|
||||
|
||||
@ -253,9 +253,21 @@ __spawni_child (void *arguments)
|
||||
break;
|
||||
|
||||
case spawn_do_dup2:
|
||||
if (__dup2 (action->action.dup2_action.fd,
|
||||
action->action.dup2_action.newfd)
|
||||
!= action->action.dup2_action.newfd)
|
||||
/* Austin Group issue #411 requires adddup2 action with source
|
||||
and destination being equal to remove close-on-exec flag. */
|
||||
if (action->action.dup2_action.fd
|
||||
== action->action.dup2_action.newfd)
|
||||
{
|
||||
int fd = action->action.dup2_action.newfd;
|
||||
int flags = __fcntl (fd, F_GETFD, 0);
|
||||
if (flags == -1)
|
||||
goto fail;
|
||||
if (__fcntl (fd, F_SETFD, flags & ~FD_CLOEXEC) == -1)
|
||||
goto fail;
|
||||
}
|
||||
else if (__dup2 (action->action.dup2_action.fd,
|
||||
action->action.dup2_action.newfd)
|
||||
!= action->action.dup2_action.newfd)
|
||||
goto fail;
|
||||
break;
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user