Commit d69b78ba1deaaa95ffa8dac5a9ca819ce454d52e
Committed by
Jens Axboe
1 parent
1ff5125fb8
ioprio: grab rcu_read_lock in sys_ioprio_{set,get}()
Using: - CONFIG_LOCKUP_DETECTOR=y - CONFIG_PREEMPT=y - CONFIG_LOCKDEP=y - CONFIG_PROVE_LOCKING=y - CONFIG_PROVE_RCU=y found a missing rcu lock during boot on a 512 MiB x86_64 ubuntu vm: =================================================== [ INFO: suspicious rcu_dereference_check() usage. ] --------------------------------------------------- kernel/pid.c:419 invoked rcu_dereference_check() without protection! other info that might help us debug this: rcu_scheduler_active = 1, debug_locks = 0 1 lock held by ureadahead/1355: #0: (tasklist_lock){.+.+..}, at: [<ffffffff8115bc09>] sys_ioprio_set+0x7f/0x29e stack backtrace: Pid: 1355, comm: ureadahead Not tainted 2.6.37-dbg-DEV #1 Call Trace: [<ffffffff8109c10c>] lockdep_rcu_dereference+0xaa/0xb3 [<ffffffff81088cbf>] find_task_by_pid_ns+0x44/0x5d [<ffffffff81088cfa>] find_task_by_vpid+0x22/0x24 [<ffffffff8115bc3e>] sys_ioprio_set+0xb4/0x29e [<ffffffff8147cf21>] ? trace_hardirqs_off_thunk+0x3a/0x3c [<ffffffff8105c409>] sysenter_dispatch+0x7/0x2c [<ffffffff8147cee2>] ? trace_hardirqs_on_thunk+0x3a/0x3f The fix is to: a) grab rcu lock in sys_ioprio_{set,get}() and b) avoid grabbing tasklist_lock. Discussion in: http://marc.info/?l=linux-kernel&m=128951324702889 Signed-off-by: Greg Thelen <gthelen@google.com> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Reviewed-by: Oleg Nesterov <oleg@redhat.com> Modified by Jens to remove the now redundant inner rcu lock and unlock since they are now protected by the outer lock. Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
Showing 1 changed file with 6 additions and 25 deletions Side-by-side Diff
fs/ioprio.c
... | ... | @@ -103,22 +103,15 @@ |
103 | 103 | } |
104 | 104 | |
105 | 105 | ret = -ESRCH; |
106 | - /* | |
107 | - * We want IOPRIO_WHO_PGRP/IOPRIO_WHO_USER to be "atomic", | |
108 | - * so we can't use rcu_read_lock(). See re-copy of ->ioprio | |
109 | - * in copy_process(). | |
110 | - */ | |
111 | - read_lock(&tasklist_lock); | |
106 | + rcu_read_lock(); | |
112 | 107 | switch (which) { |
113 | 108 | case IOPRIO_WHO_PROCESS: |
114 | - rcu_read_lock(); | |
115 | 109 | if (!who) |
116 | 110 | p = current; |
117 | 111 | else |
118 | 112 | p = find_task_by_vpid(who); |
119 | 113 | if (p) |
120 | 114 | ret = set_task_ioprio(p, ioprio); |
121 | - rcu_read_unlock(); | |
122 | 115 | break; |
123 | 116 | case IOPRIO_WHO_PGRP: |
124 | 117 | if (!who) |
... | ... | @@ -141,12 +134,7 @@ |
141 | 134 | break; |
142 | 135 | |
143 | 136 | do_each_thread(g, p) { |
144 | - int match; | |
145 | - | |
146 | - rcu_read_lock(); | |
147 | - match = __task_cred(p)->uid == who; | |
148 | - rcu_read_unlock(); | |
149 | - if (!match) | |
137 | + if (__task_cred(p)->uid != who) | |
150 | 138 | continue; |
151 | 139 | ret = set_task_ioprio(p, ioprio); |
152 | 140 | if (ret) |
... | ... | @@ -160,7 +148,7 @@ |
160 | 148 | ret = -EINVAL; |
161 | 149 | } |
162 | 150 | |
163 | - read_unlock(&tasklist_lock); | |
151 | + rcu_read_unlock(); | |
164 | 152 | return ret; |
165 | 153 | } |
166 | 154 | |
167 | 155 | |
168 | 156 | |
... | ... | @@ -204,17 +192,15 @@ |
204 | 192 | int ret = -ESRCH; |
205 | 193 | int tmpio; |
206 | 194 | |
207 | - read_lock(&tasklist_lock); | |
195 | + rcu_read_lock(); | |
208 | 196 | switch (which) { |
209 | 197 | case IOPRIO_WHO_PROCESS: |
210 | - rcu_read_lock(); | |
211 | 198 | if (!who) |
212 | 199 | p = current; |
213 | 200 | else |
214 | 201 | p = find_task_by_vpid(who); |
215 | 202 | if (p) |
216 | 203 | ret = get_task_ioprio(p); |
217 | - rcu_read_unlock(); | |
218 | 204 | break; |
219 | 205 | case IOPRIO_WHO_PGRP: |
220 | 206 | if (!who) |
... | ... | @@ -241,12 +227,7 @@ |
241 | 227 | break; |
242 | 228 | |
243 | 229 | do_each_thread(g, p) { |
244 | - int match; | |
245 | - | |
246 | - rcu_read_lock(); | |
247 | - match = __task_cred(p)->uid == user->uid; | |
248 | - rcu_read_unlock(); | |
249 | - if (!match) | |
230 | + if (__task_cred(p)->uid != user->uid) | |
250 | 231 | continue; |
251 | 232 | tmpio = get_task_ioprio(p); |
252 | 233 | if (tmpio < 0) |
... | ... | @@ -264,7 +245,7 @@ |
264 | 245 | ret = -EINVAL; |
265 | 246 | } |
266 | 247 | |
267 | - read_unlock(&tasklist_lock); | |
248 | + rcu_read_unlock(); | |
268 | 249 | return ret; |
269 | 250 | } |