Skip to content

Commit eb94cd9

Browse files
Cyrill Gorcunovtorvalds
Cyrill Gorcunov
authored andcommitted
fs, proc: fix ABBA deadlock in case of execution attempt of map_files/ entries
map_files/ entries are never supposed to be executed, still curious minds might try to run them, which leads to the following deadlock ====================================================== [ INFO: possible circular locking dependency detected ] 3.4.0-rc4-24406-g841e6a6 #121 Not tainted ------------------------------------------------------- bash/1556 is trying to acquire lock: (&sb->s_type->i_mutex_key#8){+.+.+.}, at: do_lookup+0x267/0x2b1 but task is already holding lock: (&sig->cred_guard_mutex){+.+.+.}, at: prepare_bprm_creds+0x2d/0x69 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&sig->cred_guard_mutex){+.+.+.}: validate_chain+0x444/0x4f4 __lock_acquire+0x387/0x3f8 lock_acquire+0x12b/0x158 __mutex_lock_common+0x56/0x3a9 mutex_lock_killable_nested+0x40/0x45 lock_trace+0x24/0x59 proc_map_files_lookup+0x5a/0x165 __lookup_hash+0x52/0x73 do_lookup+0x276/0x2b1 walk_component+0x3d/0x114 do_last+0xfc/0x540 path_openat+0xd3/0x306 do_filp_open+0x3d/0x89 do_sys_open+0x74/0x106 sys_open+0x21/0x23 tracesys+0xdd/0xe2 -> #0 (&sb->s_type->i_mutex_key#8){+.+.+.}: check_prev_add+0x6a/0x1ef validate_chain+0x444/0x4f4 __lock_acquire+0x387/0x3f8 lock_acquire+0x12b/0x158 __mutex_lock_common+0x56/0x3a9 mutex_lock_nested+0x40/0x45 do_lookup+0x267/0x2b1 walk_component+0x3d/0x114 link_path_walk+0x1f9/0x48f path_openat+0xb6/0x306 do_filp_open+0x3d/0x89 open_exec+0x25/0xa0 do_execve_common+0xea/0x2f9 do_execve+0x43/0x45 sys_execve+0x43/0x5a stub_execve+0x6c/0xc0 This is because prepare_bprm_creds grabs task->signal->cred_guard_mutex and when do_lookup happens we try to grab task->signal->cred_guard_mutex again in lock_trace. Fix it using plain ptrace_may_access() helper in proc_map_files_lookup() and in proc_map_files_readdir() instead of lock_trace(), the caller must be CAP_SYS_ADMIN granted anyway. Signed-off-by: Cyrill Gorcunov <[email protected]> Reported-by: Sasha Levin <[email protected]> Cc: Konstantin Khlebnikov <[email protected]> Cc: Pavel Emelyanov <[email protected]> Cc: Dave Jones <[email protected]> Cc: Vasiliy Kulikov <[email protected]> Cc: Oleg Nesterov <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent c0a5f4a commit eb94cd9

File tree

1 file changed

+8
-12
lines changed

1 file changed

+8
-12
lines changed

fs/proc/base.c

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2177,16 +2177,16 @@ static struct dentry *proc_map_files_lookup(struct inode *dir,
21772177
goto out;
21782178

21792179
result = ERR_PTR(-EACCES);
2180-
if (lock_trace(task))
2180+
if (!ptrace_may_access(task, PTRACE_MODE_READ))
21812181
goto out_put_task;
21822182

21832183
result = ERR_PTR(-ENOENT);
21842184
if (dname_to_vma_addr(dentry, &vm_start, &vm_end))
2185-
goto out_unlock;
2185+
goto out_put_task;
21862186

21872187
mm = get_task_mm(task);
21882188
if (!mm)
2189-
goto out_unlock;
2189+
goto out_put_task;
21902190

21912191
down_read(&mm->mmap_sem);
21922192
vma = find_exact_vma(mm, vm_start, vm_end);
@@ -2198,8 +2198,6 @@ static struct dentry *proc_map_files_lookup(struct inode *dir,
21982198
out_no_vma:
21992199
up_read(&mm->mmap_sem);
22002200
mmput(mm);
2201-
out_unlock:
2202-
unlock_trace(task);
22032201
out_put_task:
22042202
put_task_struct(task);
22052203
out:
@@ -2233,20 +2231,20 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir)
22332231
goto out;
22342232

22352233
ret = -EACCES;
2236-
if (lock_trace(task))
2234+
if (!ptrace_may_access(task, PTRACE_MODE_READ))
22372235
goto out_put_task;
22382236

22392237
ret = 0;
22402238
switch (filp->f_pos) {
22412239
case 0:
22422240
ino = inode->i_ino;
22432241
if (filldir(dirent, ".", 1, 0, ino, DT_DIR) < 0)
2244-
goto out_unlock;
2242+
goto out_put_task;
22452243
filp->f_pos++;
22462244
case 1:
22472245
ino = parent_ino(dentry);
22482246
if (filldir(dirent, "..", 2, 1, ino, DT_DIR) < 0)
2249-
goto out_unlock;
2247+
goto out_put_task;
22502248
filp->f_pos++;
22512249
default:
22522250
{
@@ -2257,7 +2255,7 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir)
22572255

22582256
mm = get_task_mm(task);
22592257
if (!mm)
2260-
goto out_unlock;
2258+
goto out_put_task;
22612259
down_read(&mm->mmap_sem);
22622260

22632261
nr_files = 0;
@@ -2287,7 +2285,7 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir)
22872285
flex_array_free(fa);
22882286
up_read(&mm->mmap_sem);
22892287
mmput(mm);
2290-
goto out_unlock;
2288+
goto out_put_task;
22912289
}
22922290
for (i = 0, vma = mm->mmap, pos = 2; vma;
22932291
vma = vma->vm_next) {
@@ -2332,8 +2330,6 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir)
23322330
}
23332331
}
23342332

2335-
out_unlock:
2336-
unlock_trace(task);
23372333
out_put_task:
23382334
put_task_struct(task);
23392335
out:

0 commit comments

Comments
 (0)