PR/60275 CVS commit: [netbsd-11] src/bin/sh

["Martin Husemann" <martin%netbsd.org@localhost>]

The following reply was made to PR bin/60275; it has been noted by GNATS.

From: "Martin Husemann" <martin%netbsd.org@localhost>
To: gnats-bugs%gnats.NetBSD.org@localhost
Cc: 
Subject: PR/60275 CVS commit: [netbsd-11] src/bin/sh
Date: Wed, 3 Jun 2026 18:21:41 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Wed Jun  3 18:21:41 UTC 2026
 
 Modified Files:
 	src/bin/sh [netbsd-11]: eval.c eval.h jobs.c jobs.h trap.c
 
 Log Message:
 Pull up following revision(s) (requested by kre in ticket #294):
 
 	bin/sh/trap.c: revision 1.60
 	bin/sh/trap.c: revision 1.61
 	bin/sh/trap.c: revision 1.62
 	bin/sh/jobs.h: revision 1.28
 	bin/sh/eval.h: revision 1.25
 	bin/sh/eval.c: revision 1.198
 	bin/sh/eval.c: revision 1.199
 	bin/sh/jobs.c: revision 1.126
 	bin/sh/trap.c: revision 1.59
 
 PR bin/60275 discard less arriving signals
 
 If the signal state is returned to the default state (SIG_DFL)
 while sh has a pending signal for the process, that it is not
 yet ready to process, then forget the pending signal, and instead
 send it to ourself, so the kernel can take whatever is the default
 action for that signal (if it is just ignored, then we just keep
 on processing, if it kills us, well, it was nice while it lasted!)
 
 PR bin/60275 discard even less arriving signals
 
 Avoid signals arriving immediately after a fork() (or vfork())
 by blocking everything (everything possible) while the fork()
 happens, in the parent, for (close to) the minimum possible time,
 in the child, until it has its state init'd enough that it is
 safe for signals to arrive.
 
 Further, if a signal does arrive (in a child) which was trapped
 in the parent, but hasn't been cleaned up fully yet, instead of
 simply ignoring it, send it to ourselves, after setting its state
 to SIG_DFL (which is what would eventually happen to a trapped
 signal anyway).   If that doesn't kill us, then we will end up
 (harmlessly) setting the state to SIG_DFL again later as would happen
 if the signal hadn't arrived in this short window; we cannot record that
 it happened to avoid that, as we might be in a vforked child, and
 anything recorded by that would be visible back in the parent later
 (where the signal action was not changed).
 
 Always use raise(N) rather than kill(getpid(), N) - just to save an
 unnecessary getpid() sys call.  Of course, the current implementation
 of raise() does much the same (just _lwp_self() rather than getpid())
 but one can imagine a future where raise() was raised to the status of
 being a sys call of its own (it would be a trivial one to implement).
 
 After this change, the previous change to trap.c is probably no longer
 needed (will never be invoked) as the situation which allowed a (parent)
 trapped signal to be received in the child, without immediately being
 ignored by onsig() should no longer be possible (the signal blocking
 should prevent that).   However it is very small, correct if it were to
 be invoked, and harmless otherwise.
 
 PR bin/60275 discard some arriving signals
 
 The PR is only peripherally relevant to this, but it is all much
 the same problem, over a fork() trapped signals are maintained,
 and sh does not really want that.
 
 In this case, when there is a vfork() a signal arriving for a
 child (whether or not it should arrive and be processed) can be
 treated as if it arrived for the parent, and cause a trap action
 to be executed by the parent.   (Never observed to have happened,
 as best I am aware, but certainly looks as if it could.)
 
 Avoid that, by making sure that the child process never records
 a signal as having occurred, when it is being a vfork child
 (while the parent is sharing memory with it).
 
 Doing this meant making one variable that was previously local
 to eval.c globally visible (exposing it in eval.h), and then
 because the same name is used as a parameter in many other
 functions, changing the actual variable name (just to avoid
 potential compiler "shadowing" warnings) - which amounts to
 most of the actual change here.
 
 Then, make sure that variable is only ever set in the parent
 while signals are blocked (for the actual fix for this PR),
 so the parent in a signal handler will never see it set.
 
 In the signal handler, treat that variable, if set, as
 indicating that the child should not have received the signal
 (it should have been, and will be soon, reset to SIG_DFL)
 and continue to treat it as in the previous fix.   Under no
 circumstances record it as having been received, or the
 memory shared with the parent will cause the parent to later
 detect the signal as having been sent to it.
 
 Comments added explaining things where appropriate.
 
 PR bin/60275  one more (should be the last) correction
 
 It wasn't possible to do this, previously, but after the 3rd
 in the sequence, we can (it should have been included with that one).
 
 Never even look at gotsig[signo] in a vforked child - it will
 be reflecting the state of the parent, not the child, so has
 nothing to do with the child (this wasn't true until after the
 3rd fix, which made sure that the child after a vfork doesn't
 step on the parent's memory).
 
 This will fix an unbelievably unlikely situation, where a
 trapped signal arrives at the parent immediately before it
 vforks() a child, which happens before the trap is executed.
 
 Depending upon which of the various fixes (or none of them)
 to this PR have been included various incorrect things might
 have happened - the child might have processed the signal
 trap instead of the parent, the signal might be lost, or even
 processed twice....   Now the signal will be left for the
 parent to process.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.197 -r1.197.2.1 src/bin/sh/eval.c
 cvs rdiff -u -r1.24 -r1.24.2.1 src/bin/sh/eval.h
 cvs rdiff -u -r1.124 -r1.124.2.1 src/bin/sh/jobs.c
 cvs rdiff -u -r1.27 -r1.27.4.1 src/bin/sh/jobs.h
 cvs rdiff -u -r1.58 -r1.58.2.1 src/bin/sh/trap.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.