Date: Fri, 10 Sep 2010 19:24:57 +0200 From: Oleg Nesterov <oleg@...hat.com> To: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com> Cc: Roland McGrath <roland@...hat.com>, Linus Torvalds <torvalds@...ux-foundation.org>, Andrew Morton <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org, oss-security@...ts.openwall.com, Solar Designer <solar@...nwall.com>, Kees Cook <kees.cook@...onical.com>, Al Viro <viro@...iv.linux.org.uk>, Neil Horman <nhorman@...driver.com>, linux-fsdevel@...r.kernel.org, pageexec@...email.hu, "Brad Spengler <spender@...ecurity.net>, Eugene Teo" <eugene@...hat.com>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com> 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@...hat.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.
Powered by blists - more mailing lists
Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.