From patchwork Mon Sep 19 12:41:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 15767 Received: (qmail 26814 invoked by alias); 19 Sep 2016 12:41:20 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 26801 invoked by uid 89); 19 Sep 2016 12:41:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=signals, H*r:10.0.0, catching, 1146 X-HELO: mail-yw0-f176.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=Jq+Zdz2TjeoPzF7e96syfSCf3ObpK6upk68bM9nKK7w=; b=NkgZEiHIycpa81cV4YkVILk1qzeSN58vOkINJ5w0zNj6rM/XnrNMgpYc9iXPJKH6gy 4ssceKFkCdq+6Zr4fogwTp1ys9OqsXvxIkefOcf6Sg8bTmVSJ02xO74kL430Eu7hPRfT tuhsGOlvzmp5j87hhjqHCp5+CXF+I/82S9hSD+2DrXVAz1YPGW/+ZaHmPEl6RAcVbcNO VIuOUhDG3CVikK0plBYUfFHRs7d+qgkhvxcqzTrLdnMlTnpeIINKy7sHj057a1i3Auz9 E9VYZ1gr0U3/Lp2yvfhRNSFbfOy5DHt9gnZR2f2yEgYpkcQzeEiiWQMsuNUZodnXuuZk nv3Q== X-Gm-Message-State: AE9vXwN2/HRGrJgD9UyWke3zn8O13Ht8wRm1UJotNzbrV2NSKSwQESjB6WV+BrzyUMgEeZwl X-Received: by 10.129.78.20 with SMTP id c20mr8921967ywb.76.1474288867344; Mon, 19 Sep 2016 05:41:07 -0700 (PDT) Subject: Re: [PATCH] posix: Fix open file action for posix_spawn on Linux To: Florian Weimer References: <1474060583-1277-1-git-send-email-adhemerval.zanella@linaro.org> <87shszwiym.fsf@mid.deneb.enyo.de> Cc: libc-alpha@sourceware.org From: Adhemerval Zanella Message-ID: <82eaa88f-ee29-4b78-33e9-61b27dfba9e6@linaro.org> Date: Mon, 19 Sep 2016 09:41:00 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <87shszwiym.fsf@mid.deneb.enyo.de> On 16/09/2016 19:58, Florian Weimer wrote: > * Adhemerval Zanella: > >> On posix_spawn open file action (issued by posix_spawn_file_actions_addopen) >> POSIX states that if fildes was already an open file descriptor, it shall be >> closed before the new file is openedi [1]. This avoid pontential issues when >> posix_spawn plus addopen action is called with the process already at maximum >> number of file descriptor opened and also for multiple actions on single-open >> special paths (like /dev/watchdog). >> >> This fixes its behavior on Linux posix_spawn implementation and also adds >> a tests to check for its behavior. > > The patch appears to be incomplete: > > ../sysdeps/unix/sysv/linux/spawni.c: In function ‘__spawni_child’: > ../sysdeps/unix/sysv/linux/spawni.c:244:3: error: implicit declaration of function ‘fd_is_valid’ [-Werror=implicit-function-declaration] > Thanks for catching it, below it the correct patch (with the function defined): diff --git a/posix/Makefile b/posix/Makefile index 3a7719e..586d45b 100644 --- a/posix/Makefile +++ b/posix/Makefile @@ -94,7 +94,7 @@ tests := tstgetopt testfnm runtests runptests \ xtests := bug-ga2 ifeq (yes,$(build-shared)) test-srcs := globtest -tests += wordexp-test tst-exec tst-spawn tst-spawn2 +tests += wordexp-test tst-exec tst-spawn tst-spawn2 tst-spawn3 endif tests-static = tst-exec-static tst-spawn-static tests += $(tests-static) diff --git a/posix/tst-spawn3.c b/posix/tst-spawn3.c new file mode 100644 index 0000000..e66c491 --- /dev/null +++ b/posix/tst-spawn3.c @@ -0,0 +1,189 @@ +/* Check if posix add file actions. + 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 +#include +#include + +static int do_test (void); +#define TEST_FUNCTION do_test () +#include + +static int +do_test (void) +{ + /* The test checks if posix_spawn open file action close the file descriptor + before opening a new one in case the input file descriptor is already + opened. It does by exhausting all file descriptors on the process before + issue posix_spawn. It then issues a posix_spawn for '/bin/sh echo $$' + and add two rules: + + 1. Redirect stdout to a temporary filepath + 2. Redirect stderr to stdout + + If the implementation does not close the file 1. will fail with + EMFILE. */ + + struct rlimit rl; + int max_fd = 24; + + /* Set maximum number of file descriptor to a low value to avoid open + too many files in environments where RLIMIT_NOFILE is large and to + limit the array size to track the opened file descriptors. */ + + if (getrlimit (RLIMIT_NOFILE, &rl) == -1) + { + printf ("error: getrlimit RLIMIT_NOFILE failed"); + exit (EXIT_FAILURE); + } + + max_fd = (rl.rlim_cur < max_fd ? rl.rlim_cur : max_fd); + rl.rlim_cur = max_fd; + + if (setrlimit (RLIMIT_NOFILE, &rl) == 1) + { + printf ("error: setrlimit RLIMIT_NOFILE to %u failed", max_fd); + exit (EXIT_FAILURE); + } + + /* Exhauste the file descriptor limit with temporary files. */ + int files[max_fd]; + int nfiles = 0; + for (;;) + { + int fd = create_temp_file ("tst-spawn3.", NULL); + if (fd == -1) + { + if (errno != EMFILE) + { + printf ("error: create_temp_file returned -1 with " + "errno != EMFILE\n"); + exit (EXIT_FAILURE); + } + break; + } + files[nfiles++] = fd; + } + + posix_spawn_file_actions_t a; + if (posix_spawn_file_actions_init (&a) != 0) + { + puts ("error: spawn_file_actions_init failed"); + exit (EXIT_FAILURE); + } + + /* Executes a /bin/sh echo $$ 2>&1 > /tmp/tst-spawn3.pid . */ + const char pidfile[] = "/tmp/tst-spawn3.pid"; + if (posix_spawn_file_actions_addopen (&a, STDOUT_FILENO, pidfile, O_WRONLY | + O_CREAT | O_TRUNC, 0644) != 0) + { + puts ("error: spawn_file_actions_addopen failed"); + exit (EXIT_FAILURE); + } + + if (posix_spawn_file_actions_adddup2 (&a, STDOUT_FILENO, STDERR_FILENO) != 0) + { + puts ("error: spawn_file_actions_addclose"); + exit (EXIT_FAILURE); + } + + /* Since execve (called by posix_spawn) might require to open files to + actually execute the shell script, setup to close the temporary file + descriptors. */ + for (int i=0; iaction.open_action.fd)) + close_not_cancel (action->action.open_action.fd); + ret = open_not_cancel (action->action.open_action.path, action->action. open_action.oflag | O_LARGEFILE,