login
Header Space

 
 

Re: [PATCH] kill_something_info: don't take tasklist_lock for pid==-1 case

Score:
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Oleg Nesterov <oleg@...>
Cc: <linux-kernel@...>, Eric W. Biederman <ebiederm@...>, Roland McGrath <roland@...>, Andrew Morton <akpm@...>
Date: Tuesday, May 20, 2008 - 9:48 pm

Oleg Nesterov wrote:

Sorry for late reply and thank you for your comment. I understood the
mechanism that kill(-1, SIGKILL) can miss the tasks forked by init
(and the thread group of the current process, because we don't also
send the signal to them). If kill(-1, SIGKILL) finish before the
forking init process does list_add_tail_rcu(p->tasks) in
copy_process(), the process forked by init appears on the ->tasks list
after that.  Is that right?

If so, I think this problem can happen without my patch.
Because even if kill(-1, SIGKILL) take read_lock(&tasklist_lock) in
kill_something_info(), it can finish before init process take
write_lock(&tasklist_lock) in copy_process(). So the forked process
appears after that, too.

Now, I noticed the important problem. I found the tasklist lock in
kill_something_info() can cause stall when some processes execute
kill(-1,SIGCONT) concurrently. It can happen even if a system has
only 4 CPUs (and even if a user is not privileged (not root)).  This is
because the writer cannot take the tasklist lock when a lot of readers
exist and keep holding it.

This allows a local DoS. So we have to avoid that stall. The
conversion from the tasklist lock to rcu_read_lock() can solve this
problem. I think my patch doesn't make the new problem because the
problem that kill can miss the tasks have originally occurred without
my one. If there is no problem, could you ack it?

Thanks,
-Atsushi Tsuji

--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [PATCH] kill_something_info: don't take tasklist_lock fo..., Eric W. Biederman, (Tue May 20, 11:47 pm)
Re: [PATCH] kill_something_info: don't take tasklist_lock fo..., Eric W. Biederman, (Sat May 31, 7:55 pm)
Re: [PATCH] kill_something_info: don't take tasklist_lock fo..., Eric W. Biederman, (Wed May 28, 11:03 am)
Re: [PATCH] kill_something_info: don't take tasklist_lock fo..., Atsushi Tsuji, (Tue May 20, 9:48 pm)
Re: [PATCH] kill_something_info: don't take tasklist_lock fo..., Eric W. Biederman, (Tue May 20, 10:53 pm)
speck-geostationary