Mitigated race condition of SYSTEM,EXEC child processes

This commit is contained in:
Gerhard Rieger 2020-12-28 11:10:03 +01:00
parent 2e0b0a0eff
commit f34e8a4dc1
5 changed files with 45 additions and 55 deletions

View file

@ -18,6 +18,9 @@ Corrections:
building failed on Solaris 9. building failed on Solaris 9.
Thanks to Greg Earle for reporting this issue and providing a patch. Thanks to Greg Earle for reporting this issue and providing a patch.
Mitigated race condition of quickly terminating SYSTEM or EXEC child
processes.
Porting: Porting:
In gcc version 10 the default changed from -fcommon to -fno-common. In gcc version 10 the default changed from -fcommon to -fno-common.
Consequently, linking filan and procan failed with error Consequently, linking filan and procan failed with error

58
socat.c
View file

@ -597,29 +597,20 @@ int socat(const char *address1, const char *address2) {
(XIO_RDSTREAM(sock1)->howtoend == END_KILL || (XIO_RDSTREAM(sock1)->howtoend == END_KILL ||
XIO_RDSTREAM(sock1)->howtoend == END_CLOSE_KILL || XIO_RDSTREAM(sock1)->howtoend == END_CLOSE_KILL ||
XIO_RDSTREAM(sock1)->howtoend == END_SHUTDOWN_KILL)) { XIO_RDSTREAM(sock1)->howtoend == END_SHUTDOWN_KILL)) {
if (XIO_RDSTREAM(sock1)->para.exec.pid == diedunknown1) { int i;
for (i = 0; i < NUMUNKNOWN; ++i) {
if (XIO_RDSTREAM(sock1)->para.exec.pid == diedunknown[i]) {
/* child has alread died... but it might have put regular data into /* child has alread died... but it might have put regular data into
the communication channel, so continue */ the communication channel, so continue */
Info1("child "F_pid" has already died (diedunknown1)", Info2("child "F_pid" has already died with status %d",
XIO_RDSTREAM(sock1)->para.exec.pid); XIO_RDSTREAM(sock1)->para.exec.pid, statunknown[i]);
diedunknown1 = 0; if (statunknown[i] != 0) {
return 1;
}
diedunknown[i] = 0;
XIO_RDSTREAM(sock1)->para.exec.pid = 0; XIO_RDSTREAM(sock1)->para.exec.pid = 0;
/* return STAT_RETRYLATER; */ /* return STAT_RETRYLATER; */
} else if (XIO_RDSTREAM(sock1)->para.exec.pid == diedunknown2) { }
Info1("child "F_pid" has already died (diedunknown2)",
XIO_RDSTREAM(sock1)->para.exec.pid);
diedunknown2 = 0;
XIO_RDSTREAM(sock1)->para.exec.pid = 0;
} else if (XIO_RDSTREAM(sock1)->para.exec.pid == diedunknown3) {
Info1("child "F_pid" has already died (diedunknown3)",
XIO_RDSTREAM(sock1)->para.exec.pid);
diedunknown3 = 0;
XIO_RDSTREAM(sock1)->para.exec.pid = 0;
} else if (XIO_RDSTREAM(sock1)->para.exec.pid == diedunknown4) {
Info1("child "F_pid" has already died (diedunknown4)",
XIO_RDSTREAM(sock1)->para.exec.pid);
diedunknown4 = 0;
XIO_RDSTREAM(sock1)->para.exec.pid = 0;
} }
} }
#endif #endif
@ -648,29 +639,20 @@ int socat(const char *address1, const char *address2) {
(XIO_RDSTREAM(sock2)->howtoend == END_KILL || (XIO_RDSTREAM(sock2)->howtoend == END_KILL ||
XIO_RDSTREAM(sock2)->howtoend == END_CLOSE_KILL || XIO_RDSTREAM(sock2)->howtoend == END_CLOSE_KILL ||
XIO_RDSTREAM(sock2)->howtoend == END_SHUTDOWN_KILL)) { XIO_RDSTREAM(sock2)->howtoend == END_SHUTDOWN_KILL)) {
if (XIO_RDSTREAM(sock2)->para.exec.pid == diedunknown1) { int i;
for (i = 0; i < NUMUNKNOWN; ++i) {
if (XIO_RDSTREAM(sock2)->para.exec.pid == diedunknown[i]) {
/* child has alread died... but it might have put regular data into /* child has alread died... but it might have put regular data into
the communication channel, so continue */ the communication channel, so continue */
Info1("child "F_pid" has already died (diedunknown1)", Info2("child "F_pid" has already died with status %d",
XIO_RDSTREAM(sock2)->para.exec.pid); XIO_RDSTREAM(sock2)->para.exec.pid, statunknown[i]);
diedunknown1 = 0; if (statunknown[i] != 0) {
return 1;
}
diedunknown[i] = 0;
XIO_RDSTREAM(sock2)->para.exec.pid = 0; XIO_RDSTREAM(sock2)->para.exec.pid = 0;
/* return STAT_RETRYLATER; */ /* return STAT_RETRYLATER; */
} else if (XIO_RDSTREAM(sock2)->para.exec.pid == diedunknown2) { }
Info1("child "F_pid" has already died (diedunknown2)",
XIO_RDSTREAM(sock2)->para.exec.pid);
diedunknown2 = 0;
XIO_RDSTREAM(sock2)->para.exec.pid = 0;
} else if (XIO_RDSTREAM(sock2)->para.exec.pid == diedunknown3) {
Info1("child "F_pid" has already died (diedunknown3)",
XIO_RDSTREAM(sock2)->para.exec.pid);
diedunknown3 = 0;
XIO_RDSTREAM(sock2)->para.exec.pid = 0;
} else if (XIO_RDSTREAM(sock2)->para.exec.pid == diedunknown4) {
Info1("child "F_pid" has already died (diedunknown4)",
XIO_RDSTREAM(sock2)->para.exec.pid);
diedunknown4 = 0;
XIO_RDSTREAM(sock2)->para.exec.pid = 0;
} }
} }
#endif #endif

18
test.sh
View file

@ -8520,16 +8520,17 @@ esac
N=$((N+1)) N=$((N+1))
# test the shut-null and null-eof options # Test the shut-null and null-eof options
NAME=SHUTNULLEOF NAME=SHUTNULLEOF
case "$TESTS" in case "$TESTS" in
*%$N%*|*%functions%*|*%socket%*|*%$NAME%*) *%$N%*|*%functions%*|*%socket%*|*%$NAME%*)
TEST="$NAME: options shut-null and null-eof" TEST="$NAME: options shut-null and null-eof"
# run a receiving background process with option null-eof. # Run a receiving background process with option null-eof.
# start a sending process with option shut-null that sends a test record to the # Start a sending process with option shut-null that sends a test record to the
# receiving process and then terminates. # receiving process and then terminates.
# send another test record. # Send another test record.
# whe the receiving process just got the first test record the test succeeded # When the receiving process only received and stored the first test record the
# test succeeded
if ! eval $NUMCOND; then :; else if ! eval $NUMCOND; then :; else
tf="$td/test$N.stout" tf="$td/test$N.stout"
te="$td/test$N.stderr" te="$td/test$N.stderr"
@ -8541,9 +8542,9 @@ printf "test $F_n $TEST... " $N
$CMD0 >/dev/null 2>"${te}0" & $CMD0 >/dev/null 2>"${te}0" &
pid0=$! pid0=$!
waitudp4port $PORT 1 waitudp4port $PORT 1
echo "$da" |$CMD1 >"${tf}1" 2>"${te}1" { echo "$da"; sleep 0.1; } |$CMD1 >"${tf}1" 2>"${te}1"
rc1=$? rc1=$?
echo "xyz" |$CMD1 >"${tf}2" 2>"${te}2" { echo "xyz"; sleep 0.1; } |$CMD1 >"${tf}2" 2>"${te}2"
rc2=$? rc2=$?
kill $pid0 2>/dev/null; wait kill $pid0 2>/dev/null; wait
if [ $rc1 != 0 -o $rc2 != 0 ]; then if [ $rc1 != 0 -o $rc2 != 0 ]; then
@ -12161,7 +12162,7 @@ case "$TESTS" in
TEST="$NAME: correct evaluation of range mask 0" TEST="$NAME: correct evaluation of range mask 0"
if ! eval $NUMCOND; then :; if ! eval $NUMCOND; then :;
elif [ -z "$SECONDADDR" ]; then elif [ -z "$SECONDADDR" ]; then
# we need access to a second addresses # we need access to a second address
$PRINTF "test $F_n $TEST... ${YELLOW}need a second IPv4 address${NORMAL}\n" $N $PRINTF "test $F_n $TEST... ${YELLOW}need a second IPv4 address${NORMAL}\n" $N
numCANT=$((numCANT+1)) numCANT=$((numCANT+1))
listCANT="$listCANT $N" listCANT="$listCANT $N"
@ -12179,6 +12180,7 @@ pid0=$!
waittcp4port $PORT 1 waittcp4port $PORT 1
echo "$da" |$CMD1 >"${tf}1" 2>"${te}1" echo "$da" |$CMD1 >"${tf}1" 2>"${te}1"
rc1=$? rc1=$?
sleep 1
kill $pid0 2>/dev/null; wait kill $pid0 2>/dev/null; wait
if [ $rc1 != 0 ]; then if [ $rc1 != 0 ]; then
$PRINTF "${YELLOW}invocation failed${NORMAL}\n" $PRINTF "${YELLOW}invocation failed${NORMAL}\n"

1
xio.h
View file

@ -416,6 +416,7 @@ extern pid_t diedunknown[NUMUNKNOWN]; /* child died before it is registered */
#define diedunknown2 (diedunknown[1]) #define diedunknown2 (diedunknown[1])
#define diedunknown3 (diedunknown[2]) #define diedunknown3 (diedunknown[2])
#define diedunknown4 (diedunknown[3]) #define diedunknown4 (diedunknown[3])
extern int statunknown[NUMUNKNOWN]; /* exit state of unknown dead child */
extern int xiosetsigchild(xiofile_t *xfd, int (*callback)(struct single *)); extern int xiosetsigchild(xiofile_t *xfd, int (*callback)(struct single *));
extern int xiosetchilddied(void); extern int xiosetchilddied(void);

View file

@ -11,6 +11,7 @@
/*!! with socat, at most 4 exec children exist */ /*!! with socat, at most 4 exec children exist */
pid_t diedunknown[NUMUNKNOWN]; /* children that died before they were registered */ pid_t diedunknown[NUMUNKNOWN]; /* children that died before they were registered */
int statunknown[NUMUNKNOWN]; /* exit state of unknown dead child */
size_t nextunknown; size_t nextunknown;
@ -120,7 +121,8 @@ void childdied(int signum) {
if (nextunknown == NUMUNKNOWN) { if (nextunknown == NUMUNKNOWN) {
nextunknown = 0; nextunknown = 0;
} }
diedunknown[nextunknown++] = pid; diedunknown[nextunknown] = pid;
statunknown[nextunknown++] = WEXITSTATUS(status);
Debug1("saving pid in diedunknown"F_Zu, Debug1("saving pid in diedunknown"F_Zu,
nextunknown/*sic, for compatibility*/); nextunknown/*sic, for compatibility*/);
} }