From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Newsgroups: gmane.lisp.guile.bugs Subject: bug#61095: possible misuse of posix_spawn API on non-linux OSes Date: Thu, 30 Mar 2023 00:30:04 +0200 Message-ID: <87zg7vjimr.fsf@inria.fr> References: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> <87zg7xgqxz.fsf@gnu.org> <87tty4svpo.fsf@jpoiret.xyz> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="20658"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) Cc: 61095@debbugs.gnu.org, Andrew Whatson , Omar Polo To: Josselin Poiret Original-X-From: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Thu Mar 30 00:31:21 2023 Return-path: Envelope-to: guile-bugs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pheK9-0005Cj-HX for guile-bugs@m.gmane-mx.org; Thu, 30 Mar 2023 00:31:21 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pheJs-0001Hw-WA; Wed, 29 Mar 2023 18:31:05 -0400 Original-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 1pheJq-0001GU-Td for bug-guile@gnu.org; Wed, 29 Mar 2023 18:31:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pheJq-0007v9-Jx for bug-guile@gnu.org; Wed, 29 Mar 2023 18:31:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pheJq-0006bG-7b for bug-guile@gnu.org; Wed, 29 Mar 2023 18:31:02 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: <26OIN3L5D4V9L.2M0KM95K0YSNM@venera> Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: bug-guile@gnu.org Resent-Date: Wed, 29 Mar 2023 22:31:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 61095 X-GNU-PR-Package: guile Original-Received: via spool by 61095-submit@debbugs.gnu.org id=B61095.168012902625325 (code B ref 61095); Wed, 29 Mar 2023 22:31:02 +0000 Original-Received: (at 61095) by debbugs.gnu.org; 29 Mar 2023 22:30:26 +0000 Original-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 Original-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 Original-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==; Original-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 X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: Octidi 8 Germinal an 231 de la =?UTF-8?Q?R=C3=A9volution, ?= 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 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-guile@gnu.org List-Id: "Bug reports for GUILE, GNU's Ubiquitous Extension Language" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Original-Sender: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.lisp.guile.bugs:10585 Archived-At: 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.