Prev: hmc6352: Add driver for the HMC6352 compass
Next: Eliminate duplicated timer code
From: Jiri Olsa on 29 Jul 2010 05:40
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(a)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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Paul E. McKenney on 30 Jul 2010 17:40
On Thu, Jul 29, 2010 at 09:34:20AM +0100, David Howells wrote:
> Paul E. McKenney <paulmck(a)linux.vnet.ibm.com> wrote:
>
> > It is perfectly legal for an RCU callback to invoke call_rcu(). However,
> > this should be used -only- to wait for RCU readers. If there are no
> > RCU readers, the callback might be re-invoked in very short order,
> > expecially on UP systems.
> >
> > Or am I misunderstanding what you mean by "require call_rcu() to be
> > able to cope iwth requeueing"?
>
> I mean for call_rcu() to be called on an object that's already been
> call_rcu()'d but not yet processed.

That would indeed be very bad!!!

> For example if struct cred gets its usage count reduced to 0, __put_cred()
> will call_rcu() it, but what happens if someone comes along and resurrects it
> by increasing its usage count again? And what happens if the usage count is
> reduced back to zero and __put_cred() calls call_rcu() again before
> put_cred_rcu() has a chance to run?

Doing this would mess up RCU's internal data structures. Mathieu
Desnoyers's recent debug changes (DEBUG_OBJECTS_RCU_HEAD) would catch
this sort of error.

Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  | 
Pages: 1 2 3 4 5
Prev: hmc6352: Add driver for the HMC6352 compass
Next: Eliminate duplicated timer code