Messages in this thread | | | Date | Fri, 10 Sep 2010 19:24:57 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH] move cred_guard_mutex from task_struct to signal_struct |
| |
On 09/10, KOSAKI Motohiro wrote: > > 1) moving cread_guard_mutex itself > - no increase execve overhead > -> very good > - it also prevent parallel ptrace
No, it doesn't. Only PTRACE_ATTACH needs this mutex, and as Roland pointed out it also needs write_lock(tasklist) which is worse. So this change doesn't make any practical harm for ptrace.
> 2) move in_exec_mm to signal_struct too > -> very hard. oom-killer can use very few lock because it's called > from various place. now both ->mm and ->in_exec_mm are protected > task_lock() and it help to avoid messy.
Yes. But, if ->in_exec_mm is only used by oom_badness(), then I think you can use task_lock(tsk->group_leader). oom_badness() needs tasklist anyway, this means it can't race with de_thread() changing the leader. But up to you.
Another very minor nit (but again, up to you). Perhaps exec_mmap() could clear ->in_exec_mm (in task_struct or signal_struct, this doesnt matter), it takes task_lock(current) anyway (and at this point current is always the group leader).
> Let's move ->cred_guard_mutex from task_struct to signal_struct. It > naturally prevent multiple-threads-inside-exec.
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
This is very minor, but perhaps you can also fix a couple of comments which mention task->cred_guard_mutex,
fs/exec.c:1109 the caller must hold current->cred_guard_mutex kernel/cred.c:328 The caller must hold current->cred_guard_mutex include/linux/tracehook.h:153 @task->cred_guard_mutex
Oleg.
| |