diff options
author | Pavel Emelyanov <xemul@openvz.org> | 2007-09-20 04:45:02 -0400 |
---|---|---|
committer | J. Bruce Fields <bfields@citi.umich.edu> | 2007-10-09 18:32:45 -0400 |
commit | 85c59580b30c82aa771aa33b37217a6b6851bc14 (patch) | |
tree | 02e96b437a63908ad3791021d352ad2aefff9936 | |
parent | f0c1cd0eaf0b127356c2c09e40305453bc361b0f (diff) |
locks: Fix potential OOPS in generic_setlease()
This code is run under lock_kernel(), which is dropped during
sleeping operations, so the following race is possible:
CPU1: CPU2:
vfs_setlease(); vfs_setlease();
lock_kernel();
lock_kernel(); /* spin */
generic_setlease():
...
for (before = ...)
/* here we found some lease after
* which we will insert the new one
*/
fl = locks_alloc_lock();
/* go to sleep in this allocation and
* drop the BKL
*/
generic_setlease():
...
for (before = ...)
/* here we find the "before" pointing
* at the one we found on CPU1
*/
->fl_change(my_before, arg);
lease_modify();
locks_free_lock();
/* and we freed it */
...
unlock_kernel();
locks_insert_lock(before, fl);
/* OOPS! We have just tried to add the lease
* at the tail of already removed one
*/
The similar races are already handled in other code - all the
allocations are performed before any checks/updates.
Thanks to Kamalesh Babulal for testing and for a bug report on an
earlier version.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
Cc: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
-rw-r--r-- | fs/locks.c | 21 |
1 files changed, 12 insertions, 9 deletions
diff --git a/fs/locks.c b/fs/locks.c index 282b6c11670a..43dbc7f566fa 100644 --- a/fs/locks.c +++ b/fs/locks.c | |||
@@ -1343,6 +1343,7 @@ int fcntl_getlease(struct file *filp) | |||
1343 | int generic_setlease(struct file *filp, long arg, struct file_lock **flp) | 1343 | int generic_setlease(struct file *filp, long arg, struct file_lock **flp) |
1344 | { | 1344 | { |
1345 | struct file_lock *fl, **before, **my_before = NULL, *lease; | 1345 | struct file_lock *fl, **before, **my_before = NULL, *lease; |
1346 | struct file_lock *new_fl = NULL; | ||
1346 | struct dentry *dentry = filp->f_path.dentry; | 1347 | struct dentry *dentry = filp->f_path.dentry; |
1347 | struct inode *inode = dentry->d_inode; | 1348 | struct inode *inode = dentry->d_inode; |
1348 | int error, rdlease_count = 0, wrlease_count = 0; | 1349 | int error, rdlease_count = 0, wrlease_count = 0; |
@@ -1369,6 +1370,11 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp) | |||
1369 | || (atomic_read(&inode->i_count) > 1))) | 1370 | || (atomic_read(&inode->i_count) > 1))) |
1370 | goto out; | 1371 | goto out; |
1371 | 1372 | ||
1373 | error = -ENOMEM; | ||
1374 | new_fl = locks_alloc_lock(); | ||
1375 | if (new_fl == NULL) | ||
1376 | goto out; | ||
1377 | |||
1372 | /* | 1378 | /* |
1373 | * At this point, we know that if there is an exclusive | 1379 | * At this point, we know that if there is an exclusive |
1374 | * lease on this file, then we hold it on this filp | 1380 | * lease on this file, then we hold it on this filp |
@@ -1411,18 +1417,15 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp) | |||
1411 | if (!leases_enable) | 1417 | if (!leases_enable) |
1412 | goto out; | 1418 | goto out; |
1413 | 1419 | ||
1414 | error = -ENOMEM; | 1420 | locks_copy_lock(new_fl, lease); |
1415 | fl = locks_alloc_lock(); | 1421 | locks_insert_lock(before, new_fl); |
1416 | if (fl == NULL) | ||
1417 | goto out; | ||
1418 | |||
1419 | locks_copy_lock(fl, lease); | ||
1420 | 1422 | ||
1421 | locks_insert_lock(before, fl); | 1423 | *flp = new_fl; |
1424 | return 0; | ||
1422 | 1425 | ||
1423 | *flp = fl; | ||
1424 | error = 0; | ||
1425 | out: | 1426 | out: |
1427 | if (new_fl != NULL) | ||
1428 | locks_free_lock(new_fl); | ||
1426 | return error; | 1429 | return error; |
1427 | } | 1430 | } |
1428 | EXPORT_SYMBOL(generic_setlease); | 1431 | EXPORT_SYMBOL(generic_setlease); |