Patch from Matthew Wilcox <willy@debian.org>


 - Rejig locks_delete_block() so it takes the BKL and add
   __locks_delete_block() which is lockless.  Factors out some common
   code.
 - Change locks_remove_posix() to walk the lock list ourselves rather than
   calling posix_lock_file().  Fixes dozens of reported file locking bugs.
 - call MAJOR()/MINOR() ourselves rather than using kdevname(), which is racy.




 locks.c |   61 ++++++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 38 insertions(+), 23 deletions(-)

diff -puN fs/locks.c~flock-fix fs/locks.c
--- 25/fs/locks.c~flock-fix	2003-02-24 12:03:48.000000000 -0800
+++ 25-akpm/fs/locks.c	2003-02-24 12:03:48.000000000 -0800
@@ -427,13 +427,22 @@ posix_same_owner(struct file_lock *fl1, 
 /* Remove waiter from blocker's block list.
  * When blocker ends up pointing to itself then the list is empty.
  */
-static void locks_delete_block(struct file_lock *waiter)
+static inline void __locks_delete_block(struct file_lock *waiter)
 {
 	list_del_init(&waiter->fl_block);
 	list_del_init(&waiter->fl_link);
 	waiter->fl_next = NULL;
 }
 
+/*
+ */
+static void locks_delete_block(struct file_lock *waiter)
+{
+	lock_kernel();
+	__locks_delete_block(waiter);
+	unlock_kernel();
+}
+
 /* Insert waiter into blocker's block list.
  * We use a circular list so that processes can be easily woken up in
  * the order they blocked. The documentation doesn't require this but
@@ -446,7 +455,7 @@ static void locks_insert_block(struct fi
 		printk(KERN_ERR "locks_insert_block: removing duplicated lock "
 			"(pid=%d %Ld-%Ld type=%d)\n", waiter->fl_pid,
 			waiter->fl_start, waiter->fl_end, waiter->fl_type);
-		locks_delete_block(waiter);
+		__locks_delete_block(waiter);
 	}
 	list_add_tail(&waiter->fl_block, &blocker->fl_block);
 	waiter->fl_next = blocker;
@@ -462,7 +471,7 @@ static void locks_wake_up_blocks(struct 
 	while (!list_empty(&blocker->fl_block)) {
 		struct file_lock *waiter = list_entry(blocker->fl_block.next,
 				struct file_lock, fl_block);
-		locks_delete_block(waiter);
+		__locks_delete_block(waiter);
 		if (waiter->fl_notify)
 			waiter->fl_notify(waiter);
 		else
@@ -589,7 +598,7 @@ static int locks_block_on_timeout(struct
 	int result;
 	locks_insert_block(blocker, waiter);
 	result = interruptible_sleep_on_locked(&waiter->fl_wait, time);
-	locks_delete_block(waiter);
+	__locks_delete_block(waiter);
 	return result;
 }
 
@@ -977,9 +986,7 @@ int locks_mandatory_area(int read_write,
 				continue;
 		}
 
-		lock_kernel();
 		locks_delete_block(&fl);
-		unlock_kernel();
 		break;
 	}
 
@@ -1332,9 +1339,7 @@ asmlinkage long sys_flock(unsigned int f
 		if (!error)
 			continue;
 
-		lock_kernel();
 		locks_delete_block(lock);
-		unlock_kernel();
 		break;
 	}
 
@@ -1489,9 +1494,7 @@ int fcntl_setlk(struct file *filp, unsig
 		if (!error)
 			continue;
 
-		lock_kernel();
 		locks_delete_block(file_lock);
-		unlock_kernel();
 		break;
 	}
 
@@ -1629,9 +1632,7 @@ int fcntl_setlk64(struct file *filp, uns
 		if (!error)
 			continue;
 
-		lock_kernel();
 		locks_delete_block(file_lock);
-		unlock_kernel();
 		break;
 	}
 
@@ -1648,14 +1649,15 @@ out:
  */
 void locks_remove_posix(struct file *filp, fl_owner_t owner)
 {
-	struct file_lock lock;
+	struct file_lock lock, **before;
 
 	/*
 	 * If there are no locks held on this file, we don't need to call
 	 * posix_lock_file().  Another process could be setting a lock on this
 	 * file at the same time, but we wouldn't remove that lock anyway.
 	 */
-	if (!filp->f_dentry->d_inode->i_flock)
+	before = &filp->f_dentry->d_inode->i_flock;
+	if (*before == NULL)
 		return;
 
 	lock.fl_type = F_UNLCK;
@@ -1671,7 +1673,19 @@ void locks_remove_posix(struct file *fil
 		/* Ignore any error -- we must remove the locks anyway */
 	}
 
-	posix_lock_file(filp, &lock);
+	/* Can't use posix_lock_file here; we need to remove it no matter
+	 * which pid we have.
+	 */
+	lock_kernel();
+	while (*before != NULL) {
+		struct file_lock *fl = *before;
+		if (IS_POSIX(fl) && (fl->fl_owner == owner)) {
+			locks_delete_lock(before);
+			continue;
+		}
+		before = &fl->fl_next;
+	}
+	unlock_kernel();
 }
 
 /*
@@ -1699,6 +1713,7 @@ void locks_remove_flock(struct file *fil
 				lease_modify(before, F_UNLCK);
 				continue;
 			}
+			BUG();
  		}
 		before = &fl->fl_next;
 	}
@@ -1733,7 +1748,7 @@ posix_unblock_lock(struct file *filp, st
 	 */
 	lock_kernel();
 	if (waiter->fl_next) {
-		locks_delete_block(waiter);
+		__locks_delete_block(waiter);
 		unlock_kernel();
 	} else {
 		unlock_kernel();
@@ -1785,19 +1800,19 @@ static void lock_get_status(char* out, s
 			       ? (fl->fl_type & F_UNLCK) ? "UNLCK" : "READ "
 			       : (fl->fl_type & F_WRLCK) ? "WRITE" : "READ ");
 	}
-#if WE_CAN_BREAK_LSLK_NOW
 	if (inode) {
+#if WE_CAN_BREAK_LSLK_NOW
 		out += sprintf(out, "%d %s:%ld ", fl->fl_pid,
 				inode->i_sb->s_id, inode->i_ino);
+#else
+		/* userspace relies on this representation of dev_t ;-( */
+		out += sprintf(out, "%d %02x:%02x:%ld ", fl->fl_pid,
+				MAJOR(inode->i_sb->s_dev),
+				MINOR(inode->i_sb->s_dev), inode->i_ino);
+#endif
 	} else {
 		out += sprintf(out, "%d <none>:0 ", fl->fl_pid);
 	}
-#else
-	/* kdevname is a broken interface.  but we expose it to userspace */
-	out += sprintf(out, "%d %s:%ld ", fl->fl_pid,
-			inode ? kdevname(to_kdev_t(inode->i_sb->s_dev)) : "<none>",
-			inode ? inode->i_ino : 0);
-#endif
 	if (IS_POSIX(fl)) {
 		if (fl->fl_end == OFFSET_MAX)
 			out += sprintf(out, "%Ld EOF\n", fl->fl_start);

_