Patch from Matthew Wilcox - 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 :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)) : "", - 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); _