KEYS: Fix an RCU warning in the reading of user keys
in [Kernel]
|
Prev: [PATCH tip/core/urgent 2/5] rcu: fix RCU lockdep splat on freezer_fork path
Next: What's the staging review and acceptance process?
From: Paul E. McKenney on 21 Apr 2010 16:20 On Wed, Apr 21, 2010 at 05:36:35PM +0100, David Howells wrote: > Fix an RCU warning in the reading of user keys: > > =================================================== > [ INFO: suspicious rcu_dereference_check() usage. ] > --------------------------------------------------- > security/keys/user_defined.c:202 invoked rcu_dereference_check() without protection! > > other info that might help us debug this: > > > rcu_scheduler_active = 1, debug_locks = 0 > 1 lock held by keyctl/3637: > #0: (&key->sem){+++++.}, at: [<ffffffff811a80ae>] keyctl_read_key+0x9c/0xcf > > stack backtrace: > Pid: 3637, comm: keyctl Not tainted 2.6.34-rc5-cachefs #18 > Call Trace: > [<ffffffff81051f6c>] lockdep_rcu_dereference+0xaa/0xb2 > [<ffffffff811aa55f>] user_read+0x47/0x91 > [<ffffffff811a80be>] keyctl_read_key+0xac/0xcf > [<ffffffff811a8a06>] sys_keyctl+0x75/0xb7 > [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b Queued for 2.6.34, thank you David! Thanx, Paul > Signed-off-by: David Howells <dhowells(a)redhat.com> > --- > > security/keys/user_defined.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c > index 7c687d5..e9aa079 100644 > --- a/security/keys/user_defined.c > +++ b/security/keys/user_defined.c > @@ -199,7 +199,8 @@ long user_read(const struct key *key, char __user *buffer, size_t buflen) > struct user_key_payload *upayload; > long ret; > > - upayload = rcu_dereference(key->payload.data); > + upayload = rcu_dereference_protected( > + key->payload.data, rwsem_is_locked(&((struct key *)key)->sem)); > ret = upayload->datalen; > > /* we can return the data as is */ > -- 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: David Howells on 4 May 2010 08:50 Serge E. Hallyn <serue(a)us.ibm.com> wrote: > heck it also serves to document it a bit, as looking at the fn > itself it's not clear that it is called under key->sem. Give or take the banner comment on user_read() where it states this explicitly: /* * read the key data * - the key's semaphore is read-locked */ long user_read(const struct key *key, char __user *buffer, size_t buflen) and Documentation/keys.txt where it also states this explicitly: (*) long (*read)(const struct key *key, char __user *buffer, size_t buflen); ... This method will be called with the key's semaphore read-locked. This will prevent the key's payload changing. It is not necessary to use RCU locking when accessing the key's payload. It is safe to sleep in this method, such as might happen when the userspace buffer is accessed. :-) David David -- 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: David Howells on 6 May 2010 06:50
James Morris <jmorris(a)namei.org> wrote: > The final patch does not apply to my tree -- please rebase it. Your tree is not up to date with respect to Linus's: you're missing the three patches he pulled in yesterday. David -- 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/ |