linux-kernel - Re: [PATCH 2/2] mm/linux-next: Fix rcu locking in vm_is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 8 Mar 2012 03:41:26 +0530
From:	Siddhesh Poyarekar <siddhesh.poyarekar@...il.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Mark Salter <msalter@...hat.com>,
	linux-next <linux-next@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Paul Gortmaker <paul.gortmaker@...driver.com>
Subject: Re: [PATCH 2/2] mm/linux-next: Fix rcu locking in vm_is_stack

On Wed, Mar 7, 2012 at 9:08 PM, Oleg Nesterov <oleg@...hat.com> wrote:
> rcu_read_lock() can not help without the additional checks. By the
> time you take it, task->thread_group->next can point to nowhere.

I thought I understood this the second time, but I think I haven't.

> Once again. You have the task_struct *task. It exits,
> but task->thread_group->next still points to another thread T. Now suppose
> that T exits too. But task->thread_group->next was not changed, it still
> points to T. RCU grace period passes, T is freed.

This is the point I haven't understood. From what I understand about
rcu, the rcu update will first update task->thread_group->next and
then reclaim the struct it pointed to and not the other way around. So
with:

>>               rcu_read_lock();
>> -             while_each_thread(task, t) {
>> +             t = list_first_entry_rcu(&task->thread_group,
>> +                                      struct task_struct, thread_group);

since I have the rcu_read_lock when I'm touching the rcu protected
list, if I get the old T from task->thread_group->next then I should
have a valid struct backing it too. If not, the T should have changed
_before_ the struct it points to is removed and hence should again
result in a valid reference.

I guess there is a corner case where the current task is released and
thread_group is rcu_list_del()'d. In that case too, before this
happens, the proc entry is removed and the task namespace is unmounted
from /proc. Also, the thread_group being deleted from list is merely
an update of references and we should get the next element of the list
when we get access to the lock and poke it (under an rcu_lock of
course). Is there something I am missing?

-- 
Siddhesh Poyarekar
http://siddhesh.in
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/