Patch from: Matthew Wilcox Robbie Williamson found some bugs in the mandatory locking implementation. This patch fixes all the problems he found: - Fix null pointer dereference caused by sys_truncate() passing a null filp. - Honour the O_NONBLOCK flag when calling ftruncate() - Local variable `fl' wasn't being initialised correctly in locks_mandatory_area() - Don't return -ENOLCK from __posix_lock_file() when FL_ACCESS is set. fs/locks.c | 189 +++++++++++++++++++++++++++++++++---------------------------- 1 files changed, 105 insertions(+), 84 deletions(-) diff -puN fs/locks.c~mandlock-fix fs/locks.c --- 25/fs/locks.c~mandlock-fix Thu Feb 13 13:05:56 2003 +++ 25-akpm/fs/locks.c Thu Feb 13 13:05:56 2003 @@ -652,63 +652,6 @@ next_task: return 0; } -int locks_mandatory_locked(struct inode *inode) -{ - fl_owner_t owner = current->files; - struct file_lock *fl; - - /* - * Search the lock list for this inode for any POSIX locks. - */ - lock_kernel(); - for (fl = inode->i_flock; fl != NULL; fl = fl->fl_next) { - if (!IS_POSIX(fl)) - continue; - if (fl->fl_owner != owner) - break; - } - unlock_kernel(); - return fl ? -EAGAIN : 0; -} - -int locks_mandatory_area(int read_write, struct inode *inode, - struct file *filp, loff_t offset, - size_t count) -{ - struct file_lock fl; - int error; - - fl.fl_owner = current->files; - fl.fl_pid = current->tgid; - fl.fl_file = filp; - fl.fl_flags = FL_POSIX | FL_ACCESS | FL_SLEEP; - fl.fl_type = (read_write == FLOCK_VERIFY_WRITE) ? F_WRLCK : F_RDLCK; - fl.fl_start = offset; - fl.fl_end = offset + count - 1; - - for (;;) { - error = posix_lock_file(filp, &fl); - if (error != -EAGAIN) - break; - error = wait_event_interruptible(fl.fl_wait, !fl.fl_next); - if (!error) { - /* - * If we've been sleeping someone might have - * changed the permissions behind our back. - */ - if ((inode->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID) - continue; - } - - lock_kernel(); - locks_delete_block(&fl); - unlock_kernel(); - break; - } - - return error; -} - /* Try to create a FLOCK lock on filp. We always insert new FLOCK locks * at the head of the list, but that's secret knowledge known only to * flock_lock_file and posix_lock_file. @@ -770,32 +713,13 @@ out: return error; } -/** - * posix_lock_file: - * @filp: The file to apply the lock to - * @caller: The lock to be applied - * @wait: 1 to retry automatically, 0 to return -EAGAIN - * - * Add a POSIX style lock to a file. - * We merge adjacent locks whenever possible. POSIX locks are sorted by owner - * task, then by starting address - * - * Kai Petzke writes: - * To make freeing a lock much faster, we keep a pointer to the lock before the - * actual one. But the real gain of the new coding was, that lock_it() and - * unlock_it() became one function. - * - * To all purists: Yes, I use a few goto's. Just pass on to the next function. - */ - -int posix_lock_file(struct file *filp, struct file_lock *request) +static int __posix_lock_file(struct inode *inode, struct file_lock *request) { struct file_lock *fl; struct file_lock *new_fl, *new_fl2; struct file_lock *left = NULL; struct file_lock *right = NULL; struct file_lock **before; - struct inode * inode = filp->f_dentry->d_inode; int error, added = 0; /* @@ -804,9 +728,6 @@ int posix_lock_file(struct file *filp, s */ new_fl = locks_alloc_lock(0); new_fl2 = locks_alloc_lock(0); - error = -ENOLCK; /* "no luck" */ - if (!(new_fl && new_fl2)) - goto out_nolock; lock_kernel(); if (request->fl_type != F_UNLCK) { @@ -829,9 +750,14 @@ int posix_lock_file(struct file *filp, s } /* If we're just looking for a conflict, we're done. */ + error = 0; if (request->fl_flags & FL_ACCESS) goto out; + error = -ENOLCK; /* "no luck" */ + if (!(new_fl && new_fl2)) + goto out; + /* * We've allocated the new locks in advance, so there are no * errors possible (and no blocking operations) from here on. @@ -952,9 +878,8 @@ int posix_lock_file(struct file *filp, s left->fl_end = request->fl_start - 1; locks_wake_up_blocks(left); } -out: + out: unlock_kernel(); -out_nolock: /* * Free any unused locks. */ @@ -965,6 +890,102 @@ out_nolock: return error; } +/** + * posix_lock_file - Apply a POSIX-style lock to a file + * @filp: The file to apply the lock to + * @fl: The lock to be applied + * + * Add a POSIX style lock to a file. + * We merge adjacent & overlapping locks whenever possible. + * POSIX locks are sorted by owner task, then by starting address + */ +int posix_lock_file(struct file *filp, struct file_lock *fl) +{ + return __posix_lock_file(filp->f_dentry->d_inode, fl); +} + +/** + * locks_mandatory_locked - Check for an active lock + * @inode: the file to check + * + * Searches the inode's list of locks to find any POSIX locks which conflict. + * This function is called from locks_verify_locked() only. + */ +int locks_mandatory_locked(struct inode *inode) +{ + fl_owner_t owner = current->files; + struct file_lock *fl; + + /* + * Search the lock list for this inode for any POSIX locks. + */ + lock_kernel(); + for (fl = inode->i_flock; fl != NULL; fl = fl->fl_next) { + if (!IS_POSIX(fl)) + continue; + if (fl->fl_owner != owner) + break; + } + unlock_kernel(); + return fl ? -EAGAIN : 0; +} + +/** + * locks_mandatory_area - Check for a conflicting lock + * @read_write: %FLOCK_VERIFY_WRITE for exclusive access, %FLOCK_VERIFY_READ + * for shared + * @inode: the file to check + * @file: how the file was opened (if it was) + * @offset: start of area to check + * @count: length of area to check + * + * Searches the inode's list of locks to find any POSIX locks which conflict. + * This function is called from locks_verify_area() and + * locks_verify_truncate(). + */ +int locks_mandatory_area(int read_write, struct inode *inode, + struct file *filp, loff_t offset, + size_t count) +{ + struct file_lock fl; + int error; + + locks_init_lock(&fl); + fl.fl_owner = current->files; + fl.fl_pid = current->tgid; + fl.fl_file = filp; + fl.fl_flags = FL_POSIX | FL_ACCESS; + if (filp && !(filp->f_flags & O_NONBLOCK)) + fl.fl_flags |= FL_SLEEP; + fl.fl_type = (read_write == FLOCK_VERIFY_WRITE) ? F_WRLCK : F_RDLCK; + fl.fl_start = offset; + fl.fl_end = offset + count - 1; + + for (;;) { + error = __posix_lock_file(inode, &fl); + if (error != -EAGAIN) + break; + if (!(fl.fl_flags & FL_SLEEP)) + break; + error = wait_event_interruptible(fl.fl_wait, !fl.fl_next); + if (!error) { + /* + * If we've been sleeping someone might have + * changed the permissions behind our back. + */ + if ((inode->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID) + continue; + } + + lock_kernel(); + locks_delete_block(&fl); + unlock_kernel(); + break; + } + + return error; +} + /* We already had a lease on this file; just change its type */ static int lease_modify(struct file_lock **before, int arg) { @@ -1460,7 +1481,7 @@ int fcntl_setlk(struct file *filp, unsig } for (;;) { - error = posix_lock_file(filp, file_lock); + error = __posix_lock_file(inode, file_lock); if ((error != -EAGAIN) || (cmd == F_SETLK)) break; error = wait_event_interruptible(file_lock->fl_wait, @@ -1600,7 +1621,7 @@ int fcntl_setlk64(struct file *filp, uns } for (;;) { - error = posix_lock_file(filp, file_lock); + error = __posix_lock_file(inode, file_lock); if ((error != -EAGAIN) || (cmd == F_SETLK64)) break; error = wait_event_interruptible(file_lock->fl_wait, _