ksh segfault on job_chksave () after it receive a SIGCHLD (Signal 17)

I found a similar RH bugfix[1], not upstream[2].

Both backtrace from what I have in Ubuntu[3] and RH bug[4] correlate.

ksh could terminate unexpectedly with a segfault after it received the SIGCHLD signal.
See (sig=17) at frame 1 in job_reap.

[1] https://access.redhat.com/solutions/1253243

[2] - https://github.com/att/ast

[3] (gdb) bt
(gdb) bt
#0 job_chksave (pid=pid@entry=19003) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:1948
#1 0x00000000004282ab in job_reap (sig=17) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:428
#2 <signal handler called>

[4] - (gdb) bt
#0 job_chksave (pid=5066) at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/jobs.c:1949
#1 0x0000000000429240 in job_reap (sig=17) at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/jobs.c:428
#2 <signal handler called>

Red hat fix :

ksh.spec:
--
- job locking mechanism did not survive compiler optimization (#1123467)
--

ksh-20120801-locking.patch
--
diff -up ksh-20120801/src/cmd/ksh93/include/jobs.h.locking ksh-20120801/src/cmd/ksh93/include/jobs.h
--- ksh-20120801/src/cmd/ksh93/include/jobs.h.locking 2014-06-27 15:51:07.144923719 +0200
+++ ksh-20120801/src/cmd/ksh93/include/jobs.h 2014-06-27 15:52:56.463272276 +0200
@@ -149,15 +149,18 @@ extern struct jobs job;
 #define vmbusy() 0
 #endif

-#define job_lock() (job.in_critical++)
+#define asoincint(p) __sync_fetch_and_add(p,1)
+#define asodecint(p) __sync_fetch_and_sub(p,1)
+
+#define job_lock() asoincint(&job.in_critical)
 #define job_unlock() \
  do { \
   int sig; \
- if (!--job.in_critical && (sig = job.savesig)) \
+ if (asodecint(&job.in_critical)==1 && (sig = job.savesig)) \
   { \
- if (!job.in_critical++ && !vmbusy()) \
+ if (!asoincint(&job.in_critical) && !vmbusy()) \
     job_reap(sig); \
- job.in_critical--; \
+ asodecint(&job.in_critical); \
   } \
  } while(0)
--

A test package is now available for testing (pre-sru) to determine if this fixes the problem in Ubuntu as well.

# Instructions
sudo add-apt-repository ppa:slashd/sf143225ksh
sudo apt-get update
sudo apt-get install ksh

# Validation
dpkg -l | grep -i ksh should reveal version : ksh - 93u+20120801-1+

Please provide feedbacks.

- Eric

^^ should reveal version: ksh - 93u+20120801-1+sf143225b2

I installed the test package this morning, and we will try to reproduce for the coming week.

Thanks.
--
Walter

I have also reported a bug / submitted the patch to Debian upstream via :

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867181

- Eric

The suite of scripts that had the issue were run over 3000 times over the last week.

No error have been reported.

Looks good to me!

Thanks
--
Walter

Artful (Development Release) debdiff

For "Ubuntu Sponsors Team",

Could you please sponsor the Devel Release - Artful debdiff ?

I have the privileges to sponsor in Stable Release so once Artful is completed, I'll do the sponsor the SRU myself.

- Eric

Zesty debdiff

Yakkety debdiff

Xenial debdiff

Trusty debdiff

This bug was fixed in the package ksh - 93u+20120801-3.1ubuntu1

---------------
ksh (93u+20120801-3.1ubuntu1) artful; urgency=medium

  * d/p/locking.patch: Fix job locking mechanism to prevent
    ksh to segfaults in job_chksave after receiving SIGCHLD. (LP: #1697501)

 -- Eric Desrochers <email address hidden> Tue, 04 Jul 2017 12:42:47 -0400

Patch uploaded for the following releases : T/X/Y/Z.

Now waiting for the SRU verification team to approve the uploads for the new ksh packages to start building in $RELEASE-proposed.

- Eric

Hello Eric, or anyone else affected,

Accepted ksh into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ksh/93u+20120801-1ubuntu0.14.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-trusty to verification-done-trusty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-trusty. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Hello Eric, or anyone else affected,

Accepted ksh into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ksh/93u+20120801-2ubuntu0.16.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-yakkety to verification-done-yakkety. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-yakkety. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Hello Eric, or anyone else affected,

Accepted ksh into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ksh/93u+20120801-2ubuntu0.16.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Hello Eric, or anyone else affected,

Accepted ksh into zesty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ksh/93u+20120801-2ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-zesty to verification-done-zesty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-zesty. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Bonjour,

preliminary tests were started yesterday with trusty-proposed.

We will start running in loop over the weekend with the script that mostly had the issue.

On Monday morning, this will be installed on our dev cluster which usually run 18M ksh a day.

I will try to start a few tests on xenial too some time next week.

Thanks
--
Walter

Thanks Walter.

Looking forward to hearing back from you with the outcome of your test.

- Eric

Bonjour,

we have been running tests for over a week on trusty. No issue to report.

I did not manage to get much tests going on xenial though.
--
Walter

# VERIFICATION TRUSTY #

I've been in direct communication with Walter, the reporter of this bug.
Walter is intensively using ksh using Trusty lxc container.

Before the proposed package, when running their in-house ksh scripts, ksh could unexpectedly segfaulting, but since they are using this proposed package there is no new occurrence of segfaults for approximately 2 weeks of intensive testing.

We can then conclude the proposed package is ready to be release in -updates.

To see Walter's comment :
https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1697501/comments/22

- Eric

# VERIFICATION FOR XENIAL/YAKKETY/ZESTY #

For Xenial, Yakkety Zesty, there is no easy way for me to reproduce the situation.
Most of the troubleshooting has been done through coredump analysis and code inspection on my side.

It is clear for me that all supported stable releases (Xenial/Yakkety/Zesty) are affected by this bug at code inspection.

Walter, the reporter of this bug, has a in-house reproducer, but so far he only has tested on Trusty because it was his main interest.

Unless Walter is amenable to test on Xenial/Yakkety/Zesty as well, there is no clear reproducer as of today except his in-house ksh script, but again based on code inspection I'm confident that this need to be part of ksh package for Xenial/Yakkety/Zesty.

Note that there isn't much change between ksh on Trusty and Artful, meaning the package is pretty much the same for every stable releases. ksh is a unmaintained project with no new development nowadays.

- Eric

I'll release the trusty part right now (since the target-of-interest for the issue is trusty). For the rest, as we discussed privately: please at least give a basic sanity-spin of the new ksh for each of the other stable series, at least to make sure those are not completely broken.

This bug was fixed in the package ksh - 93u+20120801-1ubuntu0.14.04.1

---------------
ksh (93u+20120801-1ubuntu0.14.04.1) trusty; urgency=medium

  * d/p/locking.patch: Fix job locking mechanism to prevent
    ksh to segfaults in job_chksave after receiving SIGCHLD. (LP: #1697501)

 -- Eric Desrochers <email address hidden> Fri, 23 Jun 2017 16:01:04 -0400

The verification of the Stable Release Update for ksh has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Hi Łukasz Zemczak (sil2100),

With regard to the basic sanity-spin of the new ksh for each of the other stable series to make sure those are not completely broken.

I have tested ksh against a number of regression tests offered in the ksh source package itself.

--
To run all these tests with the shell you just built, go to the tests directory and run the command:
SHELL=$dir/ksh $dir/ksh shtests
where dir is the directory of the ksh you want to test.
--

The output the regression tests are generating is very long, but to summarize it test the binary in depth. Testing alias, append, arrays, attributes, bracket, enum, exit, expand, functions, grep, io, leaks, locale, math, namespace, path, options, quoting sigchld, signal, variables, tilde, and many more.

Output example :

test sigchld passed at 2017-07-18+15:31:19 [ 14 tests 0 errors ]
test sigchld(C.UTF-8) begins at 2017-07-18+15:31:19
test sigchld(C.UTF-8) passed at 2017-07-18+15:31:35 [ 14 tests 0 errors ]
test sigchld(shcomp) begins at 2017-07-18+15:31:35
test sigchld(shcomp) passed at 2017-07-18+15:31:50 [ 14 tests 0 errors ]

I'm showing this above regression test output as an example and because the bug was about a segfault after receiving a sigchld.

I won't hide that the test is sometime generating errors here and there, even if the huge majority of the regression tests passed without errors. Amongst the reported errors I can't find anything obvious that could be related to the patch I have submitted.

I have strong believe these errors existed pre-SRU, and are probably there for a while, since there is no new development nowadays for ksh project.

Example of errors :
test bracket(C.UTF-8) begins at 2017-07-18+15:27:58
 bracket.sh[86]: -r: /tmp/tmp.HapRrWwjIk/original should not be readable
 bracket.sh[92]: -w: /tmp/tmp.HapRrWwjIk/original should not be writable
 bracket.sh[95]: -x: /tmp/tmp.HapRrWwjIk/original should not be executable
 bracket.sh[98]: -rw: /tmp/tmp.HapRrWwjIk/original should not be readable/writable
test bracket(C.UTF-8) failed at 2017-07-18+15:28:03 with exit code 1 [ 118 tests 1 error ]

Hope the above verification is enough for you to have confidence in the -proposed package for X/Y/Z.

- Eric

@Eric

That sounds like that exceeds what sil2000 requested to me. But please can you state how are you sure these tests ran against the ksh binary in proposed, rather than from a build in a source tree, for example? Also please confirm the package version supplying the ksh binary that you tested.

@Robie,

>How are you sure these tests ran against the ksh binary in proposed, rather than from a build in a source tree, for example?

Yes, the test was ran against the ksh binaries in proposed.

I have test both binaries with the following :

#SHELL=/bin/ksh93 /bin/ksh93 shtests
#SHELL=/bin/ksh /bin/ksh shtests

>Also please confirm the package version supplying the ksh binary that you tested.

xenial | ksh | 93u+20120801-2ubuntu0.16.04.1
yakkety | ksh | 93u+20120801-2ubuntu0.16.10.1
zesty | ksh | 93u+20120801-2ubuntu1

- Eric

This bug was fixed in the package ksh - 93u+20120801-2ubuntu1

---------------
ksh (93u+20120801-2ubuntu1) zesty; urgency=medium

  * d/p/locking.patch: Fix job locking mechanism to prevent
    ksh to segfaults in job_chksave after receiving SIGCHLD. (LP: #1697501)

 -- Eric Desrochers <email address hidden> Tue, 04 Jul 2017 15:08:52 -0400

This bug was fixed in the package ksh - 93u+20120801-2ubuntu0.16.10.1

---------------
ksh (93u+20120801-2ubuntu0.16.10.1) yakkety; urgency=medium

  * d/p/locking.patch: Fix job locking mechanism to prevent
    ksh to segfaults in job_chksave after receiving SIGCHLD. (LP: #1697501)

 -- Eric Desrochers <email address hidden> Tue, 04 Jul 2017 15:08:52 -0400

This bug was fixed in the package ksh - 93u+20120801-2ubuntu0.16.04.1

---------------
ksh (93u+20120801-2ubuntu0.16.04.1) xenial; urgency=medium

  * d/p/locking.patch: Fix job locking mechanism to prevent
    ksh to segfaults in job_chksave after receiving SIGCHLD. (LP: #1697501)

 -- Eric Desrochers <email address hidden> Tue, 04 Jul 2017 15:08:52 -0400