From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 6917E24273 for ; Thu, 29 Feb 2024 16:35:42 +0100 (CET) Received: (qmail 5258 invoked by uid 550); 29 Feb 2024 15:31:59 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 5206 invoked from network); 29 Feb 2024 15:31:59 -0000 Date: Thu, 29 Feb 2024 10:35:46 -0500 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20240229153546.GU4163@brightrain.aerifal.cx> References: <985e15962c164eb3076752d6ee4c05fe@ispras.ru> <20210524203329.GB2546@brightrain.aerifal.cx> <47bc2113ebf931665b6b88795159de2e@ispras.ru> <20210525143210.GH2546@brightrain.aerifal.cx> <4f0953e3c103fac9cf01059781742644@ispras.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4f0953e3c103fac9cf01059781742644@ispras.ru> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] Potentially infinite loop in posix_spawn'ed child On Thu, Feb 29, 2024 at 05:03:51PM +0300, Alexey Izbyshev wrote: > On 2021-05-25 17:32, Rich Felker wrote: > >On Tue, May 25, 2021 at 09:30:18AM +0300, Alexey Izbyshev wrote: > >>On 2021-05-24 23:33, Rich Felker wrote: > >>>On Mon, May 24, 2021 at 01:09:21PM +0300, Alexey Izbyshev wrote: > >>>>Hi, > >>>> > >>>>I've noticed the following loop at https://git.musl-libc.org/cgit/musl/tree/src/process/posix_spawn.c#n159: > >>>> > >>>> exec(args->path, args->argv, args->envp); > >>>> ret = -errno; > >>>> > >>>>fail: > >>>> /* Since sizeof errno < PIPE_BUF, the write is atomic. */ > >>>> ret = -ret; > >>>> if (ret) while (__syscall(SYS_write, p, &ret, sizeof ret) < 0); > >>>> _exit(127); > >>>> > >>>>Is there any reason that write is done in a loop? If SIGPIPE is > >>>>blocked or ignored and the parent dies before this point, the child > >>>>will spin in it forever. > >>> > >>>I suppose the special case of EPIPE should be considered here as no > >>>need to inform the parent. Are there any other errors that should be > >>>treated specially? > >>> > >>I'm not aware of any other errors that would need treatment. Is this > >>loop intended to be a detection/debugging aid in case of an > >>unexpected error? > > > >It's not a debugging aid so much as a guarantee against forward > >progress doing the wrong thing (wrongly reporting success to the > >parent when the execve failed). I don't think there are any errors > >that should be able to happen here aside from EPIPE though, short of > >munging with syscall semantics using seccomp or something which is > >outside the scope of what could be expected to work correctly. > > > I've never sent a patch for this, doing it now. > > Thanks, > Alexey > From 36eda01dbe0a35c4c65c394723a70c2d1b75e591 Mon Sep 17 00:00:00 2001 > From: Alexey Izbyshev > Date: Thu, 29 Feb 2024 14:13:18 +0300 > Subject: [PATCH] posix_spawn: fix child spinning on write to a broken pipe > Mail-Followup-To: musl@lists.openwall.com > > A child process created by posix_spawn reports errors to its parent via > a pipe, retrying infinitely on any write error to prevent falsely > reporting success. If the (original) parent dies before write is > attempted, there is nobody to report to, but the child will remain > stuck in the write loop forever if SIGPIPE is blocked or ignored. > Fix this by not retrying write if it fails with EPIPE. > --- > src/process/posix_spawn.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/process/posix_spawn.c b/src/process/posix_spawn.c > index 728551b36792..8294598bb7e3 100644 > --- a/src/process/posix_spawn.c > +++ b/src/process/posix_spawn.c > @@ -4,6 +4,7 @@ > #include > #include > #include > +#include > #include > #include "syscall.h" > #include "lock.h" > @@ -156,7 +157,11 @@ static int child(void *args_vp) > fail: > /* Since sizeof errno < PIPE_BUF, the write is atomic. */ > ret = -ret; > - if (ret) while (__syscall(SYS_write, p, &ret, sizeof ret) < 0); > + if (ret) { > + int r; > + do r = __syscall(SYS_write, p, &ret, sizeof ret); > + while (r<0 && r!=-EPIPE); > + } > _exit(127); > } > > -- > 2.39.2 > Thanks, applying!