* [PATCH] KEYS: Add missing smp_rmb() primitives to the keyring search code
@ 2012-01-16 16:03 David Howells
2012-01-16 23:38 ` James Morris
0 siblings, 1 reply; 3+ messages in thread
From: David Howells @ 2012-01-16 16:03 UTC (permalink / raw)
To: jmorris; +Cc: keyrings, linux-security-module, linux-kernel, David Howells
Add missing smp_rmb() primitives to the keyring search code.
When keyring payloads are appended to without replacement (thus using up spare
slots in the key pointer array), an smp_wmb() is issued between the pointer
assignment and the increment of the key count (nkeys).
There should be corresponding read barriers between the read of nkeys and
dereferences of keys[n] when n is dependent on the value of nkeys.
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
security/keys/gc.c | 4 +++-
security/keys/keyring.c | 22 +++++++++++++++-------
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/security/keys/gc.c b/security/keys/gc.c
index bf4d8da..a42b455 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -145,7 +145,9 @@ static void key_gc_keyring(struct key *keyring, time_t limit)
if (!klist)
goto unlock_dont_gc;
- for (loop = klist->nkeys - 1; loop >= 0; loop--) {
+ loop = klist->nkeys;
+ smp_rmb();
+ for (loop--; loop >= 0; loop--) {
key = klist->keys[loop];
if (test_bit(KEY_FLAG_DEAD, &key->flags) ||
(key->expiry > 0 && key->expiry <= limit))
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 37a7f3b..d605f75 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -319,7 +319,7 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref,
struct key *keyring, *key;
key_ref_t key_ref;
long err;
- int sp, kix;
+ int sp, nkeys, kix;
keyring = key_ref_to_ptr(keyring_ref);
possessed = is_key_possessed(keyring_ref);
@@ -380,7 +380,9 @@ descend:
goto not_this_keyring;
/* iterate through the keys in this keyring first */
- for (kix = 0; kix < keylist->nkeys; kix++) {
+ nkeys = keylist->nkeys;
+ smp_rmb();
+ for (kix = 0; kix < nkeys; kix++) {
key = keylist->keys[kix];
kflags = key->flags;
@@ -421,7 +423,9 @@ descend:
/* search through the keyrings nested in this one */
kix = 0;
ascend:
- for (; kix < keylist->nkeys; kix++) {
+ nkeys = keylist->nkeys;
+ smp_rmb();
+ for (; kix < nkeys; kix++) {
key = keylist->keys[kix];
if (key->type != &key_type_keyring)
continue;
@@ -515,7 +519,7 @@ key_ref_t __keyring_search_one(key_ref_t keyring_ref,
struct keyring_list *klist;
unsigned long possessed;
struct key *keyring, *key;
- int loop;
+ int nkeys, loop;
keyring = key_ref_to_ptr(keyring_ref);
possessed = is_key_possessed(keyring_ref);
@@ -524,7 +528,9 @@ key_ref_t __keyring_search_one(key_ref_t keyring_ref,
klist = rcu_dereference(keyring->payload.subscriptions);
if (klist) {
- for (loop = 0; loop < klist->nkeys; loop++) {
+ nkeys = klist->nkeys;
+ smp_rmb();
+ for (loop = 0; loop < nkeys ; loop++) {
key = klist->keys[loop];
if (key->type == ktype &&
@@ -622,7 +628,7 @@ static int keyring_detect_cycle(struct key *A, struct key *B)
struct keyring_list *keylist;
struct key *subtree, *key;
- int sp, kix, ret;
+ int sp, nkeys, kix, ret;
rcu_read_lock();
@@ -645,7 +651,9 @@ descend:
ascend:
/* iterate through the remaining keys in this keyring */
- for (; kix < keylist->nkeys; kix++) {
+ nkeys = keylist->nkeys;
+ smp_rmb();
+ for (; kix < nkeys; kix++) {
key = keylist->keys[kix];
if (key == A)
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] KEYS: Add missing smp_rmb() primitives to the keyring search code
2012-01-16 16:03 [PATCH] KEYS: Add missing smp_rmb() primitives to the keyring search code David Howells
@ 2012-01-16 23:38 ` James Morris
2012-01-17 20:40 ` David Howells
0 siblings, 1 reply; 3+ messages in thread
From: James Morris @ 2012-01-16 23:38 UTC (permalink / raw)
To: David Howells; +Cc: keyrings, linux-security-module, linux-kernel
On Mon, 16 Jan 2012, David Howells wrote:
> Add missing smp_rmb() primitives to the keyring search code.
>
Does this need to go into 3.3 and stable?
> When keyring payloads are appended to without replacement (thus using up spare
> slots in the key pointer array), an smp_wmb() is issued between the pointer
> assignment and the increment of the key count (nkeys).
>
> There should be corresponding read barriers between the read of nkeys and
> dereferences of keys[n] when n is dependent on the value of nkeys.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>
> security/keys/gc.c | 4 +++-
> security/keys/keyring.c | 22 +++++++++++++++-------
> 2 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/security/keys/gc.c b/security/keys/gc.c
> index bf4d8da..a42b455 100644
> --- a/security/keys/gc.c
> +++ b/security/keys/gc.c
> @@ -145,7 +145,9 @@ static void key_gc_keyring(struct key *keyring, time_t limit)
> if (!klist)
> goto unlock_dont_gc;
>
> - for (loop = klist->nkeys - 1; loop >= 0; loop--) {
> + loop = klist->nkeys;
> + smp_rmb();
> + for (loop--; loop >= 0; loop--) {
> key = klist->keys[loop];
> if (test_bit(KEY_FLAG_DEAD, &key->flags) ||
> (key->expiry > 0 && key->expiry <= limit))
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 37a7f3b..d605f75 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -319,7 +319,7 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref,
> struct key *keyring, *key;
> key_ref_t key_ref;
> long err;
> - int sp, kix;
> + int sp, nkeys, kix;
>
> keyring = key_ref_to_ptr(keyring_ref);
> possessed = is_key_possessed(keyring_ref);
> @@ -380,7 +380,9 @@ descend:
> goto not_this_keyring;
>
> /* iterate through the keys in this keyring first */
> - for (kix = 0; kix < keylist->nkeys; kix++) {
> + nkeys = keylist->nkeys;
> + smp_rmb();
> + for (kix = 0; kix < nkeys; kix++) {
> key = keylist->keys[kix];
> kflags = key->flags;
>
> @@ -421,7 +423,9 @@ descend:
> /* search through the keyrings nested in this one */
> kix = 0;
> ascend:
> - for (; kix < keylist->nkeys; kix++) {
> + nkeys = keylist->nkeys;
> + smp_rmb();
> + for (; kix < nkeys; kix++) {
> key = keylist->keys[kix];
> if (key->type != &key_type_keyring)
> continue;
> @@ -515,7 +519,7 @@ key_ref_t __keyring_search_one(key_ref_t keyring_ref,
> struct keyring_list *klist;
> unsigned long possessed;
> struct key *keyring, *key;
> - int loop;
> + int nkeys, loop;
>
> keyring = key_ref_to_ptr(keyring_ref);
> possessed = is_key_possessed(keyring_ref);
> @@ -524,7 +528,9 @@ key_ref_t __keyring_search_one(key_ref_t keyring_ref,
>
> klist = rcu_dereference(keyring->payload.subscriptions);
> if (klist) {
> - for (loop = 0; loop < klist->nkeys; loop++) {
> + nkeys = klist->nkeys;
> + smp_rmb();
> + for (loop = 0; loop < nkeys ; loop++) {
> key = klist->keys[loop];
>
> if (key->type == ktype &&
> @@ -622,7 +628,7 @@ static int keyring_detect_cycle(struct key *A, struct key *B)
>
> struct keyring_list *keylist;
> struct key *subtree, *key;
> - int sp, kix, ret;
> + int sp, nkeys, kix, ret;
>
> rcu_read_lock();
>
> @@ -645,7 +651,9 @@ descend:
>
> ascend:
> /* iterate through the remaining keys in this keyring */
> - for (; kix < keylist->nkeys; kix++) {
> + nkeys = keylist->nkeys;
> + smp_rmb();
> + for (; kix < nkeys; kix++) {
> key = keylist->keys[kix];
>
> if (key == A)
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-01-17 20:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-16 16:03 [PATCH] KEYS: Add missing smp_rmb() primitives to the keyring search code David Howells
2012-01-16 23:38 ` James Morris
2012-01-17 20:40 ` David Howells
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox