linux-kernel - Re: [PATCH] cred - synchronize rcu before releasing cred

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]

Message-ID: <24865.1280249187@redhat.com>
Date:	Tue, 27 Jul 2010 17:46:27 +0100
From:	David Howells <dhowells@...hat.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	dhowells@...hat.com, Jiri Olsa <jolsa@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Eric Dumazet <eric.dumazet@...il.com>,
	linux-kernel@...r.kernel.org,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH] cred - synchronize rcu before releasing cred

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

> The whole patch seems to be based on "nobody can ever use
> get_cred/put_cred, because concurrent use will then trigger the
> BUG_ON() in __put_cred()".

That's not the problem.

The problem is that task_state() accesses the target task's credentials whilst
only holding the RCU read lock.  That means that the existence of the cred
struct so accessed can only be guaranteed up to the point that the RCU read
lock is released.

What we shouldn't do is increment the usage count on the credentials because
we're not holding a lock that will prevent the target task reducing the
refcount on those credentials to zero between us reading the cred pointer and
incrementing the count.

If we want to increment the usage count on the credentials, we need to prevent
the target task from modifying its own credentials whilst we do it.  Currently,
we can't do that as, taking an example, sys_setuid() doesn't hold hold any sort
of lock.  We would have to add a spinlock or something like that for
commit_creds() to take.

What we have to do instead is grab any values we want from the cred struct
before releasing the RCU read lock.  The moment we drop the lock, the cred
struct may cease to exist, even if we did increment its count.

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