Messages in this thread Patch in this message | | | Date | Wed, 28 Sep 2011 22:20:20 +0200 | From | Oleg Nesterov <> | Subject | Q: proc: hold cred_guard_mutex in check_mem_permission() |
| |
Another change we probably need to backport, 18f661bc "proc: hold cred_guard_mutex in check_mem_permission()".
From the changelog:
Avoid a potential race when task exec's and we get a new ->mm but check against the old credentials in ptrace_may_access().
Could you please explain? How can we race with exec?
This task is either current, or it is TASK_TRACED and we are the tracer. In the latter case nobody can resume it except SIGKILL. And the killed task obviously can't exec.
Afaics, all we need is: we should read task->mm after the task_is_traced() check, we do not need the mutex.
IOW, what do you think about the patch below? I have no idea how can I test it (and it wasn't even applied/compiled).
Also, I'd appreciate if you can explain the PTRACE_MODE_ATTACH check. Again, we are already the tracer.
Oleg.
--- x/fs/proc/base.c +++ x/fs/proc/base.c @@ -194,38 +194,31 @@ static int proc_root_link(struct inode * return result; } -static struct mm_struct *__check_mem_permission(struct task_struct *task) +static int __check_mem_permission(struct task_struct *task) { - struct mm_struct *mm; - - mm = get_task_mm(task); - if (!mm) - return ERR_PTR(-EINVAL); - /* * A task can always look at itself, in case it chooses * to use system calls instead of load instructions. */ if (task == current) - return mm; + return 0; /* * If current is actively ptrace'ing, and would also be * permitted to freshly attach with ptrace now, permit it. */ - if (task_is_stopped_or_traced(task)) { + if (task_is_traced(task)) { int match; rcu_read_lock(); match = (ptrace_parent(task) == current); rcu_read_unlock(); if (match && ptrace_may_access(task, PTRACE_MODE_ATTACH)) - return mm; + return 0; } /* * No one else is allowed. */ - mmput(mm); return ERR_PTR(-EPERM); } @@ -238,18 +231,11 @@ static struct mm_struct *check_mem_permi struct mm_struct *mm; int err; - /* - * Avoid racing if task exec's as we might get a new mm but validate - * against old credentials. - */ - err = mutex_lock_killable(&task->signal->cred_guard_mutex); + err = __check_mem_permission(task); if (err) return ERR_PTR(err); - mm = __check_mem_permission(task); - mutex_unlock(&task->signal->cred_guard_mutex); - - return mm; + return get_task_mm(task) ?: ERR_PTR(-EINVAL); } struct mm_struct *mm_for_maps(struct task_struct *task)
| |