Messages in this thread | | | Date | Thu, 12 Apr 2012 16:29:27 +0100 | From | Djalal Harouni <> | Subject | Re: [PATCH 5/6] proc: use task_access_lock() instead of ptrace_may_access() |
| |
Hi Cong,
On Thu, Apr 12, 2012 at 09:22:10PM +0800, Cong Wang wrote: > On 04/11/2012 01:59 PM, Cong Wang wrote: > > There are several places in fs/proc/base.c still use ptrace_may_access() > > directly to check the permission, actually this just gets a snapshot of > > the permission, nothing prevents the target task from raising the priviledges > > itself, it is better to use task_access_lock() for these places, to hold > > the priviledges. > > > Hi, Andrew, > > Please drop this patch, it introduces a deadlock when execve() a > /proc/<pid>/exec file, and it is not a big improvement nor fixes any > bugs, so let's just drop this one. I was going to ask about this since it seems that abusing lock_trace() or task_access_lock() can cause problems.
Please see commit 5e442a493fc59f which reverts commit aa6afca5bcaba8101f that tries to protect /proc/PID/fd** files
IMHO A solution for some of the simple /proc/<pid>/* files is to use mm_access() check just after gathering data and before returning it to userspace.
So IMO the original code of proc_pid_wchan() was correct, since that data is not copied to userspace directly, and we can avoid the mm_access() and the task->signal->cred_guard_mutex lock since we do not race against them, we have already grabbed the 'wchan', a simple ptrace_may_access() check will do the job.
(I guess there is a window against another execve and ptrace_may_access() but that returned data is not useful anymore, is it ?).
For others I don't know what would be the best solution.
Thanks.
> Thanks! > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
-- tixxdz http://opendz.org
| |