freebsd stable - May 2005 - libc_r kqueue fd leak

We discovered a kqueue leak when running one of our 4.x applications on
FreeBSD 5.3 using the compat libc_r.  It turns out it's caused by
libc_r's
close() failing.

The libc_r close (in uthread_close.c) calls fstat() on the file
descriptor.  On 4.x this succeeds, while on 5.x the fstat() on the kqueue()
fd returns -1 with errno=0.  The close() in libc_r then returns this error
without doing the actual close syscall.

I built the test application shown below on a 4.7 and 5.3 machine
and fstat returns 0 on 4.7, -1 on 5.3.  If the test app is linked against
libc_r then the close() fails too.

fstat(2) indicates that fstat() returns a mostly-zeroed buffer for a socket
fd, but gives no indication of what should happen for a kqueue fd.  What is
the expected behaviour here?  The issue could be fixed by either having the
kernel not fail the fstat, or making libc_r ignore the failure and continue
on with the close.

== kqueue.c =
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/event.h>
#include <errno.h>

int main()
{
    struct stat sb;
    int kq=kqueue();
    printf("fstat returns %d (%d)\n", fstat(kq, &sb), errno);
    printf("close returns %d (%d)\n", close(kq), errno);
}

--
Ed Maste, Sandvine Incorporated

On Tue, May 24, 2005 at 12:59:07PM -0400, Ed Maste wrote:
> We discovered a kqueue leak when running one of our 4.x applications on
> FreeBSD 5.3 using the compat libc_r.  It turns out it's caused by
libc_r's
> close() failing.
I've attached a patch which stops libc_r close() from bailing if fstat()
returns an error.  This fixes the kqueue leak.  This logic would also have
to make its way into the compat library via 4.x to fully resolve the issue.

--
Ed Maste, Sandvine Incorporated
-------------- next part --------------
--- uthread_close.c.orig	2005-05-24 13:22:14.000000000 -0400
+++ uthread_close.c	2005-05-24 13:21:05.000000000 -0400
@@ -63,8 +63,7 @@
 	 * Lock the file descriptor while the file is closed and get
 	 * the file descriptor status:
 	 */
-	else if (((ret = _FD_LOCK(fd, FD_RDWR, NULL)) == 0) &&
-	    ((ret = __sys_fstat(fd, &sb)) == 0)) {
+	else if ((ret = _FD_LOCK(fd, FD_RDWR, NULL)) == 0) {
 		/*
 		 * Check if the file should be left as blocking.
 		 *
@@ -85,7 +84,8 @@
 		 * using, which would then cause any reads to block
 		 * indefinitely.
 		 */
-		if ((S_ISREG(sb.st_mode) || S_ISCHR(sb.st_mode))
+		if (__sys_fstat(fd, &sb) == 0 &&
+		    (S_ISREG(sb.st_mode) || S_ISCHR(sb.st_mode))
 		    && (_thread_fd_getflags(fd) & O_NONBLOCK) == 0) {
 			/* Get the current flags: */
 			flags = __sys_fcntl(fd, F_GETFL, NULL);