Commit 1447399b3e34af016c368b4178db7ef0e04e15b0
Committed by
Jens Axboe
Jens Axboe
1 parent
a014741c0a
Exists in
master
and in
4 other branches
ioprio: fix RCU locking around task dereference
With 2.6.37-rc1, I observe sys_ioprio_set not taking the RCU lock [1]
across access to the task credentials.
Inspecting the code in fs/ioprio.c, the tasklist_lock is held for read
across the __task_cred call, which is presumably sufficient to prevent
the task credentials becoming stale.
===================================================
[ 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 = 1
1 lock held by start-stop-daem/2246:
#0: (tasklist_lock){.?.?..}, at: [<ffffffff811a2dfa>]
sys_ioprio_set+0x8a/0x400
stack backtrace:
Pid: 2246, comm: start-stop-daem Not tainted 2.6.37-rc1-330cd+ #2
Call Trace:
[<ffffffff8109f5f4>] lockdep_rcu_dereference+0xa4/0xc0
[<ffffffff81085651>] find_task_by_pid_ns+0x81/0x90
[<ffffffff8108567d>] find_task_by_vpid+0x1d/0x20
[<ffffffff811a3160>] sys_ioprio_set+0x3f0/0x400
[<ffffffff816efa79>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[<ffffffff81003482>] system_call_fastpath+0x16/0x1b
Take the RCU lock for read across acquiring the pointer to the task
credentials and dereferencing it.
Signed-off-by: Daniel J Blueman <daniel.blueman@gmail.com>
Fixed up by Jens to fix missing rcu_read_unlock() on mismatches.
Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
Showing 1 changed file with 12 additions and 2 deletions Side-by-side Diff
fs/ioprio.c
| ... | ... | @@ -139,7 +139,12 @@ |
| 139 | 139 | break; |
| 140 | 140 | |
| 141 | 141 | do_each_thread(g, p) { |
| 142 | - if (__task_cred(p)->uid != who) | |
| 142 | + int match; | |
| 143 | + | |
| 144 | + rcu_read_lock(); | |
| 145 | + match = __task_cred(p)->uid == who; | |
| 146 | + rcu_read_unlock(); | |
| 147 | + if (!match) | |
| 143 | 148 | continue; |
| 144 | 149 | ret = set_task_ioprio(p, ioprio); |
| 145 | 150 | if (ret) |
| ... | ... | @@ -232,7 +237,12 @@ |
| 232 | 237 | break; |
| 233 | 238 | |
| 234 | 239 | do_each_thread(g, p) { |
| 235 | - if (__task_cred(p)->uid != user->uid) | |
| 240 | + int match; | |
| 241 | + | |
| 242 | + rcu_read_lock(); | |
| 243 | + match = __task_cred(p)->uid == user->uid; | |
| 244 | + rcu_read_unlock(); | |
| 245 | + if (!match) | |
| 236 | 246 | continue; |
| 237 | 247 | tmpio = get_task_ioprio(p); |
| 238 | 248 | if (tmpio < 0) |