Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ksh93 echoing wrong output due to missing EIO handling during logout #30

Closed
lijog opened this issue Feb 17, 2017 · 1 comment
Closed

Comments

@lijog
Copy link

lijog commented Feb 17, 2017

Here's a reproducible testcase on a Solaris11 host running ksh93u+(2012-08-01).
$ cat a.sh
#!/bin/sh

AAA="aaa"
echo 'insert character'
BBB=echo ${AAA} | sed "s/aaa/bbb/g"
logger "variable BBB = ${BBB}"

$ cat t.sh
#!/bin/ksh

sleep 10
/bin/ksh ./a.sh
exit 0

$

$ ./t.sh

The expected result is:

Apr 9 12:43:34 lab user: [ID 702911 user.notice] variable BBB = bbb

because variable "BBB" is supposed to be set to 'bbb' in a.sh.

But if the parent shell is terminated, the variable is wrongly set.

user@xxxxx$ telnet lab
...
$ ./t.sh & <--- Run t.sh in background.
[1] 2067
$ logout <--- CTRL + D to exit while t.sh is running.
Connection to lab closed by foreign host.

Again, access the system and check the output:

user@xxxxx$ telnet lab
...
$ tail -f /var/adm/messages
:
Apr 9 12:47:47 lab user: [ID 702911 user.notice] variable BBB = insert
character <--- !!!
Apr 9 12:47:47 lab bbb
<--- !!!

Thus the variable is wrongly set. (The previous echo string was not cleared.)

The issue happens because the EIO error during the logout is not handled properly.
The following patch fixes the issue

--- INIT.2012-08-01.old/src/cmd/ksh93/sh/io.c 2017-01-04 14:41:25.199402375 +0000
+++ INIT.2012-08-01/src/cmd/ksh93/sh/io.c 2017-01-04 14:32:20.279449987 +0000
@@ -64,9 +64,9 @@

#ifndef ERROR_PIPE
#ifdef ECONNRESET
-#define ERROR_PIPE(e) ((e)==EPIPE||(e)==ECONNRESET)
+#define ERROR_PIPE(e) ((e)==EPIPE||(e)==ECONNRESET||(e)==EIO)
#else
-#define ERROR_PIPE(e) ((e)==EPIPE)
+#define ERROR_PIPE(e) ((e)==EPIPE||(e)==EIO)
#endif
#endif

@lijog lijog changed the title ksh93 echo outputs wrong variable value if the shell process was running in background and the parent exited. ksh93 wrong variable value printed if the shell process was running in background and the parent exited. Feb 17, 2017
@lijog lijog changed the title ksh93 wrong variable value printed if the shell process was running in background and the parent exited. ksh93 echoing wrong output due to missing EIO handling during logout Feb 22, 2017
@lijog
Copy link
Author

lijog commented Feb 22, 2017

I missed out the changes in the error.h include file earlier.
Here's the complete patch.

--- INIT.2012-08-01.old/src/cmd/ksh93/sh/io.c 2017-01-04 14:41:25.199402375 +0000
+++ INIT.2012-08-01/src/cmd/ksh93/sh/io.c 2017-01-04 14:32:20.279449987 +0000
@@ -64,9 +64,9 @@

#ifndef ERROR_PIPE
#ifdef ECONNRESET
-#define ERROR_PIPE(e) ((e)==EPIPE||(e)==ECONNRESET)
+#define ERROR_PIPE(e) ((e)==EPIPE||(e)==ECONNRESET||(e)==EIO)
#else
-#define ERROR_PIPE(e) ((e)==EPIPE)
+#define ERROR_PIPE(e) ((e)==EPIPE||(e)==EIO)
#endif
#endif

--- INIT.2012-08-01.old/src/lib/libast/include/error.h 2017-02-17 02:39:37.507460057 +0000
+++ INIT.2012-08-01/src/lib/libast/include/error.h 2017-02-17 04:42:38.872435651 +0000
@@ -85,9 +85,9 @@
#define ERROR_SET 0x0080 /* set context */

