Messages in this thread Patch in this message | | | Date | Fri, 1 Oct 2004 17:52:30 -0700 | From | Chris Wright <> | Subject | [TEST PATCH] Re: kernel 2.6.8 bug in fs/locks.c |
| |
* 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; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |