]>
Commit | Line | Data |
---|---|---|
7f3fe4fa GKH |
1 | From nobody Mon Sep 17 00:00:00 2001 |
2 | From: Trond Myklebust <Trond.Myklebust@netapp.com> | |
3 | Date: Fri, 31 Mar 2006 02:30:55 -0800 | |
3401ab13 | 4 | Subject: fs/locks.c: Fix sys_flock() race |
7f3fe4fa GKH |
5 | |
6 | sys_flock() currently has a race which can result in a double free in the | |
7 | multi-thread case. | |
8 | ||
9 | Thread 1 Thread 2 | |
10 | ||
11 | sys_flock(file, LOCK_EX) | |
12 | sys_flock(file, LOCK_UN) | |
13 | ||
14 | If Thread 2 removes the lock from inode->i_lock before Thread 1 tests for | |
15 | list_empty(&lock->fl_link) at the end of sys_flock, then both threads will | |
16 | end up calling locks_free_lock for the same lock. | |
17 | ||
18 | Fix is to make flock_lock_file() do the same as posix_lock_file(), namely | |
19 | to make a copy of the request, so that the caller can always free the lock. | |
20 | ||
21 | This also has the side-effect of fixing up a reference problem in the | |
22 | lockd handling of flock. | |
23 | ||
24 | Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com> | |
25 | Signed-off-by: Andrew Morton <akpm@osdl.org> | |
26 | Signed-off-by: Linus Torvalds <torvalds@osdl.org> | |
27 | Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> | |
eea1e732 | 28 | Signed-off-by: Chris Wright <chrisw@sous-sol.org> |
7f3fe4fa GKH |
29 | |
30 | --- | |
31 | fs/locks.c | 30 ++++++++++++++++-------------- | |
32 | 1 file changed, 16 insertions(+), 14 deletions(-) | |
33 | ||
eea1e732 CW |
34 | --- linux-2.6.16.16.orig/fs/locks.c |
35 | +++ linux-2.6.16.16/fs/locks.c | |
36 | @@ -714,8 +714,9 @@ EXPORT_SYMBOL(posix_locks_deadlock); | |
7f3fe4fa GKH |
37 | * at the head of the list, but that's secret knowledge known only to |
38 | * flock_lock_file and posix_lock_file. | |
39 | */ | |
40 | -static int flock_lock_file(struct file *filp, struct file_lock *new_fl) | |
41 | +static int flock_lock_file(struct file *filp, struct file_lock *request) | |
42 | { | |
43 | + struct file_lock *new_fl = NULL; | |
44 | struct file_lock **before; | |
45 | struct inode * inode = filp->f_dentry->d_inode; | |
46 | int error = 0; | |
eea1e732 | 47 | @@ -730,17 +731,19 @@ static int flock_lock_file(struct file * |
7f3fe4fa GKH |
48 | continue; |
49 | if (filp != fl->fl_file) | |
50 | continue; | |
51 | - if (new_fl->fl_type == fl->fl_type) | |
52 | + if (request->fl_type == fl->fl_type) | |
53 | goto out; | |
54 | found = 1; | |
55 | locks_delete_lock(before); | |
56 | break; | |
57 | } | |
58 | - unlock_kernel(); | |
59 | ||
60 | - if (new_fl->fl_type == F_UNLCK) | |
61 | - return 0; | |
62 | + if (request->fl_type == F_UNLCK) | |
63 | + goto out; | |
64 | ||
65 | + new_fl = locks_alloc_lock(); | |
66 | + if (new_fl == NULL) | |
67 | + goto out; | |
68 | /* | |
69 | * If a higher-priority process was blocked on the old file lock, | |
70 | * give it the opportunity to lock the file. | |
eea1e732 | 71 | @@ -748,26 +751,27 @@ static int flock_lock_file(struct file * |
7f3fe4fa GKH |
72 | if (found) |
73 | cond_resched(); | |
74 | ||
75 | - lock_kernel(); | |
76 | for_each_lock(inode, before) { | |
77 | struct file_lock *fl = *before; | |
78 | if (IS_POSIX(fl)) | |
79 | break; | |
80 | if (IS_LEASE(fl)) | |
81 | continue; | |
82 | - if (!flock_locks_conflict(new_fl, fl)) | |
83 | + if (!flock_locks_conflict(request, fl)) | |
84 | continue; | |
85 | error = -EAGAIN; | |
86 | - if (new_fl->fl_flags & FL_SLEEP) { | |
87 | - locks_insert_block(fl, new_fl); | |
88 | - } | |
89 | + if (request->fl_flags & FL_SLEEP) | |
90 | + locks_insert_block(fl, request); | |
91 | goto out; | |
92 | } | |
93 | + locks_copy_lock(new_fl, request); | |
94 | locks_insert_lock(&inode->i_flock, new_fl); | |
95 | - error = 0; | |
96 | + new_fl = NULL; | |
97 | ||
98 | out: | |
99 | unlock_kernel(); | |
100 | + if (new_fl) | |
101 | + locks_free_lock(new_fl); | |
102 | return error; | |
103 | } | |
104 | ||
eea1e732 | 105 | @@ -1532,9 +1536,7 @@ asmlinkage long sys_flock(unsigned int f |
7f3fe4fa GKH |
106 | error = flock_lock_file_wait(filp, lock); |
107 | ||
108 | out_free: | |
109 | - if (list_empty(&lock->fl_link)) { | |
110 | - locks_free_lock(lock); | |
111 | - } | |
112 | + locks_free_lock(lock); | |
113 | ||
114 | out_putf: | |
115 | fput(filp); |