linux-kernel - Re: [PATCH 2/2] CRED: Fix __task_cred()'s lockdep check and banner comment

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]

Message-ID: <m1lj8k21uj.fsf@fess.ebiederm.org>
Date:	Thu, 05 Aug 2010 14:20:36 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	David Howells <dhowells@...hat.com>,
	Oleg Nesterov <oleg@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	paulmck@...ux.vnet.ibm.com, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org, Jiri Olsa <jolsa@...hat.com>
Subject: Re: [PATCH 2/2] CRED: Fix __task_cred()'s lockdep check and banner  comment

Linus Torvalds <torvalds@...ux-foundation.org> writes:

> On Thu, Aug 5, 2010 at 1:13 PM, Eric W. Biederman <ebiederm@...ssion.com> wrote:
>>
>> I think it is totally reasonable to add a per pid lock,
>> that would protect the pid->task[...] hlist.  That would make
>> things clearer and finer grained without a lot of effort.  Just
>> a little more struct pid bloat, and a little extra care in fork,
>> when we add to those lists.
>
> Hmm. Have you taken a look at Nick Piggin's VFS scalability patches?
> They introduce this "RCU-safe hash chain lock", where each hashchain
> has a lock-bit in the low bit. I wonder if that would be the right
> thing to use?

Interesting.  I haven't looked but a lock bit in the low bit of the
hlist head in struct pid would not add any space, and would not add
any extra cache line bounces.  So that would just be a matter of
adding the extra locks and getting the lock ordering correct.

>> Even with the per-pgrp lock we still need a lock on the global process
>> list for the kill -KILL -1 case.  Which suggests that tasklist_lock is
>> still needed for part of kill_something_info.
>
> Well, that -1 case is special anyway. The fact that we might want to
> use the tasklist_lock there is not very relevant, I think. That is
> _not_ a hotpath, really (at least not under any relevant loads, I'm
> sure you could make a silly benchmark of "kill(-1,0)").

I expect even signal deliver to process groups is relatively rare.

The interesting question is can we kill the tasklist_lock and/or the
tasklist.   A quick grep shows that we have maybe 100 users of the tasklist
in the entire kernel.  Poking a little deeper it looks like man of those
are connected to scheduling and are uses that today take the tasklist_lock
but would be fine with the rcu_read_lock().

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/