Messages in this thread | | | Date | Thu, 29 Jul 2010 11:38:37 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH] cred - synchronize rcu before releasing cred |
| |
On Wed, Jul 28, 2010 at 04:46:27PM +0200, Jiri Olsa wrote: > On Wed, Jul 28, 2010 at 02:17:27PM +0100, David Howells wrote: > > > > The attached patch should suffice to fix get_task_cred(), and should render > > Jiri's patch unnecessary. > > > > David > > --- > > From: David Howells <dhowells@redhat.com> > > Subject: [PATCH] CRED: Move get_task_cred() out of line and make it use atomic_inc_not_zero() > > > > It's possible for get_task_cred() as it currently stands to 'corrupt' a set of > > credentials by incrementing their usage count after their replacement by the > > task being accessed. > > > > What happens is that get_task_cred() engages the RCU read lock, accesses the cred > > > > > > TASK_1 TASK_2 RCU_CLEANER > > -->get_task_cred(TASK_2) > > rcu_read_lock() > > __cred = __task_cred(TASK_2) > > -->commit_creds() > > old_cred = TASK_2->real_cred > > TASK_2->real_cred = ... > > put_cred(old_cred) > > call_rcu(old_cred) > > [__cred->usage == 0] > > get_cred(__cred) > > [__cred->usage == 1] > > rcu_read_unlock() > > -->put_cred_rcu() > > [__cred->usage == 1] > > panic() > > > > However, since a tasks credentials are generally not changed very often, we can > > reasonably make use of a loop involving reading the creds pointer and using > > atomic_inc_not_zero() to attempt to increment it if it hasn't already hit zero. > > > > If successful, we can safely return the credentials in the knowledge that, even > > if the task we're accessing has released them, they haven't gone to the RCU > > cleanup code. > > looks ok, I changed the task_state to use this and I'm running the > bug reproducer... so far so good ;)
the test did not hit the issue, so we are good probably
thanks, jirka
| |