oom: __task_cred() need rcu_read_lock()

oom: __task_cred() need rcu_read_lock()

[KOSAKI Motohiro]

dump_tasks() can call __task_cred() safely because we are holding
tasklist_lock. but rcu lock validator don't have enough knowledge and
it makes following annoying warning.

Then, this patch change to call rcu_read_lock() explicitly.


	===================================================
	[ INFO: suspicious rcu_dereference_check() usage. ]
	---------------------------------------------------
	mm/oom_kill.c:410 invoked rcu_dereference_check() without protection!

	other info that might help us debug this:

	rcu_scheduler_active = 1, debug_locks = 1
	4 locks held by kworker/1:2/651:
	 #0:  (events){+.+.+.}, at: [<ffffffff8106aae7>]
	process_one_work+0x137/0x4a0
	 #1:  (moom_work){+.+...}, at: [<ffffffff8106aae7>]
	process_one_work+0x137/0x4a0
	 #2:  (tasklist_lock){.+.+..}, at: [<ffffffff810fafd4>]
	out_of_memory+0x164/0x3f0
	 #3:  (&(&p->alloc_lock)->rlock){+.+...}, at: [<ffffffff810fa48e>]
	find_lock_task_mm+0x2e/0x70

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/oom_kill.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index c48c5ef..57c05f7 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -371,11 +371,13 @@ static void dump_tasks(const struct mem_cgroup *mem)
 			continue;
 		}
 
+		rcu_read_lock();
 		pr_info("[%5d] %5d %5d %8lu %8lu %3u     %3d         %5d %s\n",
 			task->pid, __task_cred(task)->uid, task->tgid,
 			task->mm->total_vm, get_mm_rss(task->mm),
 			task_cpu(task), task->signal->oom_adj,
 			task->signal->oom_score_adj, task->comm);
+		rcu_read_unlock();
 		task_unlock(task);
 	}
 }
-- 
1.6.5.2



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: oom: __task_cred() need rcu_read_lock()

[David Howells]

KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> dump_tasks() can call __task_cred() safely because we are holding
> tasklist_lock. but rcu lock validator don't have enough knowledge and
> it makes following annoying warning.

No, it can't.  The tasklist_lock is not protection against the creds changing
on another CPU.

David

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: oom: __task_cred() need rcu_read_lock()

[KOSAKI Motohiro]

Hi

> KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
> > dump_tasks() can call __task_cred() safely because we are holding
> > tasklist_lock. but rcu lock validator don't have enough knowledge and
> > it makes following annoying warning.
> 
> No, it can't.  The tasklist_lock is not protection against the creds changing
> on another CPU.

Thank you for correction.

I suppose you mean I missed CONFIG_TREE_PREEMPT_RCU, right?
As far as my grepping, other rcu implementation and spinlock use 
preempt_disable(). In other word, Can I assume usual distro user 
don't hit this issue?

Thanks.



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: oom: __task_cred() need rcu_read_lock()

[David Howells]

KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> > KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> > 
> > > dump_tasks() can call __task_cred() safely because we are holding
> > > tasklist_lock. but rcu lock validator don't have enough knowledge and
> > > it makes following annoying warning.
> > 
> > No, it can't.  The tasklist_lock is not protection against the creds
> > changing on another CPU.
> 
> Thank you for correction.
> 
> I suppose you mean I missed CONFIG_TREE_PREEMPT_RCU, right?
> As far as my grepping, other rcu implementation and spinlock use 
> preempt_disable(). In other word, Can I assume usual distro user 
> don't hit this issue?

No.  The paths by which a process changes its credentials don't normally take
tasklist_lock, so holding tasklist_lock doesn't prevent the process you're
looking at from replacing its cred and discarding the ones you're looking at.

Further, unless you're holding the RCU read lock, there's nothing theoretically
stopping the system from deleting the discarded credentials.

David

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>