NetBSD Problem Report #51119

From kre@munnari.OZ.AU  Fri May  6 21:43:04 2016
Return-Path: <kre@munnari.OZ.AU>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 305EE7A473
	for <gnats-bugs@www.NetBSD.org>; Fri,  6 May 2016 21:43:04 +0000 (UTC)
Message-Id: <201605062142.u46LgK60012385@andromeda.noi.kre.to>
Date: Sat, 7 May 2016 04:42:20 +0700 (ICT)
From: kre@munnari.OZ.AU
To: gnats-bugs@www.NetBSD.org
Subject: /bin/sh consumes fds when refusing to source an ELF binary (+FIX)
X-Send-Pr-Version: 3.95

>Number:         51119
>Category:       bin
>Synopsis:       /bin/sh consumes fds when refusing to source an ELF binary (+FIX)
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kre
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri May 06 21:45:00 +0000 2016
>Closed-Date:    Sat May 07 20:09:56 +0000 2016
>Last-Modified:  Sat May 07 20:10:01 +0000 2016
>Originator:     Robert Elz
>Release:        NetBSD 7.99.26 (all of NetBSD 5 6 7 & current to date)
>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:/usr/obj/current/kernels/amd64/VBOX64 amd64
Architecture: x86_64
Machine: amd64
>Description:
	Before reading a file, sh reads the first few bytes, to
	determine if it is a binary (ELF, not just a binary file)
	and if so, refuses to treat it as commands.

	That's fine, but when it happens, the fd used is never closed.
	Do it often enough, and your shell will have no fd's left.

	It has been like this since this check was added in Feb 2007
	(which means in all currently supported shells, plus NetBSD 5.)

>How-To-Repeat:
	andromeda$ /bin/sh	# this is a NetBSD 7.0_BETA /bin/sh
	$ . /bin/sh
	.: Cannot execute ELF binary /bin/sh
	$ fstat -p $$
	USER     CMD          PID   FD MOUNT       INUM MODE         SZ|DV R/W
	kre      sh         25400   wd /home    4192948 drwxrwxr-x    2560 r 
	kre      sh         25400    0 /dev/pts      31 crwx-w----  pts/14 rw
	kre      sh         25400    1 /dev/pts      31 crwx-w----  pts/14 rw
	kre      sh         25400    2 /dev/pts      31 crwx-w----  pts/14 rw
	kre      sh         25400    3 /          10338 -r-xr-xr-x  175372 r 
	kre      sh         25400 1023 /          10428 crw-rw-rw-     tty rw
	$ . /bin/sh
	.: Cannot execute ELF binary /bin/sh
	$ fstat -p $$
	USER     CMD          PID   FD MOUNT       INUM MODE         SZ|DV R/W
	kre      sh         25400   wd /home    4192948 drwxrwxr-x    2560 r 
	kre      sh         25400    0 /dev/pts      31 crwx-w----  pts/14 rw
	kre      sh         25400    1 /dev/pts      31 crwx-w----  pts/14 rw
	kre      sh         25400    2 /dev/pts      31 crwx-w----  pts/14 rw
	kre      sh         25400    3 /          10338 -r-xr-xr-x  175372 r 
	kre      sh         25400    4 /          10338 -r-xr-xr-x  175372 r 
	kre      sh         25400 1023 /          10428 crw-rw-rw-     tty rw
	$ . /bin/sh
	.: Cannot execute ELF binary /bin/sh
	$ fstat -p $$
	USER     CMD          PID   FD MOUNT       INUM MODE         SZ|DV R/W
	kre      sh         25400   wd /home    4192948 drwxrwxr-x    2560 r 
	kre      sh         25400    0 /dev/pts      31 crwx-w----  pts/14 rw
	kre      sh         25400    1 /dev/pts      31 crwx-w----  pts/14 rw
	kre      sh         25400    2 /dev/pts      31 crwx-w----  pts/14 rw
	kre      sh         25400    3 /          10338 -r-xr-xr-x  175372 r 
	kre      sh         25400    4 /          10338 -r-xr-xr-x  175372 r 
	kre      sh         25400    5 /          10338 -r-xr-xr-x  175372 r 
	kre      sh         25400 1023 /          10428 crw-rw-rw-     tty rw

	Notice fd's 3, then 4, then 5, just sitting there, all the same file:

	$ ls -li /bin/sh
	10338 -r-xr-xr-x  1 root  wheel  175372 Dec 11  2014 /bin/sh

	Note (from reading the source) that the same thing would happen
	if (for some reason) it proved impossible to rewind the file.
	(I do not currently know how to provoke that one though.)

>Fix:
	As a workaround, if this has happened, it is possible to just
	use fstat (as above) then
		exec 3>&-
		exec 4>&-
	(etc) for any fd's the shell should not have open. (Don't close
	the tty shown here on fd 1023 - especially in current sh's the
	fds used for the controlling tty, and for reading files are likely
	to be close together, and biggish numbers, so it would be easy to
	mistakenly close the tty - that would do bad things to job control.)

	Better, apply this patch, which will appear in current soon...
	(in this form, or something very similar).

Index: input.c
===================================================================
RCS file: /cvsroot/src/bin/sh/input.c,v
retrieving revision 1.49
diff -u -r1.49 input.c
--- input.c	2 May 2016 01:46:31 -0000	1.49
+++ input.c	6 May 2016 21:38:01 -0000
@@ -404,11 +404,15 @@
 	 */
 	if (lseek(fd, 0, SEEK_SET) == 0) {
 		if (read(fd, magic, 4) == 4) {
-			if (memcmp(magic, "\177ELF", 4) == 0)
+			if (memcmp(magic, "\177ELF", 4) == 0) {
+				(void)close(fd);
 				error("Cannot execute ELF binary %s", fname);
+			}
 		}
-		if (lseek(fd, 0, SEEK_SET) != 0)
+		if (lseek(fd, 0, SEEK_SET) != 0) {
+			(void)close(fd);
 			error("Cannot rewind the file %s", fname);
+		}
 	}

 	fd2 = to_upper_fd(fd);	/* closes fd, returns higher equiv */

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: bin-bug-people->kre
Responsible-Changed-By: kre@NetBSD.org
Responsible-Changed-When: Fri, 06 May 2016 22:01:05 +0000
Responsible-Changed-Why:
I am (for now) handling this PR


State-Changed-From-To: open->analyzed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Fri, 06 May 2016 22:01:05 +0000
State-Changed-Why:
Patch available, will be applied after verified correct and approved.


State-Changed-From-To: analyzed->closed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Sat, 07 May 2016 20:09:56 +0000
State-Changed-Why:
Problem fixed


From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/51119 CVS commit: src/bin/sh
Date: Sat, 7 May 2016 20:06:31 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Sat May  7 20:06:30 UTC 2016

 Modified Files:
 	src/bin/sh: input.c

 Log Message:
 PR bin/51119 - don't leak FDs in unusual error cases.   OK christos@


 To generate a diff of this commit:
 cvs rdiff -u -r1.49 -r1.50 src/bin/sh/input.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search