From hsu@FreeBSD.org Fri Nov 29 15:37:16 2002
Date: Wed, 27 Nov 2002 21:22:44 -0800
From: Jeffrey Hsu <hsu@FreeBSD.org>
To: rwatson@freebsd.org
Subject: file descriptor locking problem #1 (fwd)


------- Forwarded Message

X-Mailer: exmh version 2.5 07/13/2001 with nmh-1.0.4
To: jhb@freebsd.org
Subject: file descriptor locking problem #1
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Date: Wed, 27 Nov 2002 12:40:07 -0800
From: Jeffrey Hsu <hsu@FreeBSD.org>

In kern_descrip.c, the kern_fcntl() routine, two processes could
do a fcntl(F_SETFL) and the resulting race condition could leave
f_flags in an indeterminate state due to the following read-modify-write
code not being properly locked:

        case F_SETFL:
                fhold(fp);
                FILEDESC_UNLOCK(fdp);
                fp->f_flag &= ~FCNTLFLAGS;
                fp->f_flag |= FFLAGS(arg & ~O_ACCMODE) & FCNTLFLAGS;

The fhold() only increments the f_count field and does not lock the struct 
file:

#define fhold(fp)                                                       \
        do {                                                            \
                FILE_LOCK(fp);                                          \
                fhold_locked(fp);                                       \
                FILE_UNLOCK(fp);                                        \
        } while (0)

#define fhold_locked(fp)                                                \
        do {                                                            \
                FILE_LOCK_ASSERT(fp, MA_OWNED);                         \
                (fp)->f_count++;                                        \
        } while (0)


------- End of Forwarded Message



From hsu@FreeBSD.org Fri Nov 29 15:37:23 2002
Date: Wed, 27 Nov 2002 21:22:56 -0800
From: Jeffrey Hsu <hsu@FreeBSD.org>
To: jhb@freebsd.org
Cc: rwatson@freebsd.org
Subject: Re: file descriptor locking problem #1

Code speaks louder than words, so here's a patch for file descriptor
locking problem #1.  I've already identified 2 other problems with
file descriptor locking and will mail you patches for them too.
I plan to gather them all and then commit them (in separate commits
for each race condition fixed).

							Jeffrey

Index: kern_descrip.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.168
diff -u -r1.168 kern_descrip.c
--- kern_descrip.c	27 Oct 2002 18:07:41 -0000	1.168
+++ kern_descrip.c	27 Nov 2002 23:07:16 -0000
@@ -298,10 +298,12 @@
 		break;
 
 	case F_SETFL:
-		fhold(fp);
+		FILE_LOCK(fp);
 		FILEDESC_UNLOCK(fdp);
+		fhold_locked(fp);
 		fp->f_flag &= ~FCNTLFLAGS;
 		fp->f_flag |= FFLAGS(arg & ~O_ACCMODE) & FCNTLFLAGS;
+		FILE_UNLOCK(fp);
 		tmp = fp->f_flag & FNONBLOCK;
 		error = fo_ioctl(fp, FIONBIO, &tmp, td->td_ucred, td);
 		if (error) {
@@ -314,7 +316,9 @@
 			fdrop(fp, td);
 			break;
 		}
+		FILE_LOCK(fp);
 		fp->f_flag &= ~FNONBLOCK;
+		FILE_UNLOCK(fp);
 		tmp = 0;
 		(void)fo_ioctl(fp, FIONBIO, &tmp, td->td_ucred, td);
 		fdrop(fp, td);