From debbugs-submit-bounces@debbugs.gnu.org Fri Jan 27 06:52:08 2023 Received: (at submit) by debbugs.gnu.org; 27 Jan 2023 11:52:08 +0000 Received: from localhost ([127.0.0.1]:36849 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pLNH3-0004HO-OS for submit@debbugs.gnu.org; Fri, 27 Jan 2023 06:52:08 -0500 Received: from lists.gnu.org ([209.51.188.17]:58508) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pLNGx-0004Gs-9w for submit@debbugs.gnu.org; Fri, 27 Jan 2023 06:52:03 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pLNGw-0001CZ-I2 for bug-guile@gnu.org; Fri, 27 Jan 2023 06:51:58 -0500 Received: from mail.omarpolo.com ([144.91.116.244]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pLNGi-0007dJ-Pw for bug-guile@gnu.org; Fri, 27 Jan 2023 06:51:47 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=omarpolo.com; s=20200327; t=1674820296; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc; bh=lXkxayJwYt7c0Gm/qLXJfijDmttThi7UpTRKia9CI1o=; b=3oyU0X88txNXZRKG4ofnVITuKo+SqfnHhV5FItjyHmaIpcgdAiUvcUO/DYYHAt7bTb8OgL G6C/1KlFMwgpdRn4XU3QpDL3aOrWUBbNfe+QH/KgUnUvharx/qUgeeQJ1oXnUlp+JK7c+N i2YxIs+dOUba9NekEzO0aWdE06/Dl0Y= Received: from localhost (host-82-61-20-176.retail.telecomitalia.it [82.61.20.176]) by mail.omarpolo.com (OpenSMTPD) with ESMTPSA id 984b554e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Fri, 27 Jan 2023 12:51:35 +0100 (CET) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 9eed019b for ; Fri, 27 Jan 2023 12:51:32 +0100 (CET) Date: Fri, 27 Jan 2023 12:51:32 +0100 To: bug-guile@gnu.org Subject: possible misuse of posix_spawn API on non-linux OSes From: Omar Polo Message-Id: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> User-Agent: mblaze/1.2 Received-SPF: pass client-ip=144.91.116.244; envelope-from=op@omarpolo.com; helo=mail.omarpolo.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.4 (-) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.4 (--) Hello, I've noticed that test-system-cmds fails on OpenBSD-CURRENT while testing the update to guile 3.0.9: test-system-cmds: system* exit status was 127 rather than 42 FAIL: test-system-cmds Here's an excerpt of the ktrace of the child process while executing that specific test: (the first fork() is the one implicitly done by posix_spawn(3)) 5590 guile RET fork 0 [...] 5590 guile CALL dup2(0,3) 5590 guile RET dup2 3 5590 guile CALL dup2(1,4) 5590 guile RET dup2 4 5590 guile CALL dup2(2,5) 5590 guile RET dup2 5 5590 guile CALL dup2(3,0) 5590 guile RET dup2 0 5590 guile CALL dup2(4,1) 5590 guile RET dup2 1 5590 guile CALL dup2(5,2) 5590 guile RET dup2 2 5590 guile CALL close(1023) 5590 guile RET close -1 errno 9 Bad file descriptor 5590 guile CALL kbind(0x7f7ffffd51f8,24,0x2b5c5ced59893fa9) 5590 guile RET kbind 0 5590 guile CALL exit(127) (if you prefer I can provide a full ktrace of guile executing that test case) My interpretation is that the sequence of dup2(2) is from posix_spawn_file_actions_adddup2 in do_spawn, while the strange close(1023) is from close_inherited_fds_slow. Such file descriptor is not open, so close(2) fails with EBADF and the posix_spawn machinery exits prematurely. My current RLIMIT_NOFILE is 1024, so the number would make sense. On OpenBSD I've tried to use the following patch to work around the issue: [[[ Index: libguile/posix.c --- libguile/posix.c.orig +++ libguile/posix.c @@ -1325,6 +1325,7 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0, static void close_inherited_fds_slow (posix_spawn_file_actions_t *actions, int max_fd) { + max_fd = getdtablecount(); while (--max_fd > 2) posix_spawn_file_actions_addclose (actions, max_fd); } ]]] getdtablecount(2) returns the number of file descriptor currently open by the process. unfortunately it doesn't seem to be portable. (well, tbf /proc/self/fd is not portable too.) However, while this pleases the system* test, it breaks the pipe tests: Running popen.test FAIL: popen.test: open-input-pipe: echo hello FAIL: popen.test: pipeline - arguments: (expected-value ("HELLO WORLD\n" (0 0)) actual-value ("" (127 0))) the reason seem to be similar: 74865 guile CALL dup2(7,3) 74865 guile RET dup2 3 74865 guile CALL dup2(10,4) 74865 guile RET dup2 4 74865 guile CALL dup2(2,5) 74865 guile RET dup2 5 74865 guile CALL dup2(3,0) 74865 guile RET dup2 0 74865 guile CALL dup2(4,1) 74865 guile RET dup2 1 74865 guile CALL dup2(5,2) 74865 guile RET dup2 2 74865 guile CALL close(8) 74865 guile RET close -1 errno 9 Bad file descriptor 74865 guile CALL kbind(0x7f7ffffcfa88,24,0x2125923bdf2ca9e) 74865 guile RET kbind 0 74865 guile CALL exit(127) I guess it's trying to close the fd of the pipe that was closed. I'm not sure what to do from here, I'm not used to the posix_spawn_* APIs. I'm happy to help testing diffs or by providing more info if needed. Thanks, Omar Polo From debbugs-submit-bounces@debbugs.gnu.org Fri Jan 27 07:25:24 2023 Received: (at 61095) by debbugs.gnu.org; 27 Jan 2023 12:25:24 +0000 Received: from localhost ([127.0.0.1]:36873 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pLNnI-00059G-FL for submit@debbugs.gnu.org; Fri, 27 Jan 2023 07:25:24 -0500 Received: from mail.omarpolo.com ([144.91.116.244]:51648) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pLNnD-00058r-D8 for 61095@debbugs.gnu.org; Fri, 27 Jan 2023 07:25:22 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=omarpolo.com; s=20200327; t=1674822312; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc; bh=1tTA1Blv5lTEElPHbeaYQkKqMLDTk3GoVPQX6uQYHhw=; b=sJf7sdj8SfmV0MtW0pdyJ3bYIluTGcMt2tiJjn9kLT2ebohUZpJUg17alTQuTLEvlJPuZK AV8Xlmlbjzk/842qFjktHILIgHreN04wsFM0gazxyqZWbKpv6SKPt6IWCB3E9fKnfHqd0B 4Og/wwFew/nmml5PIbNFEIdizu23XYI= Received: from localhost (host-82-61-20-176.retail.telecomitalia.it [82.61.20.176]) by mail.omarpolo.com (OpenSMTPD) with ESMTPSA id 12903135 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for <61095@debbugs.gnu.org>; Fri, 27 Jan 2023 13:25:12 +0100 (CET) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id d6c3c8f5 for <61095@debbugs.gnu.org>; Fri, 27 Jan 2023 13:25:09 +0100 (CET) Date: Fri, 27 Jan 2023 13:25:09 +0100 To: 61095@debbugs.gnu.org Subject: Re: possible misuse of posix_spawn API on non-linux OSes From: Omar Polo Message-Id: <3F1FNOS0VFO9X.356V67A0RSKPT@venera> User-Agent: mblaze/1.2 X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 61095 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Actually I can avoid the EBADF by checking that the fd is 'live' with something like fstat: [[[ Index: libguile/posix.c --- libguile/posix.c.orig +++ libguile/posix.c @@ -1325,8 +1325,12 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0, static void close_inherited_fds_slow (posix_spawn_file_actions_t *actions, int max_fd) { - while (--max_fd > 2) - posix_spawn_file_actions_addclose (actions, max_fd); + struct stat sb; + max_fd = getdtablecount(); + while (--max_fd > 2) { + if (fstat(max_fd, &sb) != -1) + posix_spawn_file_actions_addclose (actions, max_fd); + } } static void ]]] The regress passes and while this workaround may be temporarly acceptable I -personally- don't like it much. There's a reason guile can't set CLOEXEC for all the file descriptors > 2 obtained via open, socket, pipe, ... like perl -for example- does? From debbugs-submit-bounces@debbugs.gnu.org Mon Mar 27 09:33:04 2023 Received: (at control) by debbugs.gnu.org; 27 Mar 2023 13:33:05 +0000 Received: from localhost ([127.0.0.1]:46830 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pgmy8-0005qG-LD for submit@debbugs.gnu.org; Mon, 27 Mar 2023 09:33:04 -0400 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:29357) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pgmy7-0005pp-7z for control@debbugs.gnu.org; Mon, 27 Mar 2023 09:33:03 -0400 Authentication-Results: mail2-relais-roc.national.inria.fr; dkim=none (message not signed) header.i=none; spf=SoftFail smtp.mailfrom=ludo@gnu.org; dmarc=fail (p=none dis=none) d=gnu.org X-IronPort-AV: E=Sophos;i="5.98,294,1673910000"; d="scan'208";a="99321414" Received: from unknown (HELO ribbon) ([193.50.110.81]) by mail2-relais-roc.national.inria.fr with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Mar 2023 15:32:56 +0200 Date: Mon, 27 Mar 2023 15:32:56 +0200 Message-Id: <874jq6iak7.fsf@gnu.org> To: control@debbugs.gnu.org From: =?utf-8?Q?Ludovic_Court=C3=A8s?= Subject: control message for bug #61095 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.3 (/) X-Debbugs-Envelope-To: control X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) merge 61095 61079 quit From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 28 05:34:35 2023 Received: (at 61095) by debbugs.gnu.org; 28 Mar 2023 09:34:35 +0000 Received: from localhost ([127.0.0.1]:48967 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ph5it-0000JP-5Z for submit@debbugs.gnu.org; Tue, 28 Mar 2023 05:34:35 -0400 Received: from eggs.gnu.org ([209.51.188.92]:41302) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ph5ir-0000JA-36 for 61095@debbugs.gnu.org; Tue, 28 Mar 2023 05:34:33 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ph5ie-0000f5-M1; Tue, 28 Mar 2023 05:34:24 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:In-Reply-To:Date:References:Subject:To: From; bh=9WENLG4QvH8w/qU7PbrQPfjmc/cMmF76c1pHnomQgYc=; b=NT+FGUmw4hWLq/P/mvUY TVTpSu+GawufMYZvqWkGUaelhXWoC81WA6MHpRHFrZF5s0wimLsJfUS8/4M4ouPkYg3+gvCUTYK/p CwQ109BGpfiEwGl4r2xErNpmgBlZvZa2nKYsESQxVJxoNc9QnDM4q/ARH2N+ASaryCW2vAZmIrD0I JKYHNhmmBhilCaibSqSul14LAZxiYu5Qr2igXz5Zuzc8ol/JHBTiNj14bghZ02KkSu9GpXA5lX3dn Viqt3PFofSsjIB6Z/x02NcXBSDO0OOwlKJfOMViR8Pw+AW6LvQltMZ5/ih1vp2KHR+tXmOYcGH+bH 9Fv2JyhHzOzX1w==; Received: from [2001:660:6102:320:e120:2c8f:8909:cdfe] (helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ph5id-0006f3-3p; Tue, 28 Mar 2023 05:34:19 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Omar Polo Subject: Re: bug#61095: possible misuse of posix_spawn API on non-linux OSes References: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> Date: Tue, 28 Mar 2023 11:34:16 +0200 In-Reply-To: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> (Omar Polo's message of "Fri, 27 Jan 2023 12:51:32 +0100") Message-ID: <87zg7xgqxz.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 61095 Cc: Josselin Poiret , Andrew Whatson , 61095@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Omar, Apologies for the late reply. Omar Polo skribis: > I've noticed that test-system-cmds fails on OpenBSD-CURRENT while > testing the update to guile 3.0.9: > > test-system-cmds: system* exit status was 127 rather than 42 > FAIL: test-system-cmds We=E2=80=99re seeing the same failure on GNU/Hurd: https://issues.guix.gnu.org/61079 > Actually I can avoid the EBADF by checking that the fd is 'live' with > something like fstat: > > [[[ > > Index: libguile/posix.c > --- libguile/posix.c.orig > +++ libguile/posix.c > @@ -1325,8 +1325,12 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0, > static void > close_inherited_fds_slow (posix_spawn_file_actions_t *actions, int max_f= d) > { > - while (--max_fd > 2) > - posix_spawn_file_actions_addclose (actions, max_fd); > + struct stat sb; > + max_fd =3D getdtablecount(); > + while (--max_fd > 2) { > + if (fstat(max_fd, &sb) !=3D -1) > + posix_spawn_file_actions_addclose (actions, max_fd); > + } > } I came up with the following patch: --=-=-= Content-Type: text/x-patch Content-Disposition: inline diff --git a/libguile/posix.c b/libguile/posix.c index 3a8be94e4..cde199888 100644 --- a/libguile/posix.c +++ b/libguile/posix.c @@ -1322,39 +1322,18 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0, #undef FUNC_NAME #endif /* HAVE_FORK */ -static void -close_inherited_fds_slow (posix_spawn_file_actions_t *actions, int max_fd) -{ - while (--max_fd > 2) - posix_spawn_file_actions_addclose (actions, max_fd); -} - static void close_inherited_fds (posix_spawn_file_actions_t *actions, int max_fd) { - DIR *dirp; - struct dirent *d; - int fd; - - /* Try to use the platform-specific list of open file descriptors, so - we don't need to use the brute force approach. */ - dirp = opendir ("/proc/self/fd"); - - if (dirp == NULL) - return close_inherited_fds_slow (actions, max_fd); - - while ((d = readdir (dirp)) != NULL) + while (--max_fd > 2) { - fd = atoi (d->d_name); - - /* Skip "." and "..", garbage entries, stdin/stdout/stderr. */ - if (fd <= 2) - continue; - - posix_spawn_file_actions_addclose (actions, fd); + /* Adding invalid file descriptors to an 'addclose' action leads + to 'posix_spawn' failures on some operating systems: + . Hence the extra check. */ + int flags = fcntl (max_fd, F_GETFD, NULL); + if ((flags >= 0) && ((flags & FD_CLOEXEC) == 0)) + posix_spawn_file_actions_addclose (actions, max_fd); } - - closedir (dirp); } static pid_t @@ -1366,6 +1345,26 @@ do_spawn (char *exec_file, char **exec_argv, char **exec_env, posix_spawn_file_actions_t actions; posix_spawnattr_t *attrp = NULL; + posix_spawn_file_actions_init (&actions); + + /* Duplicate IN, OUT, and ERR unconditionally to clear their + FD_CLOEXEC flag, if any. */ + posix_spawn_file_actions_adddup2 (&actions, in, STDIN_FILENO); + posix_spawn_file_actions_adddup2 (&actions, out, STDOUT_FILENO); + posix_spawn_file_actions_adddup2 (&actions, err, STDERR_FILENO); + + /* TODO: Use 'closefrom' where available. */ +#if 0 + /* Version 2.34 of the GNU libc provides this function. */ + posix_spawn_file_actions_addclosefrom_np (&actions, 3); +#else + if (in > 2) + posix_spawn_file_actions_addclose (&actions, in); + if (out > 2 && out != in) + posix_spawn_file_actions_addclose (&actions, out); + if (err > 2 && err != out && err != in) + posix_spawn_file_actions_addclose (&actions, err); + int max_fd = 1024; #if defined (HAVE_GETRLIMIT) && defined (RLIMIT_NOFILE) @@ -1376,31 +1375,8 @@ do_spawn (char *exec_file, char **exec_argv, char **exec_env, } #endif - posix_spawn_file_actions_init (&actions); - - int free_fd_slots = 0; - int fd_slot[3]; - - for (int fdnum = 3; free_fd_slots < 3 && fdnum < max_fd; fdnum++) - { - if (fdnum != in && fdnum != out && fdnum != err) - { - fd_slot[free_fd_slots] = fdnum; - free_fd_slots++; - } - } - - /* Move the fds out of the way, so that duplicate fds or fds equal - to 0, 1, 2 don't trample each other */ - - posix_spawn_file_actions_adddup2 (&actions, in, fd_slot[0]); - posix_spawn_file_actions_adddup2 (&actions, out, fd_slot[1]); - posix_spawn_file_actions_adddup2 (&actions, err, fd_slot[2]); - posix_spawn_file_actions_adddup2 (&actions, fd_slot[0], 0); - posix_spawn_file_actions_adddup2 (&actions, fd_slot[1], 1); - posix_spawn_file_actions_adddup2 (&actions, fd_slot[2], 2); - close_inherited_fds (&actions, max_fd); +#endif int res = -1; if (spawnp) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Could you confirm that it works on OpenBSD and that there=E2=80=99s no performance regression? Andrew: it removes the /proc/self/fd loop you added to fix , but it reduces the number of =E2=80=98close= =E2=80=99 calls in the child. Could you check whether that=E2=80=99s okay performance-wise? Eventually I plan to use =E2=80=98posix_spawn_file_actions_addclosefrom_np= =E2=80=99 on glibc >=3D 2.34, but I have yet to test it. That will be the best solution. Josselin: I simplified the =E2=80=98dup2=E2=80=99 logic somewhat. Feedback welcome! > The regress passes and while this workaround may be temporarly > acceptable I -personally- don't like it much. There's a reason guile > can't set CLOEXEC for all the file descriptors > 2 obtained via open, > socket, pipe, ... like perl -for example- does? Guile does that for file descriptors it opens internally, but applications using =E2=80=98open-file=E2=80=99 without the recently-added = =E2=80=9Ce=E2=80=9D flag, or =E2=80=98socket=E2=80=99 without =E2=80=98SOCK_CLOEXEC=E2=80=99, etc., end = up with more file descriptors that need to be taken care of. I wish the default were close-on-exec, but we=E2=80=99re not there yet. Thanks, Ludo=E2=80=99. --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 28 12:10:34 2023 Received: (at 61095) by debbugs.gnu.org; 28 Mar 2023 16:10:34 +0000 Received: from localhost ([127.0.0.1]:50563 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1phBu6-0005XE-Az for submit@debbugs.gnu.org; Tue, 28 Mar 2023 12:10:34 -0400 Received: from jpoiret.xyz ([206.189.101.64]:33816) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1phBu4-0005X3-1Z for 61095@debbugs.gnu.org; Tue, 28 Mar 2023 12:10:32 -0400 Received: from authenticated-user (jpoiret.xyz [206.189.101.64]) by jpoiret.xyz (Postfix) with ESMTPA id 4CC73184F1E; Tue, 28 Mar 2023 16:10:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jpoiret.xyz; s=dkim; t=1680019830; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=HEYvsizn9jAYcZaSuFRuHtVO8WhPWurC300y+rIfHXo=; b=LzhUFRYiQ7AhVFcVKgNLUBrjK75sYyi6YDTJuGSeAd5BQICKJn9aHHLko2ZILST4HIJuEs 8K+T1TwvPuRfOiwpr9uoAXfB1RAp5gmQBWAJxe1CYXp2V1gWruyTOXzvoyWMBB934zg1ak Y9vhTgR9gXoEoipErr1v/DA6L3X80aoC3byJDrVujb5kOFBoPMo+4oQHRt1hGWtz3rDtpW L7TRA4wVI8iVY7IM9ZS5vjWHbJwvMRssDP+0j9PomdrWMXp1FahCZXWlfwkhSPt9wFNxni rbZDJxQhRqi4jYyTCmy7Q/J9emED1/oq8UsV8wSQDB3Rw9gLKqCs+5IGLC0GMg== From: Josselin Poiret To: Ludovic =?utf-8?Q?Court=C3=A8s?= , Omar Polo Subject: Re: bug#61095: possible misuse of posix_spawn API on non-linux OSes In-Reply-To: <87zg7xgqxz.fsf@gnu.org> References: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> <87zg7xgqxz.fsf@gnu.org> Date: Tue, 28 Mar 2023 18:10:27 +0200 Message-ID: <87tty4svpo.fsf@jpoiret.xyz> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Authentication-Results: jpoiret.xyz; auth=pass smtp.auth=jpoiret@jpoiret.xyz smtp.mailfrom=dev@jpoiret.xyz X-Spamd-Bar: -- X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 61095 Cc: Andrew Whatson , 61095@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Ludo, Ludovic Court=C3=A8s writes: > - posix_spawn_file_actions_addclose (actions, fd); > + /* Adding invalid file descriptors to an 'addclose' action leads > + to 'posix_spawn' failures on some operating systems: > + . Hence the extra check. */ > + int flags =3D fcntl (max_fd, F_GETFD, NULL); > + if ((flags >=3D 0) && ((flags & FD_CLOEXEC) =3D=3D 0)) > + posix_spawn_file_actions_addclose (actions, max_fd); > } I'm worried about TOCTOU in multi-threaded contexts here, which is why I opted for the heavy handed-approach here. In general I don't think we can know in advance which fdes to close :/ It's a shame that the posix_spawn actions fails on other kernels, since I don't really see a way to mitigate this problem (apart from the new posix_spawn_file_actions_addclosefrom_np as you mentioned). I don't know what we could do here. Maybe not provide spawn? Or provide it in spite of the broken fd closing? > - > - closedir (dirp); > } >=20=20 > static pid_t > @@ -1366,6 +1345,26 @@ do_spawn (char *exec_file, char **exec_argv, char = **exec_env, > posix_spawn_file_actions_t actions; > posix_spawnattr_t *attrp =3D NULL; >=20=20 > + posix_spawn_file_actions_init (&actions); > + > + /* Duplicate IN, OUT, and ERR unconditionally to clear their > + FD_CLOEXEC flag, if any. */ > + posix_spawn_file_actions_adddup2 (&actions, in, STDIN_FILENO); > + posix_spawn_file_actions_adddup2 (&actions, out, STDOUT_FILENO); > + posix_spawn_file_actions_adddup2 (&actions, err, STDERR_FILENO); This won't work, and actually this was one of the original logic bugs I was trying to fix. If err is equal to, let's say, STDIN_FILENO, then the first call will overwrite the initial file descriptor at STDIN_FILENO, and the second call won't do what the caller intended. This is why I was moving them out of the way first, so that they would not overwrite each other. > + /* TODO: Use 'closefrom' where available. */ > +#if 0 > + /* Version 2.34 of the GNU libc provides this function. */ > + posix_spawn_file_actions_addclosefrom_np (&actions, 3); > +#else > + if (in > 2) > + posix_spawn_file_actions_addclose (&actions, in); > + if (out > 2 && out !=3D in) > + posix_spawn_file_actions_addclose (&actions, out); > + if (err > 2 && err !=3D out && err !=3D in) > + posix_spawn_file_actions_addclose (&actions, err); Isn't this unneeded given we call close_inherited_fds below? > [...] > > Josselin: I simplified the =E2=80=98dup2=E2=80=99 logic somewhat. See my comments above. > Guile does that for file descriptors it opens internally, but > applications using =E2=80=98open-file=E2=80=99 without the recently-added= =E2=80=9Ce=E2=80=9D flag, or > =E2=80=98socket=E2=80=99 without =E2=80=98SOCK_CLOEXEC=E2=80=99, etc., en= d up with more file descriptors > that need to be taken care of. > > I wish the default were close-on-exec, but we=E2=80=99re not there yet. +1 Best, =2D-=20 Josselin Poiret --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQHEBAEBCAAuFiEEOSSM2EHGPMM23K8vUF5AuRYXGooFAmQjEXMQHGRldkBqcG9p cmV0Lnh5egAKCRBQXkC5Fhcail3ZC/4q3n0W2AOoRdNaXOzROGyxrfTd7dz3Ehx/ xEko14Ury/kPiV+x8MxIQXZ2Vo9m8LXTbuhhiLzeRNwS+fBwwnZQbEDDE4GvDkLd Vwh6IRmxKk3A5PxVqaX3EJypcwCABxMHzj1et3I/393na3UbPB/nVJaLAs27L/al XrFwd6k1aUGCPNYMPOVkulJXx6NNrOVcV1fSm23VlwpQzeeRUqVQBWWLPWi7MJ4n lnH+iwbLOAPIgywltXrCkf4rsook8KEBGWvQXSCgAqlCycdNIiBid7bTNaoitbeW HX5LP314uiMjSktOVVQrK+Q1+PhLJXUSGPxXRrxDTZf6/fXYAF4OrR343KjwxHJQ KWwC2yCENFkyHlh3RKkXkfmH41ezkaHQu2o64uD/1QcpFyv7Vx5Rde41pTPSEd8M 3umstB4vWNb2bzuBczeNXaoTFpAGKZHwXkGjfCWCZVEmyGBnbRm16VWxyPHtlvAp WizmvpOzWpM63lTajCdzwlPnckxPlfM= =pbCP -----END PGP SIGNATURE----- --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Wed Mar 29 18:30:25 2023 Received: (at 61095) by debbugs.gnu.org; 29 Mar 2023 22:30:26 +0000 Received: from localhost ([127.0.0.1]:55141 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheJF-0006aP-CG for submit@debbugs.gnu.org; Wed, 29 Mar 2023 18:30:25 -0400 Received: from eggs.gnu.org ([209.51.188.92]:58550) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheJD-0006a9-7D for 61095@debbugs.gnu.org; Wed, 29 Mar 2023 18:30:23 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pheJ4-0007dQ-6P; Wed, 29 Mar 2023 18:30:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:Date:References:Subject:To:From: in-reply-to; bh=MAIz/tfoyFa+10p5aXtGUBnOp0m/o3RAAoJDh9kX7ls=; b=fYxZpfzkWASNK q5N7kfq1eppY31NSzOqDARjJ7zSjzbxchvgGsu27cdhdM8AhTugKGV28H+4/skBXVSNtm5BfzhT8u SewD+QGg90rJShshAOQyAW+RlfGnsC3vSNOGR6NyF47yCZechLBgy7F5n7d2IAfunt5go1c6p6zUZ 8uWui7LWUyXZMbywnJa90RxgZ1VnNnSSjJnejnV1QSJlq0Kk/12Sba+12JX+lXZJGy2NsWxXAdxpo 1AECo5lg1oVhPbhijvr0+t8H/nXh0rZ4gw6t0gO9ppFYLNZxHnk5V1dcwr+dIth72ZGxShNScz/9Y BSjl5Ox1dzi1hKUtlia2g==; Received: from vpn-0-27.aquilenet.fr ([2a0c:e300:4:27::] helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pheIy-0004TB-4k; Wed, 29 Mar 2023 18:30:13 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Josselin Poiret Subject: Re: bug#61095: possible misuse of posix_spawn API on non-linux OSes References: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> <87zg7xgqxz.fsf@gnu.org> <87tty4svpo.fsf@jpoiret.xyz> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: Octidi 8 Germinal an 231 de la =?utf-8?Q?R=C3=A9volu?= =?utf-8?Q?tion=2C?= jour de la Jonquille X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Thu, 30 Mar 2023 00:30:04 +0200 Message-ID: <87zg7vjimr.fsf@inria.fr> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 61095 Cc: 61095@debbugs.gnu.org, Andrew Whatson , Omar Polo X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hi! Josselin Poiret skribis: > Ludovic Court=C3=A8s writes: > >> - posix_spawn_file_actions_addclose (actions, fd); >> + /* Adding invalid file descriptors to an 'addclose' action leads >> + to 'posix_spawn' failures on some operating systems: >> + . Hence the extra check. */ >> + int flags =3D fcntl (max_fd, F_GETFD, NULL); >> + if ((flags >=3D 0) && ((flags & FD_CLOEXEC) =3D=3D 0)) >> + posix_spawn_file_actions_addclose (actions, max_fd); >> } > > I'm worried about TOCTOU in multi-threaded contexts here, Yes, that=E2=80=99s a problem. The current /proc/self/fd optimization has = that problem too. :-/ > which is why I opted for the heavy handed-approach here. In general I > don't think we can know in advance which fdes to close :/ It's a shame > that the posix_spawn actions fails on other kernels, since I don't > really see a way to mitigate this problem (apart from the new > posix_spawn_file_actions_addclosefrom_np as you mentioned). I don't > know what we could do here. Maybe not provide spawn? Or provide it > in spite of the broken fd closing? Not providing =E2=80=98spawn=E2=80=99 is no longer an option. We can expect the problem to practically vanish =E2=80=9Csoon=E2=80=9D on G= NU variants with =E2=80=98closefrom=E2=80=99 (glibc 2.34 was released in Aug. 2021). On Linux with glibc < 2.34, we=E2=80=99d keep the current code (maybe witho= ut the /proc/self/fd optimization?). On other systems, we can have the racy code above as a last resort or OS-specific tricks, like Omar was suggesting for OpenBSD. It sucks but what else can we do? (BTW, reads: It shall not be considered an error for the fildes argument passed to these functions to specify a file descriptor for which the specified operation could not be performed at the time of the call. Any such error will be detected when the associated file actions object is later used during a posix_spawn() or posix_spawnp() operation. OpenBSD and GNU/Hurd follow this to the letter. OTOH =E2=80=98linux/spawni.c=E2=80=99 in glibc is purposefully more liberal: /* Signal errors only for file descriptors out of range. */ ) >> + /* Duplicate IN, OUT, and ERR unconditionally to clear their >> + FD_CLOEXEC flag, if any. */ >> + posix_spawn_file_actions_adddup2 (&actions, in, STDIN_FILENO); >> + posix_spawn_file_actions_adddup2 (&actions, out, STDOUT_FILENO); >> + posix_spawn_file_actions_adddup2 (&actions, err, STDERR_FILENO); > > This won't work, and actually this was one of the original logic bugs I > was trying to fix. If err is equal to, let's say, STDIN_FILENO, then > the first call will overwrite the initial file descriptor at > STDIN_FILENO, and the second call won't do what the caller intended. > This is why I was moving them out of the way first, so that they would > not overwrite each other. Oh, my bad. >> + /* TODO: Use 'closefrom' where available. */ >> +#if 0 >> + /* Version 2.34 of the GNU libc provides this function. */ >> + posix_spawn_file_actions_addclosefrom_np (&actions, 3); >> +#else >> + if (in > 2) >> + posix_spawn_file_actions_addclose (&actions, in); >> + if (out > 2 && out !=3D in) >> + posix_spawn_file_actions_addclose (&actions, out); >> + if (err > 2 && err !=3D out && err !=3D in) >> + posix_spawn_file_actions_addclose (&actions, err); > > Isn't this unneeded given we call close_inherited_fds below? No, because of the FD_CLOEXEC selection. Coming next is an updated patch series addressing this as proposed above. Let me know what y=E2=80=99all think! I tested the =E2=80=98posix_spawn_file_actions_addclosefrom_np=E2=80=99 pat= h by building in: guix time-machine --branch=3Dcore-updates -- shell -CP -D -f guix.scm =E2=80=A6 which gives us glibc 2.35. Ludo=E2=80=99. From debbugs-submit-bounces@debbugs.gnu.org Wed Mar 29 18:31:28 2023 Received: (at 61095) by debbugs.gnu.org; 29 Mar 2023 22:31:28 +0000 Received: from localhost ([127.0.0.1]:55146 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheKF-0006c7-V3 for submit@debbugs.gnu.org; Wed, 29 Mar 2023 18:31:28 -0400 Received: from eggs.gnu.org ([209.51.188.92]:44730) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheKE-0006br-Fw for 61095@debbugs.gnu.org; Wed, 29 Mar 2023 18:31:26 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pheK8-0007x9-FK; Wed, 29 Mar 2023 18:31:20 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:References:In-Reply-To:Date:Subject:To: From; bh=yjuh/vN49lyFeD/2ScYl7MpNxHxFOQ+r2zSHLKmzB3A=; b=jRtSa9OOgJh/5t6s2U2h kk87MrIyABqJjce+r0GqJ29Kidj9d80MauUBuoj/f+uQMSARohNdV7WS9oNbFu3HUe8hqj7eeOX2g szvF2ybaDJ2qLxrwQADCgOCi5sucqKVB9rb+DtkkCbTTYw6Qugp/GHQoGSs/qaz1pgzeHba0lcF7T n8pPOqLrGC36SMpMfBlqnGF5r98qpeR5Tb5SdSMuckMBVvmTPxvvgf6wIrcKnGsqEOzT6HSACiNS1 YKb/+z1cGCeuYP2kDkQGMCG1oTD8AkOW9xP60om4SHeUKhckifxrjDz3zQhA8F0tk3fKtcIaZMf22 eEFmrT6+dReE3A==; Received: from vpn-0-27.aquilenet.fr ([2a0c:e300:4:27::] helo=gnu.org) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pheK8-00015b-1K; Wed, 29 Mar 2023 18:31:20 -0400 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= To: 61095@debbugs.gnu.org Subject: [PATCH 1/3] 'spawn' closes only open file descriptors on non-GNU/Linux systems. Date: Thu, 30 Mar 2023 00:30:55 +0200 Message-Id: <20230329223057.28100-1-ludo@gnu.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <87zg7vjimr.fsf@inria.fr> References: <87zg7vjimr.fsf@inria.fr> MIME-Version: 1.0 X-Debbugs-Cc: Josselin Poiret , Omar Polo , Andrew Whatson Content-Transfer-Encoding: 8bit X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 61095 Cc: =?UTF-8?q?Ludovic=20Court=C3=A8s?= X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Fixes . Reported by Omar Polo . * libguile/posix.c (close_inherited_fds_slow): On systems other than GNU/Linux, call 'addclose' only when 'fcntl' succeeds on MAX_FD. --- libguile/posix.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/libguile/posix.c b/libguile/posix.c index 3a8be94e4..68e9bfade 100644 --- a/libguile/posix.c +++ b/libguile/posix.c @@ -1326,7 +1326,24 @@ static void close_inherited_fds_slow (posix_spawn_file_actions_t *actions, int max_fd) { while (--max_fd > 2) - posix_spawn_file_actions_addclose (actions, max_fd); + { + /* Adding a 'close' action for a file descriptor that is not open + causes 'posix_spawn' to fail on GNU/Hurd and on OpenBSD, but + not on GNU/Linux: . Hence this + strategy: + + - On GNU/Linux, close every FD, since that's the only + race-free way to make sure the child doesn't inherit one. + - On other systems, only close FDs currently open in the + parent; it works, but it's racy (XXX). + + The only reliable option is 'addclosefrom'. */ +#if ! (defined __GLIBC__ && defined __linux__) + int flags = fcntl (max_fd, F_GETFD, NULL); + if (flags >= 0) +#endif + posix_spawn_file_actions_addclose (actions, max_fd); + } } static void base-commit: e334e59589c3cbfc68d3f7d0d739000e0876b36d -- 2.39.2 From debbugs-submit-bounces@debbugs.gnu.org Wed Mar 29 18:31:33 2023 Received: (at 61095) by debbugs.gnu.org; 29 Mar 2023 22:31:33 +0000 Received: from localhost ([127.0.0.1]:55152 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheKL-0006cU-Ap for submit@debbugs.gnu.org; Wed, 29 Mar 2023 18:31:33 -0400 Received: from eggs.gnu.org ([209.51.188.92]:44736) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheKH-0006bv-Tr for 61095@debbugs.gnu.org; Wed, 29 Mar 2023 18:31:30 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pheKC-0007yD-N6; Wed, 29 Mar 2023 18:31:24 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:References:In-Reply-To:Date:Subject:To: From; bh=9A48uyemplKqgvzDip8m1sP7er/I6W9txwwc+lujWFw=; b=FhnX0w0YfjPLOUOMEvx+ 20DjZ4JcVW1JsHuK8Pj+ibBQ8Msd2NXTRDaNPz17eOnXS+iGnUj3ufk70AWflPSetA6jtqdwGPgGC yF1axWFi7CZDKT+dM3X2XahI1mk7guy3DHsBKgJ9RduqzPfv5JKTc4j4ntLGWf/y8wxRYq/WlV5It bNw/gnR246+6Qcba3L8YaUYZJLJvty5o+QtDBmpQ3GYfqpyshElxLnHJ6XKo8Tyy11kGwNMipT7o6 4zRb5TcIYLwE0lNZDLdKAq32xdhT67OhDWNU3umK43VubyuL5ZFS/hTnCO+KXMmuldl+TW5iW8qlp Lh2YFd6BRaCUPg==; Received: from vpn-0-27.aquilenet.fr ([2a0c:e300:4:27::] helo=gnu.org) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pheK8-00015b-N0; Wed, 29 Mar 2023 18:31:23 -0400 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= To: 61095@debbugs.gnu.org Subject: [PATCH 2/3] Remove racy optimized file descriptor closing loop in 'spawn'. Date: Thu, 30 Mar 2023 00:30:56 +0200 Message-Id: <20230329223057.28100-2-ludo@gnu.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230329223057.28100-1-ludo@gnu.org> References: <87zg7vjimr.fsf@inria.fr> <20230329223057.28100-1-ludo@gnu.org> MIME-Version: 1.0 X-Debbugs-Cc: Josselin Poiret , Omar Polo , Andrew Whatson Content-Transfer-Encoding: 8bit X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 61095 Cc: =?UTF-8?q?Ludovic=20Court=C3=A8s?= X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) This reverts 9332b632407894c2e1951cce1bc678f19e1fa8e4, thereby reinstating the performance issue in . This optimization was subject to race conditions in multi-threaded code: new file descriptors could pop up at any time and thus leak in the child. * libguile/posix.c (close_inherited_fds): Remove. (close_inherited_fds_slow): Rename to... (close_inherited_fds): ... this. --- libguile/posix.c | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/libguile/posix.c b/libguile/posix.c index 68e9bfade..b5830c43b 100644 --- a/libguile/posix.c +++ b/libguile/posix.c @@ -1323,7 +1323,7 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0, #endif /* HAVE_FORK */ static void -close_inherited_fds_slow (posix_spawn_file_actions_t *actions, int max_fd) +close_inherited_fds (posix_spawn_file_actions_t *actions, int max_fd) { while (--max_fd > 2) { @@ -1346,34 +1346,6 @@ close_inherited_fds_slow (posix_spawn_file_actions_t *actions, int max_fd) } } -static void -close_inherited_fds (posix_spawn_file_actions_t *actions, int max_fd) -{ - DIR *dirp; - struct dirent *d; - int fd; - - /* Try to use the platform-specific list of open file descriptors, so - we don't need to use the brute force approach. */ - dirp = opendir ("/proc/self/fd"); - - if (dirp == NULL) - return close_inherited_fds_slow (actions, max_fd); - - while ((d = readdir (dirp)) != NULL) - { - fd = atoi (d->d_name); - - /* Skip "." and "..", garbage entries, stdin/stdout/stderr. */ - if (fd <= 2) - continue; - - posix_spawn_file_actions_addclose (actions, fd); - } - - closedir (dirp); -} - static pid_t do_spawn (char *exec_file, char **exec_argv, char **exec_env, int in, int out, int err, int spawnp) -- 2.39.2 From debbugs-submit-bounces@debbugs.gnu.org Wed Mar 29 18:31:34 2023 Received: (at 61095) by debbugs.gnu.org; 29 Mar 2023 22:31:34 +0000 Received: from localhost ([127.0.0.1]:55154 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheKL-0006cX-Kn for submit@debbugs.gnu.org; Wed, 29 Mar 2023 18:31:34 -0400 Received: from eggs.gnu.org ([209.51.188.92]:44744) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheKJ-0006bx-19 for 61095@debbugs.gnu.org; Wed, 29 Mar 2023 18:31:31 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pheKD-0007yR-Q1; Wed, 29 Mar 2023 18:31:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:References:In-Reply-To:Date:Subject:To: From; bh=f+e3EZwIPoMN6TK98HeBx6eclBn838umUj+BHP93xys=; b=Rslhptbk+10gRfgSj6kf fF4Af7W49whJLxLtX63ZzspUN2Wpm8oGIIcaoeqYFTtGkwZKR7daMS8z3po1SzCNi42jx5zk8ogx4 Jn/YO46Yh30dCCP6GLxq7xATI6hAnHgzcIcJXMb+RCf1c88CHjiI8/vFPSX1rcJJnE0XEfwyou4BJ yUmFJeAoz3t7KsLdabcHmN0UcwVQHSjTT4uI1NFm/mz4+G9hYGrbnaubyxfva+QrUTb5X6mpXFJqr fRouo2tKYiT/twYmcsDDxPeZYsfB8DmDQUn62ztW3hbGr0vR4xlSa0yUq/hT+rA5trwhpt66fAljo jwxNXXTgXTf1jA==; Received: from vpn-0-27.aquilenet.fr ([2a0c:e300:4:27::] helo=gnu.org) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pheKC-00015b-Qy; Wed, 29 Mar 2023 18:31:25 -0400 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= To: 61095@debbugs.gnu.org Subject: [PATCH 3/3] Use 'posix_spawn_file_actions_addclosefrom_np' where available. Date: Thu, 30 Mar 2023 00:30:57 +0200 Message-Id: <20230329223057.28100-3-ludo@gnu.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230329223057.28100-1-ludo@gnu.org> References: <87zg7vjimr.fsf@inria.fr> <20230329223057.28100-1-ludo@gnu.org> MIME-Version: 1.0 X-Debbugs-Cc: Josselin Poiret , Omar Polo , Andrew Whatson Content-Transfer-Encoding: 8bit X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 61095 Cc: =?UTF-8?q?Ludovic=20Court=C3=A8s?= X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) * configure.ac: Check for 'posix_spawn_file_actions_addclosefrom_np'. * libguile/posix.c (HAVE_ADDCLOSEFROM): New macro. (close_inherited_fds): Wrap in #ifdef HAVE_ADDCLOSEFROM. (do_spawn) [HAVE_ADDCLOSEFROM]: Use 'posix_spawn_file_actions_addclosefrom_np'. --- configure.ac | 4 +++- libguile/posix.c | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index d5ce1c4ac..4a93be979 100644 --- a/configure.ac +++ b/configure.ac @@ -515,6 +515,7 @@ AC_CHECK_HEADERS([crt_externs.h]) # sched_getaffinity, sched_setaffinity - GNU extensions (glibc) # sendfile - non-POSIX, found in glibc # pipe2 - non-POSIX, found in glibc (GNU/Linux and GNU/Hurd) +# posix_spawn_file_actions_addclosefrom_np - glibc >= 2.34 # AC_CHECK_FUNCS([DINFINITY DQNAN cexp chsize clog clog10 ctermid \ fesetround ftime ftruncate fchown fchownat fchmod fchdir readlinkat \ @@ -528,7 +529,8 @@ AC_CHECK_FUNCS([DINFINITY DQNAN cexp chsize clog clog10 ctermid \ index bcopy rindex truncate isblank _NSGetEnviron \ strcoll_l strtod_l strtol_l newlocale uselocale utimensat \ fstatat futimens openat \ - sched_getaffinity sched_setaffinity sendfile pipe2]) + sched_getaffinity sched_setaffinity sendfile pipe2 + posix_spawn_file_actions_addclosefrom_np]) # The newlib C library uses _NL_ prefixed locale langinfo constants. AC_CHECK_DECLS([_NL_NUMERIC_GROUPING], [], [], [[#include ]]) diff --git a/libguile/posix.c b/libguile/posix.c index b5830c43b..3adc743c4 100644 --- a/libguile/posix.c +++ b/libguile/posix.c @@ -1322,6 +1322,12 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0, #undef FUNC_NAME #endif /* HAVE_FORK */ +#ifdef HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCLOSEFROM_NP +# define HAVE_ADDCLOSEFROM 1 +#endif + +#ifndef HAVE_ADDCLOSEFROM + static void close_inherited_fds (posix_spawn_file_actions_t *actions, int max_fd) { @@ -1346,6 +1352,8 @@ close_inherited_fds (posix_spawn_file_actions_t *actions, int max_fd) } } +#endif + static pid_t do_spawn (char *exec_file, char **exec_argv, char **exec_env, int in, int out, int err, int spawnp) @@ -1389,7 +1397,13 @@ do_spawn (char *exec_file, char **exec_argv, char **exec_env, posix_spawn_file_actions_adddup2 (&actions, fd_slot[1], 1); posix_spawn_file_actions_adddup2 (&actions, fd_slot[2], 2); +#ifdef HAVE_ADDCLOSEFROM + /* This function appears in glibc 2.34. It's both free from race + conditions and more efficient than the alternative. */ + posix_spawn_file_actions_addclosefrom_np (&actions, 3); +#else close_inherited_fds (&actions, max_fd); +#endif int res = -1; if (spawnp) -- 2.39.2 From debbugs-submit-bounces@debbugs.gnu.org Wed Mar 29 18:34:02 2023 Received: (at control) by debbugs.gnu.org; 29 Mar 2023 22:34:02 +0000 Received: from localhost ([127.0.0.1]:55170 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheMk-0006gw-DS for submit@debbugs.gnu.org; Wed, 29 Mar 2023 18:34:02 -0400 Received: from eggs.gnu.org ([209.51.188.92]:46494) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pheMi-0006gT-LA for control@debbugs.gnu.org; Wed, 29 Mar 2023 18:34:01 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pheMd-0008Pk-El for control@debbugs.gnu.org; Wed, 29 Mar 2023 18:33:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:Subject:From:To:Date:in-reply-to: references; bh=FKhZnlABGi15CHC8FQjhldajqky0Xun55pVzK35wrQQ=; b=jPBrR3XU4VzSo7 50z5JiOJQdbZ1z1fsg43DZKI0lIEDHAX99t1teIkHhYT9bHJd6rhiYecv2yw6WyyfJwlybgAwHXRh p56m+XocUHk4I71O88hh1z6oXhyM5Kh+k9R+zgYV0bvOzxIRNvs3059fkIj7TIV8ahOMIDR2CP2j7 8/Ioe74Cv5CgvVTwu9ZQ+tA+dUoos0cMMQrVNYIaLNWuVhhdhd0oBLKSbQrVO16iR7m4oZUfF5XWk tZ0+RwdhepVWNEgNMcYR2mKeN0aCh341Z/WmlP1hEZ9MJLBpEluvBAoqQx5FRjv5Z+MHpDbs4ebo+ SpJqu0UFiGtUoozNEjwg==; Received: from vpn-0-27.aquilenet.fr ([2a0c:e300:4:27::] helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pheMc-0001CG-RX for control@debbugs.gnu.org; Wed, 29 Mar 2023 18:33:55 -0400 Date: Thu, 30 Mar 2023 00:33:53 +0200 Message-Id: <87v8ijjige.fsf@gnu.org> To: control@debbugs.gnu.org From: =?utf-8?Q?Ludovic_Court=C3=A8s?= Subject: control message for bug #61095 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: control X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) tags 61095 + patch quit From debbugs-submit-bounces@debbugs.gnu.org Thu Mar 30 16:21:36 2023 Received: (at 61095) by debbugs.gnu.org; 30 Mar 2023 20:21:36 +0000 Received: from localhost ([127.0.0.1]:59512 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1phym7-0004GE-OF for submit@debbugs.gnu.org; Thu, 30 Mar 2023 16:21:35 -0400 Received: from jpoiret.xyz ([206.189.101.64]:53102) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1phym5-0004G5-7e for 61095@debbugs.gnu.org; Thu, 30 Mar 2023 16:21:33 -0400 Received: from authenticated-user (jpoiret.xyz [206.189.101.64]) by jpoiret.xyz (Postfix) with ESMTPA id 2A767184F0F; Thu, 30 Mar 2023 20:21:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jpoiret.xyz; s=dkim; t=1680207691; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=mUS7wkpxFO+/NZdUenFbAVwR2IUBTL0TOymQfD/BnBw=; b=uGTjXTlLY/c3bQ0phb9G44PHT+YjBJmYbFOphBRpEf7TIo/LoA6BEjwCljz9Qijfvk1x1S 0gqeXDT4toWAAoIGMVTQHdPvr/amIIgu+SLIR8txwMjn79S0QrwPEdnWnX6pWfMXsY3ZK0 DlgRi7+NHARPKWltQsbjJSNgn6hXSRp6njqCacdV/IdxeiTJgTcAc9GZ8BG9F0Rnnmt8wx y8JBMOxzoFp1dS3x3ctIh3YB5LH8IwCdJUHlSVn+1xklhuqNSdhpPmjigFmgb6jdEYAUjI OCY9rHs6bdryvvBJKnSBSS4mwm5rtq1PvWRysTFFgtC+NQMu62BMU4dJIe54mQ== From: Josselin Poiret To: Ludovic =?utf-8?Q?Court=C3=A8s?= Subject: Re: bug#61095: possible misuse of posix_spawn API on non-linux OSes In-Reply-To: <87zg7vjimr.fsf@inria.fr> References: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> <87zg7xgqxz.fsf@gnu.org> <87tty4svpo.fsf@jpoiret.xyz> <87zg7vjimr.fsf@inria.fr> Date: Thu, 30 Mar 2023 22:21:28 +0200 Message-ID: <87ileirnw7.fsf@jpoiret.xyz> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Authentication-Results: jpoiret.xyz; auth=pass smtp.auth=jpoiret@jpoiret.xyz smtp.mailfrom=dev@jpoiret.xyz X-Spamd-Bar: -- X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 61095 Cc: 61095@debbugs.gnu.org, Andrew Whatson , Omar Polo X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Ludo, Ludovic Court=C3=A8s writes: > Coming next is an updated patch series addressing this as proposed > above. Let me know what y=E2=80=99all think! > > I tested the =E2=80=98posix_spawn_file_actions_addclosefrom_np=E2=80=99 p= ath by building in: > > guix time-machine --branch=3Dcore-updates -- shell -CP -D -f guix.scm I didn't test, but this LGTM! Maybe someone on OpenBSD could test this patchset? Best, =2D-=20 Josselin Poiret --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQHEBAEBCAAuFiEEOSSM2EHGPMM23K8vUF5AuRYXGooFAmQl70gQHGRldkBqcG9p cmV0Lnh5egAKCRBQXkC5Fhcaika2C/4vZu9E2Yhc94DQV0ptmwlmFlAWlwSdvvbH +PPbHQwrY0rq5G1MqoxeegqVR2RU2o/mtP5edUcLlz7f6pbU8jPuPwDGAIaM5jLU gxrWXyadvbMk6HjaGm6T4lXhFvt4OT2dl8DoEPHzd+eqX/Tv9Xnx2j3kynMsY389 +RqBhQcLtH+Qk6Z9qvEYRXUrBYQhR7OVoKpu3Vg8tM6T50a7bgc9Xtnnxai/aqQW J3cJahq6oi1fkUaNiOAWsS7IN6XLJm01QZekkHdnAd9dJi1zJUdyP0/qIBjkXYXr ge3AYL3kxDWIus6zO3x8aWm4ZM57G0dw++VKU/qFora1BShSQCwg5J7496T5Q3fc sQ+zJzC27UnptMoTFZP2M3gJzNV/FH89KSVNSBGIe+8KVJY9INobr9fGNzUyR2mc bDaNvn9d9rRaRsvqpdAhaKGMtb1IwA06tWl262beYs9CDO7Lnx4XSEt4xyCeFpwg oU4RlegH8ChQtzf2O7J99CSjmcN7liU= =xCrc -----END PGP SIGNATURE----- --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Fri Mar 31 13:46:05 2023 Received: (at 61095) by debbugs.gnu.org; 31 Mar 2023 17:46:05 +0000 Received: from localhost ([127.0.0.1]:34435 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1piIpB-0002my-D8 for submit@debbugs.gnu.org; Fri, 31 Mar 2023 13:46:05 -0400 Received: from mail.omarpolo.com ([144.91.116.244]:65267) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1piIp9-0002mK-G6 for 61095@debbugs.gnu.org; Fri, 31 Mar 2023 13:46:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=omarpolo.com; s=20200327; t=1680284756; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gX0Qww+3KoTbXkWrUbtSd3lcncwlDY3TMRefTYiD8Fc=; b=XohcRwUgXdNHu4n+2v3TpotEa/iyesyvl6hNvHHeyM44ijE/4lYKBoJ8wEqa+5AR0Og9fF gooCCaTCo7/ZyABqPjDWApQ3PMVzykFqUpnAqhccd613Aw65e5J3p0UTRB3aWgAU2V5E0W pL1US4phsqXNCy27CNufa0sjjmP5x2g= Received: from localhost (host-82-61-20-205.retail.telecomitalia.it [82.61.20.205]) by mail.omarpolo.com (OpenSMTPD) with ESMTPSA id 9f3f726d (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 31 Mar 2023 19:45:56 +0200 (CEST) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id ce8644ed; Fri, 31 Mar 2023 19:45:55 +0200 (CEST) Date: Fri, 31 Mar 2023 19:45:55 +0200 To: Josselin Poiret Subject: Re: bug#61095: possible misuse of posix_spawn API on non-linux OSes From: Omar Polo References: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> <87zg7xgqxz.fsf@gnu.org> <87tty4svpo.fsf@jpoiret.xyz> <87zg7vjimr.fsf@inria.fr> <87ileirnw7.fsf@jpoiret.xyz> In-Reply-To: <87ileirnw7.fsf@jpoiret.xyz> Message-Id: <2KFIQJHFVA0GP.2GKMWUIGIVCMU@venera> User-Agent: mblaze/1.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 61095 Cc: Ludovic =?UTF-8?Q?Court=C3=A8s?= , Andrew Whatson , 61095@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) On 2023/03/30 22:21:28 +0200, Josselin Poiret wrote: > Hi Ludo, >=20 > Ludovic Court=C3=A8s writes: >=20 > > Coming next is an updated patch series addressing this as proposed > > above. Let me know what y=E2=80=99all think! > > > > I tested the =E2=80=98posix_spawn_file_actions_addclosefrom_np=E2=80=99= path by building in: > > > > guix time-machine --branch=3Dcore-updates -- shell -CP -D -f guix.scm= >=20 > I didn't test, but this LGTM! Maybe someone on OpenBSD could test this > patchset? % gmake check gmake[5]: Entering directory '/home/op/w/guile/test-suite/standalone' PASS: test-system-cmds it seems to work on OpenBSD 7.3 :) but note that our libc doesn't have posix_spawn_file_actions_addclosefrom_n= p, so this is using the "racy" code path. Just for curiosity, as it's outside the scope of the bug, what's the reason posix_spawn was used instead of a more classic fork() + closefrom()? Thanks, Omar Polo From debbugs-submit-bounces@debbugs.gnu.org Sun Apr 02 09:44:16 2023 Received: (at 61095-done) by debbugs.gnu.org; 2 Apr 2023 13:44:16 +0000 Received: from localhost ([127.0.0.1]:39371 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1piy0G-00019g-3d for submit@debbugs.gnu.org; Sun, 02 Apr 2023 09:44:16 -0400 Received: from eggs.gnu.org ([209.51.188.92]:37574) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1piy0D-00019O-7o for 61095-done@debbugs.gnu.org; Sun, 02 Apr 2023 09:44:14 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1piy04-0007yy-9I; Sun, 02 Apr 2023 09:44:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:In-Reply-To:Date:References:Subject:To: From; bh=klFypizrwT6+3SScZ2kp1dW/2+AQPwgYQETQjWstCTw=; b=B9+eCEdI8nRscqAoqj7z ajGMeg5Z/64d5Zb202wceAMm+IRMcm6xt0MGMEuvJY5nEgO9hGYprN9+NvVMyPQx0eKLFm+b07mca gF6aT8TlsM5NKucYU8GhXbAK6EXg3Ta5vtj0MGrIPg1yNGdJxgTqpHFzoSQp6OC3RNhurBfyBNayg cRPLaK2pyaJi5iMxYoBS5eB8h3tEGlXfF3CI4gdGf29HMY+E3ZcCxyPrI1t4zH4gAL2e/ypFZRWVV tsT6LrxjA/RYCELfd42TW8PHjnNalYdbrtsjGKzhW5Iow2sb1yLp9fA6z8DHLjTFU61McW1+Bq6co Sh5goKabDKtZDA==; Received: from 91-160-117-201.subs.proxad.net ([91.160.117.201] helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1piy03-000208-Gw; Sun, 02 Apr 2023 09:44:03 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Omar Polo Subject: Re: bug#61095: possible misuse of posix_spawn API on non-linux OSes References: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> <87zg7xgqxz.fsf@gnu.org> <87tty4svpo.fsf@jpoiret.xyz> <87zg7vjimr.fsf@inria.fr> <87ileirnw7.fsf@jpoiret.xyz> <2KFIQJHFVA0GP.2GKMWUIGIVCMU@venera> Date: Sun, 02 Apr 2023 15:44:01 +0200 In-Reply-To: <2KFIQJHFVA0GP.2GKMWUIGIVCMU@venera> (Omar Polo's message of "Fri, 31 Mar 2023 19:45:55 +0200") Message-ID: <87zg7qtn4u.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 61095-done Cc: Josselin Poiret , Andrew Whatson , 61095-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hi! Omar Polo skribis: > On 2023/03/30 22:21:28 +0200, Josselin Poiret wrote: >> Hi Ludo, >>=20 >> Ludovic Court=C3=A8s writes: >>=20 >> > Coming next is an updated patch series addressing this as proposed >> > above. Let me know what y=E2=80=99all think! >> > >> > I tested the =E2=80=98posix_spawn_file_actions_addclosefrom_np=E2=80= =99 path by building in: >> > >> > guix time-machine --branch=3Dcore-updates -- shell -CP -D -f guix.scm >>=20 >> I didn't test, but this LGTM! Maybe someone on OpenBSD could test this >> patchset? > > % gmake check > > gmake[5]: Entering directory '/home/op/w/guile/test-suite/standalone' > PASS: test-system-cmds > > it seems to work on OpenBSD 7.3 :) Awesome! Pushed as 9cc85a4f52147fcdaa4c52a62bcc87bdb267d0a9. > but note that our libc doesn't have posix_spawn_file_actions_addclosefrom= _np, > so this is using the "racy" code path. Yeah, not great. :-/ I hope that function will be adopted by other libcs, especially since =E2=80=98closefrom=E2=80=99 is already available. > Just for curiosity, as it's outside the scope of the bug, what's the > reason posix_spawn was used instead of a more classic fork() + > closefrom()? There=E2=80=99s a long discussion at: https://issues.guix.gnu.org/52835 Essentially, =E2=80=98fork=E2=80=99 is unusable in multi-threaded context, = in addition to being inefficient. Thanks, Ludo=E2=80=99. From unknown Fri Aug 15 20:29:11 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Mon, 01 May 2023 11:24:12 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator