[TEST PATCH] Re: kernel 2.6.8 bug in fs/locks.c

[Chris Wright <chrisw@osdl.org>]

* Chris Wright (chrisw@osdl.org) wrote:
> * Ivan Kalatchev (ivan.kalatchev@esg.ca) wrote:
> > I'm using pthreads for each user-application connections. To protect
> > configuration file from corruption I used file locking mechanism - fcntl
> > with F_WRLCK/F_RDLCK.
> 
> I must be confused.  pthreads and fcntl locking...that does't give
> proper exclusion?  The BUG, however, is no good.  Despite the fact
> that it appears to come at the result of an application bug, we should
> be able to handle this w/out a BUG.  AFAICT, one thread has closed the
> file descriptor, whilst another is mucking with the locks.  So the locker
> winds up holding the last ref to the filp.  This blows the logic of when
> locks_remove_posix gets called.  Thanks for the bug report.

This is a rather ugly fix, but it demonstrates the hole.  It's possible
for a posix lock to get applied after the final close of the file.  This
means locks_remove_posix misses the yet to be created posix lock, and
on final fput when leaving fcntl, locks_remove_flock will BUG().
I was able to reproduce Ivan's bugreport.  I doubt it's the right/best
fix, but it's functional (no more BUG()).  Against -bk-curr.

Signed-off-by: Chris Wright <chrisw@osdl.org>

===== fs/fcntl.c 1.40 vs edited =====
--- 1.40/fs/fcntl.c	2004-09-02 02:48:03 -07:00
+++ edited/fs/fcntl.c	2004-10-01 17:34:15 -07:00
@@ -364,7 +364,7 @@
 
 asmlinkage long sys_fcntl(int fd, unsigned int cmd, unsigned long arg)
 {	
-	struct file *filp;
+	struct file *filp, *filp2;
 	long err = -EBADF;
 
 	filp = fget(fd);
@@ -379,6 +379,11 @@
 
 	err = do_fcntl(fd, cmd, arg, filp);
 
+	filp2 = fget(fd);
+	if (filp2 != filp)
+		locks_remove_posix(filp, current->files);
+	if (filp2)
+		fput(filp2);
  	fput(filp);
 out:
 	return err;
@@ -387,7 +392,7 @@
 #if BITS_PER_LONG == 32
 asmlinkage long sys_fcntl64(unsigned int fd, unsigned int cmd, unsigned long arg)
 {	
-	struct file * filp;
+	struct file *filp, *filp2;
 	long err;
 
 	err = -EBADF;
@@ -414,6 +419,12 @@
 			err = do_fcntl(fd, cmd, arg, filp);
 			break;
 	}
+
+	filp2 = fget(fd);
+	if (filp2 != filp)
+		locks_remove_posix(filp, current->files);
+	if (filp2)
+		fput(filp2);
 	fput(filp);
 out:
 	return err;