Subject: lib/5170: getcap.c uses closed FILE *
To: None <gnats-bugs@gnats.netbsd.org>
From: David Holland <dholland@eecs.harvard.edu>
List: netbsd-bugs
Date: 03/17/1998 17:19:01
>Number:         5170
>Category:       lib
>Synopsis:       getcap.c in libc uses ferror() after fclose()
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people (Library Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 17 14:50:01 1998
>Last-Modified:
>Originator:     David A. Holland <dholland@eecs.harvard.edu>
>Organization:
   - David A. Holland             |    VINO project home page:
     dholland@eecs.harvard.edu    | http://www.eecs.harvard.edu/vino
>Release:        -current of 19980317
>Environment:
	
System: NetBSD chianti.eecs.harvard.edu 1.2.1 NetBSD 1.2.1 (CHIANTI) #1: Tue Sep 9 16:52:39 EDT 1997 root@chianti.eecs.harvard.edu:/usr/src/sys/arch/i386/compile/CHIANTI i386

Note: the problem still exists in -current as of this afternoon.

>Description:
	In getcap.c, in libc/gen, when fgetln returns NULL to say it's
	done, the code checks with ferror() to see if it was EOF or
	error. This is fine, but it does it after calling fclose() on
	the file, so the results are undefined. Specifically, it fails
	on some platforms other than NetBSD, and it might start failing
	on NetBSD too if someone modified stdio.

>How-To-Repeat:
	I discovered this trying to compile cap_mkdb(1) on Linux to
	build the vgrind database while cross-compiling VINO. You
	probably don't want to try to duplicate this setup. 

	Building getcap.c and cap_mkdb on assorted platforms should
	yield at least one where it doesn't work due to this problem.
	Fortunately, there's no need to do this.


>Fix:
	Apply the following patch, or equivalent:


Index: getcap.c
===================================================================
RCS file: /home/vino/repo/src/libc/libc/gen/getcap.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 getcap.c
--- getcap.c	1996/12/20 17:16:23	1.1.1.1
+++ getcap.c	1998/03/17 22:21:08
@@ -672,11 +672,12 @@
 		} else {
 			line = fgetln(pfp, &len);
 			if (line == NULL && pfp) {
-				(void)fclose(pfp);
 				if (ferror(pfp)) {
 					(void)cgetclose();
+					(void)fclose(pfp);
 					return (-1);
 				} else {
+					(void)fclose(pfp);
 					if (*++dbp == NULL) {
 						(void)cgetclose();
 						return (0);
@@ -731,11 +732,12 @@
 			} else { /* name field extends beyond the line */
 				line = fgetln(pfp, &len);
 				if (line == NULL && pfp) {
-					(void)fclose(pfp);
 					if (ferror(pfp)) {
+						(void)fclose(pfp);
 						(void)cgetclose();
 						return (-1);
 					}
+					(void)fclose(pfp);
 				} else
 					line[len - 1] = '\0';
 			}

>Audit-Trail:
>Unformatted: