Messages in this thread | | | Date | Thu, 9 Jul 2009 22:07:23 -0500 | From | "Serge E. Hallyn" <> | Subject | Re: [PATCH 2/2] mm_for_maps: take ->cred_guard_mutex to fix the race with exec |
| |
Quoting Oleg Nesterov (oleg@redhat.com): > The problem is minor, but without ->cred_guard_mutex held we can race > with exec() and get the new ->mm but check old creds. > > Now we do not need to re-check task->mm after ptrace_may_access(), it > can't be changed to the new mm under us. > > Strictly speaking, this also fixes another very minor problem. Unless > security check fails or the task exits mm_for_maps() should never > return NULL, the caller should get either old or new ->mm. > > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
> --- > > fs/proc/base.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > --- WAIT/fs/proc/base.c~2_CRED_MUTEX 2009-07-10 02:05:14.000000000 +0200 > +++ WAIT/fs/proc/base.c 2009-07-10 03:23:01.000000000 +0200 > @@ -234,19 +234,19 @@ static int check_mem_permission(struct t > > struct mm_struct *mm_for_maps(struct task_struct *task) > { > - struct mm_struct *mm = get_task_mm(task); > + struct mm_struct *mm; > > - if (mm && mm != current->mm) { > - /* > - * task->mm can be changed before security check, > - * in that case we must notice the change after. > - */ > - if (!ptrace_may_access(task, PTRACE_MODE_READ) || > - mm != task->mm) { > - mmput(mm); > - mm = NULL; > - } > + if (mutex_lock_killable(&task->cred_guard_mutex)) > + return NULL; > + > + mm = get_task_mm(task); > + if (mm && mm != current->mm && > + !ptrace_may_access(task, PTRACE_MODE_READ)) { > + mmput(mm); > + mm = NULL; > } > + mutex_unlock(&task->cred_guard_mutex); > + > return mm; > } >
| |