#ifdef ECONNRESET
-#define ERROR_PIPE(e) ((e)==EPIPE||(e)==ECONNRESET)
+#define ERROR_PIPE(e) ((e)==EPIPE||(e)==ECONNRESET||(e)==EIO)
#else
-#define ERROR_PIPE(e) ((e)==EPIPE)
+#define ERROR_PIPE(e) ((e)==EPIPE||(e)==EIO)
#endif

/*

citrus-it added a commit to citrus-it/ast that referenced this issue Dec 30, 2020
citrus-it added a commit to citrus-it/ast that referenced this issue Dec 30, 2020
citrus-it added a commit to citrus-it/ast that referenced this issue Jan 4, 2021
citrus-it added a commit to citrus-it/ast that referenced this issue Jan 4, 2021
citrus-it added a commit to citrus-it/ast that referenced this issue Jan 4, 2021
citrus-it added a commit to citrus-it/ast that referenced this issue Jan 5, 2021
citrus-it added a commit to citrus-it/ast that referenced this issue Jan 8, 2021
citrus-it added a commit to citrus-it/ast that referenced this issue Jan 8, 2021
McDutchie added a commit to ksh93/ksh that referenced this issue Jan 8, 2021
This change is pulled from here:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/275-20855453.patch
att#30

George Lijo wrote on 17 Feb 2017:
> Here's a reproducible testcase on a Solaris11 host running
> ksh93u+(2012-08-01).
> $ cat a.sh
> #!/bin/sh
>
> AAA="aaa"
> echo 'insert character'
> BBB=`echo ${AAA} | sed "s/aaa/bbb/g"`
> logger "variable BBB = ${BBB}"
>
> $ cat t.sh
> #!/bin/ksh
>
> sleep 10
> /bin/ksh ./a.sh
> exit 0
>
> $
>
> $ ./t.sh
>
> The expected result is:
>
> Apr 9 12:43:34 lab user: [ID 702911 user.notice] variable BBB = bbb
>
> because variable "BBB" is supposed to be set to 'bbb' in a.sh.
>
> But if the parent shell is terminated, the variable is wrongly set.
>
> user@xxxxx$ telnet lab
> ...
> $ ./t.sh & <--- Run t.sh in background.
> [1] 2067
> $ logout <--- CTRL + D to exit while t.sh is running.
> Connection to lab closed by foreign host.
>
> Again, access the system and check the output:
>
> user@xxxxx$ telnet lab
> ...
> $ tail -f /var/adm/messages
> :
> Apr 9 12:47:47 lab user: [ID 702911 user.notice] variable BBB =
> insert character <--- !!!
> Apr 9 12:47:47 lab bbb
> <--- !!!
>
> Thus the variable is wrongly set. (The previous echo string was
> not cleared.)
>
> The issue happens because the EIO error during the logout is not
> handled properly.

src/cmd/ksh93/sh/io.c,
src/lib/libast/include/error.h:
- Amend the ERROR_PIPE() macro to check for EIO as well as EPIPE
  and ECONNRESET.
citrus-it added a commit to citrus-it/ast that referenced this issue Jan 16, 2021
citrus-it pushed a commit to citrus-it/ast that referenced this issue Apr 15, 2021
After making PATH readonly in a virtual subshell (without otherwise
changing it, so the subshell is never forked), then the main shell
would erroneously fork into a background process immediately after
leaving the virtual subshell. This was caused by a bug in the
forking workaround that prevents changes in PATH in a virtual
subshell from clearing the parent shell's hash table.

src/cmd/ksh93/sh/name.c: nv_putval():
- If we're either setting or restoring PATH, do an additional check
  for the NV_RDONLY flag, which means the function was told to
  ignore the variable's readonly state. It is told to ignore that
  when restoring the parent shell state after exiting a virtual
  subshell. If we don't fork then, we don't fork the parent shell.

src/cmd/ksh93/tests/subshell.sh:
- Add regression test verifying that no forking happens when making
  PATH readonly in a subshell.

Fixes att#30.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant