linux-kernel - Re: [PATCH] proc: Fix proc_tid_readdir so it handles the weird cases.

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] [day] [month] [year] [list]

Date:	Wed, 18 Mar 2009 20:40:20 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Alexey Dobriyan <adobriyan@...il.com>
Cc:	linux-kernel@...r.kernel.org, torvalds@...l.org,
	Zhang Le <r0bertz@...too.org>,
	Al Viro <viro@...IV.linux.org.uk>,
	Oleg Nesterov <oleg@...sign.ru>
Subject: Re: [PATCH] proc:  Fix proc_tid_readdir so it handles the weird cases.

Louis Rilling <Louis.Rilling@...labs.com> writes:

> On 18/03/09  6:39 -0700, Eric W. Biederman wrote:
>> 
>> Fix the logic in proc_task_readdir so that we provide the
>> guarantee that in the sequence:
>> opendir
>> readdir
>> readdir
>> ....
>> readdir
>> closedir
>> If a thread exists from the beginning until the end we are
>> guaranteed to see it, even if some threads are created or
>> worse are destroyed during the readdir.
>> 
>> This guarantee is provided by reusing the same logic used
>> by proc_pid_readdir for the root directory of /proc.  The
>> pid is encoded into the offset, and we walk the pid bitmap
>> of all pids in the system, and test each of them to see if
>> it belongs to the specified thread group.
>> 
>> If we seek to a bad location or if the task we say last
>> exits it is ok because we can numerically find the next
>> task we would have returned.
>> 
>> This is slower that the previous version, but it should
>> not be too slow as this techinique is already use for
>> the root directory of /proc without problems.
>
> This makes 'ps -T x' an O(n^2) thing, compared to O(n) before the patch, right?
> It would be good to, at least, check for thread_group_empty().

Sort of.  It is more like it adds a constant to the traversing each thread
directory.  With everything in cache I don't expect it to be a big constant,
as I have reports that when I changed the pid directory it sped things up.

We definitely don't hold any locks so only the process doing the traversal will pay
the price.

Since this is actually a correctness issue I'm willing to pay a little more to get
the right result.

Eric

--
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/