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

[David Howells]

Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/