[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 29 Feb 2024 10:35:46 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: 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 <izbyshev@...ras.ru>
> 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@...ts.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 <unistd.h>
> #include <signal.h>
> #include <fcntl.h>
> +#include <errno.h>
> #include <sys/wait.h>
> #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!
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
