Commit eb94cd96 authored by Cyrill Gorcunov's avatar Cyrill Gorcunov Committed by Linus Torvalds

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: default avatarCyrill Gorcunov <gorcunov@openvz.org>
Reported-by: default avatarSasha Levin <levinsasha928@gmail.com>
Cc: Konstantin Khlebnikov <khlebnikov@openvz.org>
Cc: Pavel Emelyanov <xemul@openvz.org>
Cc: Dave Jones <davej@redhat.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent c0a5f4a0
...@@ -2177,16 +2177,16 @@ static struct dentry *proc_map_files_lookup(struct inode *dir, ...@@ -2177,16 +2177,16 @@ static struct dentry *proc_map_files_lookup(struct inode *dir,
goto out; goto out;
result = ERR_PTR(-EACCES); result = ERR_PTR(-EACCES);
if (lock_trace(task)) if (!ptrace_may_access(task, PTRACE_MODE_READ))
goto out_put_task; goto out_put_task;
result = ERR_PTR(-ENOENT); result = ERR_PTR(-ENOENT);
if (dname_to_vma_addr(dentry, &vm_start, &vm_end)) if (dname_to_vma_addr(dentry, &vm_start, &vm_end))
goto out_unlock; goto out_put_task;
mm = get_task_mm(task); mm = get_task_mm(task);
if (!mm) if (!mm)
goto out_unlock; goto out_put_task;
down_read(&mm->mmap_sem); down_read(&mm->mmap_sem);
vma = find_exact_vma(mm, vm_start, vm_end); vma = find_exact_vma(mm, vm_start, vm_end);
...@@ -2198,8 +2198,6 @@ static struct dentry *proc_map_files_lookup(struct inode *dir, ...@@ -2198,8 +2198,6 @@ static struct dentry *proc_map_files_lookup(struct inode *dir,
out_no_vma: out_no_vma:
up_read(&mm->mmap_sem); up_read(&mm->mmap_sem);
mmput(mm); mmput(mm);
out_unlock:
unlock_trace(task);
out_put_task: out_put_task:
put_task_struct(task); put_task_struct(task);
out: out:
...@@ -2233,7 +2231,7 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -2233,7 +2231,7 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir)
goto out; goto out;
ret = -EACCES; ret = -EACCES;
if (lock_trace(task)) if (!ptrace_may_access(task, PTRACE_MODE_READ))
goto out_put_task; goto out_put_task;
ret = 0; ret = 0;
...@@ -2241,12 +2239,12 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -2241,12 +2239,12 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir)
case 0: case 0:
ino = inode->i_ino; ino = inode->i_ino;
if (filldir(dirent, ".", 1, 0, ino, DT_DIR) < 0) if (filldir(dirent, ".", 1, 0, ino, DT_DIR) < 0)
goto out_unlock; goto out_put_task;
filp->f_pos++; filp->f_pos++;
case 1: case 1:
ino = parent_ino(dentry); ino = parent_ino(dentry);
if (filldir(dirent, "..", 2, 1, ino, DT_DIR) < 0) if (filldir(dirent, "..", 2, 1, ino, DT_DIR) < 0)
goto out_unlock; goto out_put_task;
filp->f_pos++; filp->f_pos++;
default: default:
{ {
...@@ -2257,7 +2255,7 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -2257,7 +2255,7 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir)
mm = get_task_mm(task); mm = get_task_mm(task);
if (!mm) if (!mm)
goto out_unlock; goto out_put_task;
down_read(&mm->mmap_sem); down_read(&mm->mmap_sem);
nr_files = 0; nr_files = 0;
...@@ -2287,7 +2285,7 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -2287,7 +2285,7 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir)
flex_array_free(fa); flex_array_free(fa);
up_read(&mm->mmap_sem); up_read(&mm->mmap_sem);
mmput(mm); mmput(mm);
goto out_unlock; goto out_put_task;
} }
for (i = 0, vma = mm->mmap, pos = 2; vma; for (i = 0, vma = mm->mmap, pos = 2; vma;
vma = vma->vm_next) { vma = vma->vm_next) {
...@@ -2332,8 +2330,6 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -2332,8 +2330,6 @@ proc_map_files_readdir(struct file *filp, void *dirent, filldir_t filldir)
} }
} }
out_unlock:
unlock_trace(task);
out_put_task: out_put_task:
put_task_struct(task); put_task_struct(task);
out: out:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment