public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: "Serge E. Hallyn" <serue@us.ibm.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: dhowells@redhat.com, torvalds@osdl.org,
	akpm@linux-foundation.org, keyrings@linux-nfs.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/7] KEYS: Use RCU dereference wrappers in keyring key type code
Date: Tue, 04 May 2010 14:00:51 +0100	[thread overview]
Message-ID: <31577.1272978051@redhat.com> (raw)
In-Reply-To: <20100503223023.GC6968@us.ibm.com>

Serge E. Hallyn <serue@us.ibm.com> wrote:

> <shrug> does this mean that the
>         klist = rcu_dereference_check(keyring->payload.subscriptions,
> 				      lockdep_is_held(&key_serial_lock));
> in security/keys/gc.c:key_gc_keyring() should become a
> rcu_dereference_protected() to avoid the rcu_dereference_raw() and for
> consistency?

Well spotted.  That's incorrect modification by commit
e7b0a61b7929632d36cf052d9e2820ef0a9c1bfe.

key_serial_lock is not a guarantor of the current keyring subscriptions not
changing as we read it.  We need to hold either the RCU read lock (so that the
what the pointer currently points to isn't trashed) or the keyring semaphore
(so that the keyring isn't changed under us).

The real error is not holding the RCU read lock (I'd assumed that the fact it
was holding a spinlock implied this - which I now know to be wrong).

David

  reply	other threads:[~2010-05-04 13:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-30 13:32 [PATCH 1/7] KEYS: Fix an RCU warning in the reading of user keys David Howells
2010-04-30 13:32 ` [PATCH 2/7] KEYS: find_keyring_by_name() can gain access to a freed keyring David Howells
2010-05-03 22:14   ` Serge E. Hallyn
2010-04-30 13:32 ` [PATCH 3/7] KEYS: Use RCU dereference wrappers in keyring key type code David Howells
2010-05-03 22:30   ` Serge E. Hallyn
2010-05-04 13:00     ` David Howells [this message]
2010-04-30 13:32 ` [PATCH 4/7] KEYS: call_sbin_request_key() must write lock keyrings before modifying them David Howells
2010-04-30 13:32 ` [PATCH 5/7] KEYS: keyring_serialise_link_sem is only needed for keyring->keyring links David Howells
2010-04-30 13:32 ` [PATCH 6/7] KEYS: Better handling of errors from construct_alloc_key() David Howells
2010-04-30 13:32 ` [PATCH 7/7] KEYS: Do preallocation for __key_link() David Howells
2010-05-03 22:04 ` [PATCH 1/7] KEYS: Fix an RCU warning in the reading of user keys Serge E. Hallyn
2010-05-04 12:48   ` David Howells
2010-05-06  2:45 ` James Morris
2010-05-06 10:38   ` David Howells
2010-05-06 12:25     ` James Morris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=31577.1272978051@redhat.com \
    --to=dhowells@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=keyrings@linux-nfs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=serue@us.ibm.com \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox