Bug 1554364

Summary: /etc/init.d/functions uses non-posix syntax - in recent update
Product: Red Hat Enterprise Linux 7 Reporter: David Kaspar // Dee'Kej <deekej>
Component: initscriptsAssignee: David Kaspar // Dee'Kej <deekej>
Status: CLOSED ERRATA QA Contact: Daniel Rusek <drusek>
Severity: urgent Docs Contact: Lenka Špačková <lkuprova>
Priority: urgent    
Version: 7.6CC: bbreard, ccheney, deekej, drusek, fkrska, fsumsal, initscripts-maint-list, kdudka, mkolaja, msekleta, qe-baseos-daemons, svashisht, thozza
Target Milestone: rcKeywords: EasyFix, Patch, Reproducer
Target Release: ---   
Hardware: All   
OS: All   
Whiteboard:
Fixed In Version: initscripts-9.49.42-1.el7 Doc Type: Release Note
Doc Text:
*KSH* no longer fails to process `/etc/init.d/functions` The Korn Shell (KSH) is unable to process code where the word `local` appears on the same line as an array definition. This previously caused *KSH* to fail to source the `/etc/init.d/functions` file. This update provides a workaround to the *KSH* limitation, and the function file is now being sourced as expected.​ Note that *KSH* may still be unable use some of the functions in `/etc/init.d/functions` file. This update only allows KSH to not fail during the sourcing of `/etc/init.d/functions`.
Story Points: ---
Clone Of: 1518429 Environment:
Last Closed: 2018-10-30 10:15:57 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1518429    
Bug Blocks: 1549617, 1549689, 1551061    

Description David Kaspar // Dee'Kej 2018-03-12 14:07:17 UTC
+++ This bug was initially created as a clone of Bug #1518429 +++

Description of problem:

We have a customer report that a recent update to initscripts /etc/init.d/functions breaks init scripts written in ksh. Looking at the code it appears to be the following line:

local stat=($(< /proc/self/stat))

This should be changed to be posix compliant so that ksh init scripts can continue to work.

Version-Release number of selected component (if applicable):

I believe the bug was added in 9.03.50-1

It wasn't there in 9.03.49-1 (RHEL 6.7) but was in 9.03.53-1 (RHEL 6.8) and newer.

How reproducible:

Always

Steps to Reproduce:

#!/bin/ksh
. /etc/init.d/functions


Actual results:

./test[4]: .: syntax error at line 114: '(' unexpected

Expected results:

(no syntax error)

--- Additional comment from David Kaspar [Dee'Kej] on 2017-11-29 12:00:23 CET ---

Hello Chris,

looking at this piece of code I can tell you this:

* the syntax used there was chosen to obtain the stat value directly from the shell script (i.e. to not use any subshells/subprocesses, which would have different stat value)

* because of this limitation some utilities (like 'cat' for example) can't be used to solve this in other way

However, based on my testing I have found out this:

* if you use 'local stat=', you will get the syntax error
* if you use just 'stat=', the ksh will parse it successfully

This looks like a bug in ksh, and I will discuss with ksh maintainer what to do about it.

For now, the workaround for this issue could be this:
---------------------------------
diff --git a/rc.d/init.d/functions b/rc.d/init.d/functions
index e1d6ecd..e8e3bfd 100644
--- a/rc.d/init.d/functions
+++ b/rc.d/init.d/functions
@@ -111,8 +111,12 @@ __kill_pids_term_kill() {
     local try=0
     local delay=3;
     local pid=
-    local stat=($(< /proc/self/stat))
+
+    # 'stat' variable can't be a local variable, because it leads to syntax
+    # error when ksh is used. Therefore we use global variable & unset it later.
+    stat=($(< /proc/self/stat))
     local base_stime=${stat[21]}
+    unset stat
 
     if [ "$1" = "-d" ]; then
         delay=$2
---------------------------------

> THIS IS FOR TESTING PURPOSES ONLY! DO NOT USE IT ON PRODUCTION MACHINES!

--- Additional comment from Chris Cheney on 2017-12-08 21:28:49 CET ---

Thanks for the workaround, I let the customer know.

--- Additional comment from Siteshwar Vashisht on 2017-12-12 03:06:39 CET ---

With the latest upstream I am getting this error under ksh :

local: local can only be used in a function

So it looks like it is parsed incorrectly. I have opened an upstream issue for it https://github.com/att/ast/issues/220 to follow up.

--- Additional comment from Tomáš Hozza on 2018-01-18 16:11:26 CET ---

David, this bug is on 6.10 RPL and does not have devel_ack+ nor Devel Conditional NAK set. What is the current plan with this bug with regard to the Devel Freeze on Feb-27?

--- Additional comment from David Kaspar [Dee'Kej] on 2018-01-18 16:39:51 CET ---

Tomáš - we expect to fix this with the proposed workaround above.

--- Additional comment from Marcel Kolaja on 2018-01-30 13:07:13 CET ---

Ben, could you please review this bug for inclusion in RHEL 6.10? Thanks!

--- Additional comment from Michal Sekletar on 2018-02-12 17:34:30 CET ---

Problem while sourcing the functions in ksh isn't the local keyword (even though it will blow up if you try to call the function using it), but the combination of local keyword and arrays on the same line.

This is the original piece of code that causes source in ksh to fail,
local stat=($(/proc/self/stat))

However, if you instead rewrite it as follows, then sourcing in ksh will work.
local stat=
stat=($(/proc/self/stat))

I propose to use this approach instead of a workaround suggested in comment #4, because this approach has the benefit that it doesn't change the semantics for users running bash, i.e. it is safer.

--- Additional comment from Kamil Dudka on 2018-02-12 17:59:33 CET ---

(In reply to Michal Sekletar from comment #10)
> However, if you instead rewrite it as follows, then sourcing in ksh will
> work.
> local stat=
> stat=($(/proc/self/stat))

It should be $(<...) here ^^^.  Otherwise I totally agree with that.  We ended up with the same conclusion while we were discussing it with Siteshwar a month ago:

http://pastebin.test.redhat.com/548171 ( http://pastebin.test.redhat.com/555010 )

Unfortunately, nobody cared to summarize it here to avoid the duplicated work.  Sorry about that.

--- Additional comment from Michal Sekletar on 2018-02-12 18:16:09 CET ---

(In reply to Michal Sekletar from comment #10)

> This is the original piece of code that causes source in ksh to fail,
> local stat=($(/proc/self/stat))

Minor correction. Original code reads,

local stat=($(< /proc/self/stat))

Hence, fix should look as follows,

local stat
stat=($(< /proc/self/stat))

Comment 2 David Kaspar // Dee'Kej 2018-04-26 15:13:40 UTC
Pull-request has been accepted:
https://github.com/fedora-sysv/initscripts/pull/164

Comment 10 errata-xmlrpc 2018-10-30 10:15:57 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:3131