dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Denys Vlasenko <vda.linux@googlemail.com>
To: Harald van Dijk <harald@gigawatt.nl>
Cc: Robert Elz <kre@munnari.oz.au>, Martijn Dekker <martijn@inlv.org>,
	busybox <busybox@busybox.net>,
	DASH shell mailing list <dash@vger.kernel.org>,
	Jilles Tjoelker <jilles@stack.nl>,
	Bug reports for the GNU Bourne Again SHell <bug-bash@gnu.org>
Subject: Re: Bizarre interaction bug involving bash w/ lastpipe + Almquist 'wait'
Date: Tue, 18 Feb 2020 17:46:23 +0100	[thread overview]
Message-ID: <CAK1hOcO_S_T=5SWJ0jpZWxDYwdUFqJisw_nC+JysnQvZ6XUuKw@mail.gmail.com> (raw)
In-Reply-To: <2b436500-671b-b143-a4bb-2230f157e1b7@gigawatt.nl>

On Sat, Feb 8, 2020 at 7:41 PM Harald van Dijk <harald@gigawatt.nl> wrote:
> I changed gwsh to call sigclearmask() on shell startup, but plan to
> check whether this loop is really necessary at some later time. It was
> added to dash to fix a race condition, where that race condition was
> apparently introduced by a fix for another race condition.

sigsuspend() is needed to make "wait" builtin interruptible by signals.

Attempts to use EINTR error return of waitpid() a-la:

               if (got_sigs) { handle signals }
               got_sigs = 0;
               pid = waitpid(...);  /* without WNOHANG */
               if (pid < 0 && errno == EINTR) { handle signals }

are racy, since signals can be delivered not only while waitpid() syscall
is in kernel, but also when we are only about to enter the kernel
- and in this case, the shell's sighandler will set the flag variable,
but then we enter the kernel and sleep.

Masking signals doesn't help, since you need to unmask them just before
waitpid() if you want to get EINTR on a signal, hence there is still
a window for the race.

> If NetBSD sh
> manages to avoid this pattern, and assuming NetBSD sh is not still
> susceptible to one of those race conditions

Please let us know what you discovered.

  parent reply	other threads:[~2020-02-18 16:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-06 16:12 Martijn Dekker
2020-02-06 19:29 ` Harald van Dijk
2020-02-07 11:19   ` AW: " Walter Harms
2020-02-07 14:33     ` Martijn Dekker
2020-02-07 16:16   ` Chet Ramey
2020-02-06 20:43 ` Robert Elz
2020-02-07  2:41 ` Robert Elz
2020-02-08 18:39   ` Harald van Dijk
2020-02-09 19:03     ` Jilles Tjoelker
2020-02-18 16:46     ` Denys Vlasenko [this message]
2020-02-18 21:59       ` Harald van Dijk
2020-02-18 18:17     ` Robert Elz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAK1hOcO_S_T=5SWJ0jpZWxDYwdUFqJisw_nC+JysnQvZ6XUuKw@mail.gmail.com' \
    --to=vda.linux@googlemail.com \
    --cc=bug-bash@gnu.org \
    --cc=busybox@busybox.net \
    --cc=dash@vger.kernel.org \
    --cc=harald@gigawatt.nl \
    --cc=jilles@stack.nl \
    --cc=kre@munnari.oz.au \
    --cc=martijn@inlv.org \
    --subject='Re: Bizarre interaction bug involving bash w/ lastpipe + Almquist '\''wait'\''' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).