From: kn <kn@openbsd.org>
Date: Mon, 10 Oct 2022 14:57:48 +0000 (+0000)
Subject: Trigger ERR trap on permanent I/O redirection failure
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=f8c806b8e6f01d12a654a7bf412d3ed5ef82af70;p=openbsd

Trigger ERR trap on permanent I/O redirection failure

The following three cases behave identical in bash(1), but our ksh
(ksh93 also) fails to run the trap in the last case:

(non-zero exit code is trigger, no redirection)
	$ ksh -c 'trap "echo ERR" ERR ; false'
	ERR

(failed redirection is trigger, 'echo' was not executed)
	$ ksh -c 'trap "echo ERR" ERR ; echo >/'
	ksh: cannot create /: Is a directory
	ERR

(failed redirection, no execution, trap was NOT triggered)
	$ ksh -c 'trap "echo ERR" ERR ; exec >/'
	ksh: cannot create /: Is a directory


bash(1) prints "ERR" in all three cases, as expected.
ksh93 behaves like our ksh(1).

In ksh `exec' is a builtin (CSHELL), but also special (SPEC_BI):
	$ type alias
	alias is a shell builtin
	$ type exec
	exec is a special shell builtin

Without command and redirection alone, `exec' permanently redirects I/O for
the shell itself, not executing anything;  it is the only (special) builtin
with such a special use-case, implemented as c_sh.c:c_exec().

This corner-case is overlooked in exec.c:execute() which handles iosetup()
failure for all commands, incl. builtins.

Exclude c_exec() from the rest of special builtins to ensure it runs the
ERR trap as expected:

	$ ./obj/ksh -c 'trap "echo ERR" ERR ; exec >/'
	ksh: cannot create /: Is a directory
	ERR

Also add three new regress cases covering this;  rest keep passing.

OK millert
---

diff --git a/bin/ksh/exec.c b/bin/ksh/exec.c
index d2c27e9b2f4..887f6f95a9b 100644
--- a/bin/ksh/exec.c
+++ b/bin/ksh/exec.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: exec.c,v 1.75 2021/10/24 21:24:21 deraadt Exp $	*/
+/*	$OpenBSD: exec.c,v 1.76 2022/10/10 14:57:48 kn Exp $	*/
 
 /*
  * execute command tree
@@ -114,10 +114,12 @@ execute(struct op *volatile t,
 		for (iowp = t->ioact; *iowp != NULL; iowp++) {
 			if (iosetup(*iowp, tp) < 0) {
 				exstat = rv = 1;
-				/* Redirection failures for special commands
+				/* Except in the permanent case (exec 2>afile),
+				 * redirection failures for special commands
 				 * cause (non-interactive) shell to exit.
 				 */
-				if (tp && tp->type == CSHELL &&
+				if (tp && tp->val.f != c_exec &&
+				    tp->type == CSHELL &&
 				    (tp->flag & SPEC_BI))
 					errorf(NULL);
 				/* Deal with FERREXIT, quitenv(), etc. */
diff --git a/regress/bin/ksh/trap.t b/regress/bin/ksh/trap.t
new file mode 100644
index 00000000000..624f37af902
--- /dev/null
+++ b/regress/bin/ksh/trap.t
@@ -0,0 +1,49 @@
+#	$OpenBSD: trap.t,v 1.1 2022/10/10 14:57:48 kn Exp $
+
+#
+# Check that I/O redirection failure triggers the ERR trap.
+# stderr patterns are minimal to match all of bash, ksh and ksh93.
+# Try writing the root directory to guarantee EISDIR.
+#
+
+name: failed-redirect-triggers-ERR-restricted
+description:
+	Check that restricted mode prevents valid redirections that may write.
+arguments: !-r!
+stdin:
+	trap 'echo ERR' ERR
+	true >/dev/null
+expected-stdout:
+	ERR
+expected-stderr-pattern:
+	/restricted/
+expected-exit: e != 0
+---
+
+
+name: failed-redirect-triggers-ERR-command
+description:
+	Redirect standard output for a single command.
+stdin:
+	trap 'echo ERR' ERR
+	true >/
+expected-stdout:
+	ERR
+expected-stderr-pattern:
+	/Is a directory/
+expected-exit: e != 0
+---
+
+
+name: failed-redirect-triggers-ERR-permanent
+description:
+	Permanently redirect standard output of the shell without execution.
+stdin:
+	trap 'echo ERR' ERR
+	exec >/
+expected-stdout:
+	ERR
+expected-stderr-pattern:
+	/Is a directory/
+expected-exit: e != 0
+---