bin/51123: /bin/sh fails to close file (N>&-) in some cases (+FIX)

[kre%munnari.OZ.AU@localhost]

>Number:         51123
>Category:       bin
>Synopsis:       /bin/sh fails to close file (N>&-) in some cases (+FIX)
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May 08 04:15:00 +0000 2016
>Originator:     Robert Elz
>Release:        NetBSD 7.99.26   (all NetBSD 7 -> now versions)
>Organization:
>Environment:
System: NetBSD andromeda.noi.kre.to 7.99.26 NetBSD 7.99.26 (VBOX64-1.1-20160128) #43: Thu Jan 28 16:09:08 ICT 2016 kre%onyx.coe.psu.ac.th@localhost:/usr/obj/current/kernels/amd64/VBOX64 amd64
Architecture: x86_64
Machine: amd64
>Description:
	In some cases, when explicitly closing a file on the command
	line (using >&- or <&-) sh fails to close the file (or do
	anything at all to it.)

	This only happens when the redirect is applied in one of the
	following cases ...
		a function call
		a compound statement (not including (...) subshells)
		a builtin command (other than exec)
	That is, something executed in the current shell (other than exec).

	This is a side effect of a change made in June 2013, and so
	applies to NetBSD 7, and current (but not to NetBSd 6 or 6.1)

	Note: at first glance, it appears as if this may be the cause
	of the bug reported in PR bin/48875 - however, that was reported
	against NetBSD 6.1, which does not have this bug in it.

	It was while testing a fix for that PR that this problem was
	uncovered (the fix itself worked without fixing this bug, for
	other reasons, writing test cases to validate that the fix
	broke nothing exposed this problem...)

>How-To-Repeat:
	One easy way, if you are running NetBSD 7 or current is ...

		for f in 1 2 3
		do
		echo hello $f
		done >&-

	There should be no output, as stdout should be closed, but
	there won't be none...

	Running that test as
		 ( for f in 1 2 3
		 do
		 echo hello $f
		 done ) >&-
	does indeed produce no output (the sub-shell exemption above)

	An even simpler test is

		echo hello >&-

	(or printf if you prefer).   That's the builtin case.

>Fix:
	Apply the following patch (against current).   This will be
	committed once approved, and a pull-up for NetBSD 7 requested.

Index: redir.c
===================================================================
RCS file: /cvsroot/src/bin/sh/redir.c,v
retrieving revision 1.44
diff -u -u -r1.44 redir.c
--- redir.c	8 May 2016 03:51:15 -0000	1.44
+++ redir.c	8 May 2016 03:54:07 -0000
@@ -219,9 +219,15 @@
 				(void)fcntl(i, F_SETFD, FD_CLOEXEC);
 			fd_rename(sv, fd, i);
 			INTON;
-		} else {
-			close(fd);
 		}
+
+		/*
+		 * openredirect only closes the target fd if it successfuly
+		 * opens a new one, which it does not do for the >&- (<&-)
+		 * case .. so we need to do it here in all cases, not
+		 * only when we did not do the fd_rename() above.
+		 */
+		close(fd);
 		if (fd == 0)
 			fd0_redirected++;
 		openredirect(n, memory, flags);