Re: [RFC] install_session_keyring

[David Howells <dhowells@redhat.com>]

Suzanne Wood <suzannew@cs.pdx.edu> wrote:

> In a study of the control flow graph dumps to check that 
> an rcu_assign_pointer() with a given argument type has 
> preceded a call to rcu_dereference(), I've come across 
> install_session_keyring() of security/keys/process_keys.c.  
> We note that although no rcu_read_lock() is in place 
> locally or in the function's kernel callers, siglock 
> likely addresses that.  While the rcu_dereference() would 
> indicate a desire for 'old' to persist, synchronize_rcu() 
> is called prior to key_put(old) which "disposes of 
> reference to a key."  The order of events with a use of 
> the copy of the pointer following synchronize_rcu() is 
> what I question.

Are you simply suggesting that the rcu_dereference() in:

	/* install the keyring */
       	spin_lock_irqsave(&tsk->sighand->siglock, flags);
       	old = rcu_dereference(tsk->signal->session_keyring);
       	rcu_assign_pointer(tsk->signal->session_keyring, keyring);
       	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);

is unnecessary?

If so, I think you are right since the pointer is only changed with the
siglock held[*], and so modify/modify conflict isn't a problem and doesn't
need memory barriers.

[*] Apart from during the exit() cleanup, when I don't think this should be a
problem anyway.

David