Commit 9c57693e authored by Matthew Wilcox's avatar Matthew Wilcox Committed by Jens Axboe

[PATCH] Fix mandatory locking

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.
parent ffd23335
...@@ -652,63 +652,6 @@ int posix_locks_deadlock(struct file_lock *caller_fl, ...@@ -652,63 +652,6 @@ int posix_locks_deadlock(struct file_lock *caller_fl,
return 0; 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 /* 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 * at the head of the list, but that's secret knowledge known only to
* flock_lock_file and posix_lock_file. * flock_lock_file and posix_lock_file.
...@@ -770,32 +713,13 @@ static int flock_lock_file(struct file *filp, struct file_lock *new_fl) ...@@ -770,32 +713,13 @@ static int flock_lock_file(struct file *filp, struct file_lock *new_fl)
return error; return error;
} }
/** static int __posix_lock_file(struct inode *inode, struct file_lock *request)
* 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)
{ {
struct file_lock *fl; struct file_lock *fl;
struct file_lock *new_fl, *new_fl2; struct file_lock *new_fl, *new_fl2;
struct file_lock *left = NULL; struct file_lock *left = NULL;
struct file_lock *right = NULL; struct file_lock *right = NULL;
struct file_lock **before; struct file_lock **before;
struct inode * inode = filp->f_dentry->d_inode;
int error, added = 0; int error, added = 0;
/* /*
...@@ -804,9 +728,6 @@ int posix_lock_file(struct file *filp, struct file_lock *request) ...@@ -804,9 +728,6 @@ int posix_lock_file(struct file *filp, struct file_lock *request)
*/ */
new_fl = locks_alloc_lock(0); new_fl = locks_alloc_lock(0);
new_fl2 = locks_alloc_lock(0); new_fl2 = locks_alloc_lock(0);
error = -ENOLCK; /* "no luck" */
if (!(new_fl && new_fl2))
goto out_nolock;
lock_kernel(); lock_kernel();
if (request->fl_type != F_UNLCK) { if (request->fl_type != F_UNLCK) {
...@@ -829,9 +750,14 @@ int posix_lock_file(struct file *filp, struct file_lock *request) ...@@ -829,9 +750,14 @@ int posix_lock_file(struct file *filp, struct file_lock *request)
} }
/* If we're just looking for a conflict, we're done. */ /* If we're just looking for a conflict, we're done. */
error = 0;
if (request->fl_flags & FL_ACCESS) if (request->fl_flags & FL_ACCESS)
goto out; 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 * We've allocated the new locks in advance, so there are no
* errors possible (and no blocking operations) from here on. * errors possible (and no blocking operations) from here on.
...@@ -952,9 +878,8 @@ int posix_lock_file(struct file *filp, struct file_lock *request) ...@@ -952,9 +878,8 @@ int posix_lock_file(struct file *filp, struct file_lock *request)
left->fl_end = request->fl_start - 1; left->fl_end = request->fl_start - 1;
locks_wake_up_blocks(left); locks_wake_up_blocks(left);
} }
out: out:
unlock_kernel(); unlock_kernel();
out_nolock:
/* /*
* Free any unused locks. * Free any unused locks.
*/ */
...@@ -965,6 +890,102 @@ int posix_lock_file(struct file *filp, struct file_lock *request) ...@@ -965,6 +890,102 @@ int posix_lock_file(struct file *filp, struct file_lock *request)
return error; 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 */ /* We already had a lease on this file; just change its type */
static int lease_modify(struct file_lock **before, int arg) static int lease_modify(struct file_lock **before, int arg)
{ {
...@@ -1460,7 +1481,7 @@ int fcntl_setlk(struct file *filp, unsigned int cmd, struct flock *l) ...@@ -1460,7 +1481,7 @@ int fcntl_setlk(struct file *filp, unsigned int cmd, struct flock *l)
} }
for (;;) { for (;;) {
error = posix_lock_file(filp, file_lock); error = __posix_lock_file(inode, file_lock);
if ((error != -EAGAIN) || (cmd == F_SETLK)) if ((error != -EAGAIN) || (cmd == F_SETLK))
break; break;
error = wait_event_interruptible(file_lock->fl_wait, error = wait_event_interruptible(file_lock->fl_wait,
...@@ -1600,7 +1621,7 @@ int fcntl_setlk64(struct file *filp, unsigned int cmd, struct flock64 *l) ...@@ -1600,7 +1621,7 @@ int fcntl_setlk64(struct file *filp, unsigned int cmd, struct flock64 *l)
} }
for (;;) { for (;;) {
error = posix_lock_file(filp, file_lock); error = __posix_lock_file(inode, file_lock);
if ((error != -EAGAIN) || (cmd == F_SETLK64)) if ((error != -EAGAIN) || (cmd == F_SETLK64))
break; break;
error = wait_event_interruptible(file_lock->fl_wait, error = wait_event_interruptible(file_lock->fl_wait,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment