linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/9] KEYS: Allow special keyrings to be cleared
@ 2012-02-08 11:02 David Howells
  2012-02-08 11:03 ` [PATCH 2/9] keys: update the description with info about "logon" keys David Howells
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: David Howells @ 2012-02-08 11:02 UTC (permalink / raw)
  To: steved, jmorris; +Cc: keyrings, linux-nfs, linux-security-module, linux-kernel

The kernel contains some special internal keyrings, for instance the DNS
resolver keyring :

2a93faf1 I-----     1 perm 1f030000     0     0 keyring   .dns_resolver: empty

It would occasionally be useful to allow the contents of such keyrings to be
flushed by root (cache invalidation).

Allow a flag to be set on a keyring to mark that someone possessing the
sysadmin capability can clear the keyring, even without normal write access to
the keyring.

Set this flag on the special keyrings created by the DNS resolver, the NFS
identity mapper and the CIFS identity mapper.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Jeff Layton <jlayton@redhat.com>
Acked-by: Steve Dickson <steved@redhat.com>
---

 Documentation/networking/dns_resolver.txt |    4 ++++
 Documentation/security/keys.txt           |    4 ++++
 fs/cifs/cifsacl.c                         |    1 +
 fs/nfs/idmap.c                            |    1 +
 include/linux/key.h                       |    1 +
 net/dns_resolver/dns_key.c                |    1 +
 security/keys/keyctl.c                    |   15 ++++++++++++++-
 7 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/Documentation/networking/dns_resolver.txt b/Documentation/networking/dns_resolver.txt
index 7f531ad..d86adcd 100644
--- a/Documentation/networking/dns_resolver.txt
+++ b/Documentation/networking/dns_resolver.txt
@@ -102,6 +102,10 @@ implemented in the module can be called after doing:
      If _expiry is non-NULL, the expiry time (TTL) of the result will be
      returned also.
 
+The kernel maintains an internal keyring in which it caches looked up keys.
+This can be cleared by any process that has the CAP_SYS_ADMIN capability by
+the use of KEYCTL_KEYRING_CLEAR on the keyring ID.
+
 
 ===============================
 READING DNS KEYS FROM USERSPACE
diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 4d75931..713ec23 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -554,6 +554,10 @@ The keyctl syscall functions are:
      process must have write permission on the keyring, and it must be a
      keyring (or else error ENOTDIR will result).
 
+     This function can also be used to clear special kernel keyrings if they
+     are appropriately marked if the user has CAP_SYS_ADMIN capability.  The
+     DNS resolver cache keyring is an example of this.
+
 
  (*) Link a key into a keyring:
 
diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index c1b2544..3cc1b25 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -556,6 +556,7 @@ init_cifs_idmap(void)
 
 	/* instruct request_key() to use this special keyring as a cache for
 	 * the results it looks up */
+	set_bit(KEY_FLAG_ROOT_CAN_CLEAR, &keyring->flags);
 	cred->thread_keyring = keyring;
 	cred->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING;
 	root_cred = cred;
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 2c05f19..a1bbf77 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -198,6 +198,7 @@ int nfs_idmap_init(void)
 	if (ret < 0)
 		goto failed_put_key;
 
+	set_bit(KEY_FLAG_ROOT_CAN_CLEAR, &keyring->flags);
 	cred->thread_keyring = keyring;
 	cred->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING;
 	id_resolver_cache = cred;
diff --git a/include/linux/key.h b/include/linux/key.h
index 5253471..1600ebf 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -155,6 +155,7 @@ struct key {
 #define KEY_FLAG_IN_QUOTA	3	/* set if key consumes quota */
 #define KEY_FLAG_USER_CONSTRUCT	4	/* set if key is being constructed in userspace */
 #define KEY_FLAG_NEGATIVE	5	/* set if key is negative */
+#define KEY_FLAG_ROOT_CAN_CLEAR	6	/* set if key can be cleared by root without permission */
 
 	/* the description string
 	 * - this is used to match a key against search criteria
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index fa000d2..c73bba3 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -281,6 +281,7 @@ static int __init init_dns_resolver(void)
 
 	/* instruct request_key() to use this special keyring as a cache for
 	 * the results it looks up */
+	set_bit(KEY_FLAG_ROOT_CAN_CLEAR, &keyring->flags);
 	cred->thread_keyring = keyring;
 	cred->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING;
 	dns_resolver_cache = cred;
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 0b3f5d7..6523599 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -388,11 +388,24 @@ long keyctl_keyring_clear(key_serial_t ringid)
 	keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_WRITE);
 	if (IS_ERR(keyring_ref)) {
 		ret = PTR_ERR(keyring_ref);
+
+		/* Root is permitted to invalidate certain special keyrings */
+		if (capable(CAP_SYS_ADMIN)) {
+			keyring_ref = lookup_user_key(ringid, 0, 0);
+			if (IS_ERR(keyring_ref))
+				goto error;
+			if (test_bit(KEY_FLAG_ROOT_CAN_CLEAR,
+				     &key_ref_to_ptr(keyring_ref)->flags))
+				goto clear;
+			goto error_put;
+		}
+
 		goto error;
 	}
 
+clear:
 	ret = keyring_clear(key_ref_to_ptr(keyring_ref));
-
+error_put:
 	key_ref_put(keyring_ref);
 error:
 	return ret;


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/9] keys: update the description with info about "logon" keys
  2012-02-08 11:02 [PATCH 1/9] KEYS: Allow special keyrings to be cleared David Howells
@ 2012-02-08 11:03 ` David Howells
  2012-02-08 11:03 ` [PATCH 3/9] KEYS: Move the key config into security/keys/Kconfig David Howells
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2012-02-08 11:03 UTC (permalink / raw)
  To: steved, jmorris; +Cc: keyrings, linux-nfs, linux-security-module, linux-kernel

From: Jeff Layton <jlayton@redhat.com>

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 Documentation/security/keys.txt |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 713ec23..be3d229 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -123,7 +123,7 @@ KEY SERVICE OVERVIEW
 
 The key service provides a number of features besides keys:
 
- (*) The key service defines two special key types:
+ (*) The key service defines three special key types:
 
      (+) "keyring"
 
@@ -137,6 +137,19 @@ The key service provides a number of features besides keys:
 	 blobs of data. These can be created, updated and read by userspace,
 	 and aren't intended for use by kernel services.
 
+     (+) "logon"
+
+         Like a "user" key, a "logon" key has a payload that is an arbitrary
+         blob of data. It is intended as a place to store secrets that the
+         to which the kernel should have access but that should not be
+         accessable from userspace.
+
+         The description can be arbitrary, but must be prefixed with a non-zero
+         length string that describes the key "subclass". The subclass is
+         separated from the rest of the description by a ':'. "logon" keys can
+         be created and updated by userspace, but the payload is only readable
+         from kernel space.
+
  (*) Each process subscribes to three keyrings: a thread-specific keyring, a
      process-specific keyring, and a session-specific keyring.
 


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/9] KEYS: Move the key config into security/keys/Kconfig
  2012-02-08 11:02 [PATCH 1/9] KEYS: Allow special keyrings to be cleared David Howells
  2012-02-08 11:03 ` [PATCH 2/9] keys: update the description with info about "logon" keys David Howells
@ 2012-02-08 11:03 ` David Howells
  2012-02-08 11:03 ` [PATCH 4/9] KEYS: Reorganise keys Makefile David Howells
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2012-02-08 11:03 UTC (permalink / raw)
  To: steved, jmorris; +Cc: keyrings, linux-nfs, linux-security-module, linux-kernel

Move the key config into security/keys/Kconfig as there are going to be a lot
of key-related options.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
---

 security/Kconfig      |   68 +----------------------------------------------
 security/keys/Kconfig |   71 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 67 deletions(-)
 create mode 100644 security/keys/Kconfig

diff --git a/security/Kconfig b/security/Kconfig
index 51bd5a0..1c5a7a4 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -4,73 +4,7 @@
 
 menu "Security options"
 
-config KEYS
-	bool "Enable access key retention support"
-	help
-	  This option provides support for retaining authentication tokens and
-	  access keys in the kernel.
-
-	  It also includes provision of methods by which such keys might be
-	  associated with a process so that network filesystems, encryption
-	  support and the like can find them.
-
-	  Furthermore, a special type of key is available that acts as keyring:
-	  a searchable sequence of keys. Each process is equipped with access
-	  to five standard keyrings: UID-specific, GID-specific, session,
-	  process and thread.
-
-	  If you are unsure as to whether this is required, answer N.
-
-config TRUSTED_KEYS
-	tristate "TRUSTED KEYS"
-	depends on KEYS && TCG_TPM
-	select CRYPTO
-	select CRYPTO_HMAC
-	select CRYPTO_SHA1
-	help
-	  This option provides support for creating, sealing, and unsealing
-	  keys in the kernel. Trusted keys are random number symmetric keys,
-	  generated and RSA-sealed by the TPM. The TPM only unseals the keys,
-	  if the boot PCRs and other criteria match.  Userspace will only ever
-	  see encrypted blobs.
-
-	  If you are unsure as to whether this is required, answer N.
-
-config ENCRYPTED_KEYS
-	tristate "ENCRYPTED KEYS"
-	depends on KEYS
-	select CRYPTO
-	select CRYPTO_HMAC
-	select CRYPTO_AES
-	select CRYPTO_CBC
-	select CRYPTO_SHA256
-	select CRYPTO_RNG
-	help
-	  This option provides support for create/encrypting/decrypting keys
-	  in the kernel.  Encrypted keys are kernel generated random numbers,
-	  which are encrypted/decrypted with a 'master' symmetric key. The
-	  'master' key can be either a trusted-key or user-key type.
-	  Userspace only ever sees/stores encrypted blobs.
-
-	  If you are unsure as to whether this is required, answer N.
-
-config KEYS_DEBUG_PROC_KEYS
-	bool "Enable the /proc/keys file by which keys may be viewed"
-	depends on KEYS
-	help
-	  This option turns on support for the /proc/keys file - through which
-	  can be listed all the keys on the system that are viewable by the
-	  reading process.
-
-	  The only keys included in the list are those that grant View
-	  permission to the reading process whether or not it possesses them.
-	  Note that LSM security checks are still performed, and may further
-	  filter out keys that the current process is not authorised to view.
-
-	  Only key attributes are listed here; key payloads are not included in
-	  the resulting table.
-
-	  If you are unsure as to whether this is required, answer N.
+source security/keys/Kconfig
 
 config SECURITY_DMESG_RESTRICT
 	bool "Restrict unprivileged access to the kernel syslog"
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
new file mode 100644
index 0000000..a90d6d3
--- /dev/null
+++ b/security/keys/Kconfig
@@ -0,0 +1,71 @@
+#
+# Key management configuration
+#
+
+config KEYS
+	bool "Enable access key retention support"
+	help
+	  This option provides support for retaining authentication tokens and
+	  access keys in the kernel.
+
+	  It also includes provision of methods by which such keys might be
+	  associated with a process so that network filesystems, encryption
+	  support and the like can find them.
+
+	  Furthermore, a special type of key is available that acts as keyring:
+	  a searchable sequence of keys. Each process is equipped with access
+	  to five standard keyrings: UID-specific, GID-specific, session,
+	  process and thread.
+
+	  If you are unsure as to whether this is required, answer N.
+
+config TRUSTED_KEYS
+	tristate "TRUSTED KEYS"
+	depends on KEYS && TCG_TPM
+	select CRYPTO
+	select CRYPTO_HMAC
+	select CRYPTO_SHA1
+	help
+	  This option provides support for creating, sealing, and unsealing
+	  keys in the kernel. Trusted keys are random number symmetric keys,
+	  generated and RSA-sealed by the TPM. The TPM only unseals the keys,
+	  if the boot PCRs and other criteria match.  Userspace will only ever
+	  see encrypted blobs.
+
+	  If you are unsure as to whether this is required, answer N.
+
+config ENCRYPTED_KEYS
+	tristate "ENCRYPTED KEYS"
+	depends on KEYS
+	select CRYPTO
+	select CRYPTO_HMAC
+	select CRYPTO_AES
+	select CRYPTO_CBC
+	select CRYPTO_SHA256
+	select CRYPTO_RNG
+	help
+	  This option provides support for create/encrypting/decrypting keys
+	  in the kernel.  Encrypted keys are kernel generated random numbers,
+	  which are encrypted/decrypted with a 'master' symmetric key. The
+	  'master' key can be either a trusted-key or user-key type.
+	  Userspace only ever sees/stores encrypted blobs.
+
+	  If you are unsure as to whether this is required, answer N.
+
+config KEYS_DEBUG_PROC_KEYS
+	bool "Enable the /proc/keys file by which keys may be viewed"
+	depends on KEYS
+	help
+	  This option turns on support for the /proc/keys file - through which
+	  can be listed all the keys on the system that are viewable by the
+	  reading process.
+
+	  The only keys included in the list are those that grant View
+	  permission to the reading process whether or not it possesses them.
+	  Note that LSM security checks are still performed, and may further
+	  filter out keys that the current process is not authorised to view.
+
+	  Only key attributes are listed here; key payloads are not included in
+	  the resulting table.
+
+	  If you are unsure as to whether this is required, answer N.


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/9] KEYS: Reorganise keys Makefile
  2012-02-08 11:02 [PATCH 1/9] KEYS: Allow special keyrings to be cleared David Howells
  2012-02-08 11:03 ` [PATCH 2/9] keys: update the description with info about "logon" keys David Howells
  2012-02-08 11:03 ` [PATCH 3/9] KEYS: Move the key config into security/keys/Kconfig David Howells
@ 2012-02-08 11:03 ` David Howells
  2012-02-08 11:03 ` [PATCH 5/9] KEYS: Announce key type (un)registration David Howells
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2012-02-08 11:03 UTC (permalink / raw)
  To: steved, jmorris; +Cc: keyrings, linux-nfs, linux-security-module, linux-kernel

Reorganise the keys directory Makefile to put all the core bits together and
the type-specific bits after.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
---

 security/keys/Makefile |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/security/keys/Makefile b/security/keys/Makefile
index a56f1ff..504aaa0 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -2,6 +2,9 @@
 # Makefile for key management
 #
 
+#
+# Core
+#
 obj-y := \
 	gc.o \
 	key.o \
@@ -12,9 +15,12 @@ obj-y := \
 	request_key.o \
 	request_key_auth.o \
 	user_defined.o
-
-obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
-obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys/
 obj-$(CONFIG_KEYS_COMPAT) += compat.o
 obj-$(CONFIG_PROC_FS) += proc.o
 obj-$(CONFIG_SYSCTL) += sysctl.o
+
+#
+# Key types
+#
+obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
+obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys/


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/9] KEYS: Announce key type (un)registration
  2012-02-08 11:02 [PATCH 1/9] KEYS: Allow special keyrings to be cleared David Howells
                   ` (2 preceding siblings ...)
  2012-02-08 11:03 ` [PATCH 4/9] KEYS: Reorganise keys Makefile David Howells
@ 2012-02-08 11:03 ` David Howells
  2012-02-08 11:03 ` [PATCH 6/9] KEYS: Perform RCU synchronisation on keys prior to key destruction David Howells
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2012-02-08 11:03 UTC (permalink / raw)
  To: steved, jmorris; +Cc: keyrings, linux-nfs, linux-security-module, linux-kernel

Announce the (un)registration of a key type in the core key code rather than
in the callers.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
---

 net/dns_resolver/dns_key.c |    5 -----
 security/keys/key.c        |    3 +++
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index c73bba3..14b2c3d 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -249,9 +249,6 @@ static int __init init_dns_resolver(void)
 	struct key *keyring;
 	int ret;
 
-	printk(KERN_NOTICE "Registering the %s key type\n",
-	       key_type_dns_resolver.name);
-
 	/* create an override credential set with a special thread keyring in
 	 * which DNS requests are cached
 	 *
@@ -301,8 +298,6 @@ static void __exit exit_dns_resolver(void)
 	key_revoke(dns_resolver_cache->thread_keyring);
 	unregister_key_type(&key_type_dns_resolver);
 	put_cred(dns_resolver_cache);
-	printk(KERN_NOTICE "Unregistered %s key type\n",
-	       key_type_dns_resolver.name);
 }
 
 module_init(init_dns_resolver)
diff --git a/security/keys/key.c b/security/keys/key.c
index 7ada801..9173253 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -960,6 +960,8 @@ int register_key_type(struct key_type *ktype)
 
 	/* store the type */
 	list_add(&ktype->link, &key_types_list);
+
+	pr_notice("Key type %s registered\n", ktype->name);
 	ret = 0;
 
 out:
@@ -982,6 +984,7 @@ void unregister_key_type(struct key_type *ktype)
 	list_del_init(&ktype->link);
 	downgrade_write(&key_types_sem);
 	key_gc_keytype(ktype);
+	pr_notice("Key type %s unregistered\n", ktype->name);
 	up_read(&key_types_sem);
 }
 EXPORT_SYMBOL(unregister_key_type);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/9] KEYS: Perform RCU synchronisation on keys prior to key destruction
  2012-02-08 11:02 [PATCH 1/9] KEYS: Allow special keyrings to be cleared David Howells
                   ` (3 preceding siblings ...)
  2012-02-08 11:03 ` [PATCH 5/9] KEYS: Announce key type (un)registration David Howells
@ 2012-02-08 11:03 ` David Howells
  2012-03-19 14:31   ` [Keyrings] " Jeff Layton
  2012-02-08 11:04 ` [PATCH 7/9] KEYS: Permit in-place link replacement in keyring list David Howells
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: David Howells @ 2012-02-08 11:03 UTC (permalink / raw)
  To: steved, jmorris; +Cc: keyrings, linux-nfs, linux-security-module, linux-kernel

Make the keys garbage collector invoke synchronize_rcu() prior to destroying
keys with a zero usage count.  This means that a key can be examined under the
RCU read lock in the safe knowledge that it won't get deallocated until after
the lock is released - even if its usage count becomes zero whilst we're
looking at it.

This is useful in keyring search vs key link.  Consider a keyring containing a
link to a key.  That link can be replaced in-place in the keyring without
requiring an RCU copy-and-replace on the keyring contents without breaking a
search underway on that keyring when the displaced key is released, provided
the key is actually destroyed only after the RCU read lock held by the search
algorithm is released.

This permits __key_link() to replace a key without having to reallocate the key
payload.  A key gets replaced if a new key being linked into a keyring has the
same type and description.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/key.h |    5 +++
 security/keys/gc.c  |   73 +++++++++++++++++++++++++++++++--------------------
 2 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 1600ebf..9832399 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -124,7 +124,10 @@ static inline unsigned long is_key_possessed(const key_ref_t key_ref)
 struct key {
 	atomic_t		usage;		/* number of references */
 	key_serial_t		serial;		/* key serial number */
-	struct rb_node		serial_node;
+	union {
+		struct list_head graveyard_link;
+		struct rb_node	serial_node;
+	};
 	struct key_type		*type;		/* type of key */
 	struct rw_semaphore	sem;		/* change vs change sem */
 	struct key_user		*user;		/* owner of this key */
diff --git a/security/keys/gc.c b/security/keys/gc.c
index a42b455..27610bf 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -168,38 +168,45 @@ do_gc:
 }
 
 /*
- * Garbage collect an unreferenced, detached key
+ * Garbage collect a list of unreferenced, detached keys
  */
-static noinline void key_gc_unused_key(struct key *key)
+static noinline void key_gc_unused_keys(struct list_head *keys)
 {
-	key_check(key);
-
-	security_key_free(key);
-
-	/* deal with the user's key tracking and quota */
-	if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
-		spin_lock(&key->user->lock);
-		key->user->qnkeys--;
-		key->user->qnbytes -= key->quotalen;
-		spin_unlock(&key->user->lock);
-	}
+	while (!list_empty(keys)) {
+		struct key *key =
+			list_entry(keys->next, struct key, graveyard_link);
+		list_del(&key->graveyard_link);
+
+		kdebug("- %u", key->serial);
+		key_check(key);
+
+		security_key_free(key);
+
+		/* deal with the user's key tracking and quota */
+		if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
+			spin_lock(&key->user->lock);
+			key->user->qnkeys--;
+			key->user->qnbytes -= key->quotalen;
+			spin_unlock(&key->user->lock);
+		}
 
-	atomic_dec(&key->user->nkeys);
-	if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
-		atomic_dec(&key->user->nikeys);
+		atomic_dec(&key->user->nkeys);
+		if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
+			atomic_dec(&key->user->nikeys);
 
-	key_user_put(key->user);
+		key_user_put(key->user);
 
-	/* now throw away the key memory */
-	if (key->type->destroy)
-		key->type->destroy(key);
+		/* now throw away the key memory */
+		if (key->type->destroy)
+			key->type->destroy(key);
 
-	kfree(key->description);
+		kfree(key->description);
 
 #ifdef KEY_DEBUGGING
-	key->magic = KEY_DEBUG_MAGIC_X;
+		key->magic = KEY_DEBUG_MAGIC_X;
 #endif
-	kmem_cache_free(key_jar, key);
+		kmem_cache_free(key_jar, key);
+	}
 }
 
 /*
@@ -211,6 +218,7 @@ static noinline void key_gc_unused_key(struct key *key)
  */
 static void key_garbage_collector(struct work_struct *work)
 {
+	static LIST_HEAD(graveyard);
 	static u8 gc_state;		/* Internal persistent state */
 #define KEY_GC_REAP_AGAIN	0x01	/* - Need another cycle */
 #define KEY_GC_REAPING_LINKS	0x02	/* - We need to reap links */
@@ -316,15 +324,22 @@ maybe_resched:
 		key_schedule_gc(new_timer);
 	}
 
-	if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2)) {
-		/* Make sure everyone revalidates their keys if we marked a
-		 * bunch as being dead and make sure all keyring ex-payloads
-		 * are destroyed.
+	if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2) ||
+	    !list_empty(&graveyard)) {
+		/* Make sure that all pending keyring payload destructions are
+		 * fulfilled and that people aren't now looking at dead or
+		 * dying keys that they don't have a reference upon or a link
+		 * to.
 		 */
-		kdebug("dead sync");
+		kdebug("gc sync");
 		synchronize_rcu();
 	}
 
+	if (!list_empty(&graveyard)) {
+		kdebug("gc keys");
+		key_gc_unused_keys(&graveyard);
+	}
+
 	if (unlikely(gc_state & (KEY_GC_REAPING_DEAD_1 |
 				 KEY_GC_REAPING_DEAD_2))) {
 		if (!(gc_state & KEY_GC_FOUND_DEAD_KEY)) {
@@ -359,7 +374,7 @@ found_unreferenced_key:
 	rb_erase(&key->serial_node, &key_serial_tree);
 	spin_unlock(&key_serial_lock);
 
-	key_gc_unused_key(key);
+	list_add_tail(&key->graveyard_link, &graveyard);
 	gc_state |= KEY_GC_REAP_AGAIN;
 	goto maybe_resched;
 


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 7/9] KEYS: Permit in-place link replacement in keyring list
  2012-02-08 11:02 [PATCH 1/9] KEYS: Allow special keyrings to be cleared David Howells
                   ` (4 preceding siblings ...)
  2012-02-08 11:03 ` [PATCH 6/9] KEYS: Perform RCU synchronisation on keys prior to key destruction David Howells
@ 2012-02-08 11:04 ` David Howells
  2012-03-19 14:44   ` [Keyrings] " Jeff Layton
  2012-03-19 15:39   ` David Howells
  2012-02-08 11:04 ` [PATCH 8/9] KEYS: Do LRU discard in full keyrings David Howells
  2012-02-08 11:04 ` [PATCH 9/9] KEYS: Add invalidation support David Howells
  7 siblings, 2 replies; 13+ messages in thread
From: David Howells @ 2012-02-08 11:04 UTC (permalink / raw)
  To: steved, jmorris; +Cc: keyrings, linux-nfs, linux-security-module, linux-kernel

Make use of the previous patch that makes the garbage collector perform RCU
synchronisation before destroying defunct keys.  Key pointers can now be
replaced in-place without creating a new keyring payload and replacing the
whole thing as the discarded keys will not be destroyed until all currently
held RCU read locks are released.

If the keyring payload space needs to be expanded or contracted, then a
replacement will still need allocating, and the original will still have to be
freed by RCU.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/keys/keyring-type.h |    2 -
 security/keys/gc.c          |    2 -
 security/keys/keyring.c     |   95 +++++++++++++++++++++++++------------------
 3 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/include/keys/keyring-type.h b/include/keys/keyring-type.h
index 843f872..cf49159 100644
--- a/include/keys/keyring-type.h
+++ b/include/keys/keyring-type.h
@@ -24,7 +24,7 @@ struct keyring_list {
 	unsigned short	maxkeys;	/* max keys this list can hold */
 	unsigned short	nkeys;		/* number of keys currently held */
 	unsigned short	delkey;		/* key to be unlinked by RCU */
-	struct key	*keys[0];
+	struct key __rcu *keys[0];
 };
 
 
diff --git a/security/keys/gc.c b/security/keys/gc.c
index 27610bf..adddaa2 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -148,7 +148,7 @@ static void key_gc_keyring(struct key *keyring, time_t limit)
 	loop = klist->nkeys;
 	smp_rmb();
 	for (loop--; loop >= 0; loop--) {
-		key = klist->keys[loop];
+		key = rcu_dereference(klist->keys[loop]);
 		if (test_bit(KEY_FLAG_DEAD, &key->flags) ||
 		    (key->expiry > 0 && key->expiry <= limit))
 			goto do_gc;
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index d605f75..459b3cc 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -25,6 +25,11 @@
 		(keyring)->payload.subscriptions,			\
 		rwsem_is_locked((struct rw_semaphore *)&(keyring)->sem)))
 
+#define rcu_deref_link_locked(klist, index, keyring)			\
+	(rcu_dereference_protected(					\
+		(klist)->keys[index],					\
+		rwsem_is_locked((struct rw_semaphore *)&(keyring)->sem)))
+
 #define KEY_LINK_FIXQUOTA 1UL
 
 /*
@@ -138,6 +143,11 @@ static int keyring_match(const struct key *keyring, const void *description)
 /*
  * Clean up a keyring when it is destroyed.  Unpublish its name if it had one
  * and dispose of its data.
+ *
+ * The garbage collector detects the final key_put(), removes the keyring from
+ * the serial number tree and then does RCU synchronisation before coming here,
+ * so we shouldn't need to worry about code poking around here with the RCU
+ * readlock held by this time.
  */
 static void keyring_destroy(struct key *keyring)
 {
@@ -154,11 +164,10 @@ static void keyring_destroy(struct key *keyring)
 		write_unlock(&keyring_name_lock);
 	}
 
-	klist = rcu_dereference_check(keyring->payload.subscriptions,
-				      atomic_read(&keyring->usage) == 0);
+	klist = rcu_access_pointer(keyring->payload.subscriptions);
 	if (klist) {
 		for (loop = klist->nkeys - 1; loop >= 0; loop--)
-			key_put(klist->keys[loop]);
+			key_put(rcu_access_pointer(klist->keys[loop]));
 		kfree(klist);
 	}
 }
@@ -214,7 +223,8 @@ static long keyring_read(const struct key *keyring,
 			ret = -EFAULT;
 
 			for (loop = 0; loop < klist->nkeys; loop++) {
-				key = klist->keys[loop];
+				key = rcu_deref_link_locked(klist, loop,
+							    keyring);
 
 				tmp = sizeof(key_serial_t);
 				if (tmp > buflen)
@@ -383,7 +393,7 @@ descend:
 	nkeys = keylist->nkeys;
 	smp_rmb();
 	for (kix = 0; kix < nkeys; kix++) {
-		key = keylist->keys[kix];
+		key = rcu_dereference(keylist->keys[kix]);
 		kflags = key->flags;
 
 		/* ignore keys not of this type */
@@ -426,7 +436,7 @@ ascend:
 	nkeys = keylist->nkeys;
 	smp_rmb();
 	for (; kix < nkeys; kix++) {
-		key = keylist->keys[kix];
+		key = rcu_dereference(keylist->keys[kix]);
 		if (key->type != &key_type_keyring)
 			continue;
 
@@ -531,8 +541,7 @@ key_ref_t __keyring_search_one(key_ref_t keyring_ref,
 		nkeys = klist->nkeys;
 		smp_rmb();
 		for (loop = 0; loop < nkeys ; loop++) {
-			key = klist->keys[loop];
-
+			key = rcu_dereference(klist->keys[loop]);
 			if (key->type == ktype &&
 			    (!key->type->match ||
 			     key->type->match(key, description)) &&
@@ -654,7 +663,7 @@ ascend:
 	nkeys = keylist->nkeys;
 	smp_rmb();
 	for (; kix < nkeys; kix++) {
-		key = keylist->keys[kix];
+		key = rcu_dereference(keylist->keys[kix]);
 
 		if (key == A)
 			goto cycle_detected;
@@ -711,7 +720,7 @@ static void keyring_unlink_rcu_disposal(struct rcu_head *rcu)
 		container_of(rcu, struct keyring_list, rcu);
 
 	if (klist->delkey != USHRT_MAX)
-		key_put(klist->keys[klist->delkey]);
+		key_put(rcu_access_pointer(klist->keys[klist->delkey]));
 	kfree(klist);
 }
 
@@ -749,24 +758,16 @@ int __key_link_begin(struct key *keyring, const struct key_type *type,
 	/* see if there's a matching key we can displace */
 	if (klist && klist->nkeys > 0) {
 		for (loop = klist->nkeys - 1; loop >= 0; loop--) {
-			if (klist->keys[loop]->type == type &&
-			    strcmp(klist->keys[loop]->description,
-				   description) == 0
-			    ) {
-				/* found a match - we'll replace this one with
-				 * the new key */
-				size = sizeof(struct key *) * klist->maxkeys;
-				size += sizeof(*klist);
-				BUG_ON(size > PAGE_SIZE);
-
-				ret = -ENOMEM;
-				nklist = kmemdup(klist, size, GFP_KERNEL);
-				if (!nklist)
-					goto error_sem;
-
-				/* note replacement slot */
-				klist->delkey = nklist->delkey = loop;
-				prealloc = (unsigned long)nklist;
+			struct key *key = rcu_deref_link_locked(klist, loop,
+								keyring);
+			if (key->type == type &&
+			    strcmp(key->description, description) == 0) {
+				/* Found a match - we'll replace the link with
+				 * one to the new key.  We record the slot
+				 * position.
+				 */
+				klist->delkey = loop;
+				prealloc = 0;
 				goto done;
 			}
 		}
@@ -780,7 +781,7 @@ int __key_link_begin(struct key *keyring, const struct key_type *type,
 
 	if (klist && klist->nkeys < klist->maxkeys) {
 		/* there's sufficient slack space to append directly */
-		nklist = NULL;
+		klist->delkey = klist->nkeys;
 		prealloc = KEY_LINK_FIXQUOTA;
 	} else {
 		/* grow the key list */
@@ -813,10 +814,10 @@ int __key_link_begin(struct key *keyring, const struct key_type *type,
 		}
 
 		/* add the key into the new space */
-		nklist->keys[nklist->delkey] = NULL;
+		RCU_INIT_POINTER(nklist->keys[nklist->delkey], NULL);
+		prealloc = (unsigned long)nklist | KEY_LINK_FIXQUOTA;
 	}
 
-	prealloc = (unsigned long)nklist | KEY_LINK_FIXQUOTA;
 done:
 	*_prealloc = prealloc;
 	kleave(" = 0");
@@ -862,6 +863,7 @@ void __key_link(struct key *keyring, struct key *key,
 		unsigned long *_prealloc)
 {
 	struct keyring_list *klist, *nklist;
+	struct key *discard;
 
 	nklist = (struct keyring_list *)(*_prealloc & ~KEY_LINK_FIXQUOTA);
 	*_prealloc = 0;
@@ -875,10 +877,10 @@ void __key_link(struct key *keyring, struct key *key,
 	/* there's a matching key we can displace or an empty slot in a newly
 	 * allocated list we can fill */
 	if (nklist) {
-		kdebug("replace %hu/%hu/%hu",
+		kdebug("reissue %hu/%hu/%hu",
 		       nklist->delkey, nklist->nkeys, nklist->maxkeys);
 
-		nklist->keys[nklist->delkey] = key;
+		RCU_INIT_POINTER(nklist->keys[nklist->delkey], key);
 
 		rcu_assign_pointer(keyring->payload.subscriptions, nklist);
 
@@ -889,9 +891,23 @@ void __key_link(struct key *keyring, struct key *key,
 			       klist->delkey, klist->nkeys, klist->maxkeys);
 			call_rcu(&klist->rcu, keyring_unlink_rcu_disposal);
 		}
+	} else if (klist->delkey < klist->nkeys) {
+		kdebug("replace %hu/%hu/%hu",
+		       klist->delkey, klist->nkeys, klist->maxkeys);
+
+		discard = rcu_dereference_protected(
+			klist->keys[klist->delkey],
+			rwsem_is_locked(&keyring->sem));
+		rcu_assign_pointer(klist->keys[klist->delkey], key);
+		/* The garbage collector will take care of RCU
+		 * synchronisation */
+		key_put(discard);
 	} else {
 		/* there's sufficient slack space to append directly */
-		klist->keys[klist->nkeys] = key;
+		kdebug("append %hu/%hu/%hu",
+		       klist->delkey, klist->nkeys, klist->maxkeys);
+
+		RCU_INIT_POINTER(klist->keys[klist->delkey], key);
 		smp_wmb();
 		klist->nkeys++;
 	}
@@ -998,7 +1014,7 @@ int key_unlink(struct key *keyring, struct key *key)
 	if (klist) {
 		/* search the keyring for the key */
 		for (loop = 0; loop < klist->nkeys; loop++)
-			if (klist->keys[loop] == key)
+			if (rcu_access_pointer(klist->keys[loop]) == key)
 				goto key_is_present;
 	}
 
@@ -1061,7 +1077,7 @@ static void keyring_clear_rcu_disposal(struct rcu_head *rcu)
 	klist = container_of(rcu, struct keyring_list, rcu);
 
 	for (loop = klist->nkeys - 1; loop >= 0; loop--)
-		key_put(klist->keys[loop]);
+		key_put(rcu_access_pointer(klist->keys[loop]));
 
 	kfree(klist);
 }
@@ -1161,7 +1177,8 @@ void keyring_gc(struct key *keyring, time_t limit)
 	/* work out how many subscriptions we're keeping */
 	keep = 0;
 	for (loop = klist->nkeys - 1; loop >= 0; loop--)
-		if (!key_is_dead(klist->keys[loop], limit))
+		if (!key_is_dead(rcu_deref_link_locked(klist, loop, keyring),
+				 limit))
 			keep++;
 
 	if (keep == klist->nkeys)
@@ -1182,11 +1199,11 @@ void keyring_gc(struct key *keyring, time_t limit)
 	 */
 	keep = 0;
 	for (loop = klist->nkeys - 1; loop >= 0; loop--) {
-		key = klist->keys[loop];
+		key = rcu_deref_link_locked(klist, loop, keyring);
 		if (!key_is_dead(key, limit)) {
 			if (keep >= max)
 				goto discard_new;
-			new->keys[keep++] = key_get(key);
+			RCU_INIT_POINTER(new->keys[keep++], key_get(key));
 		}
 	}
 	new->nkeys = keep;


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 8/9] KEYS: Do LRU discard in full keyrings
  2012-02-08 11:02 [PATCH 1/9] KEYS: Allow special keyrings to be cleared David Howells
                   ` (5 preceding siblings ...)
  2012-02-08 11:04 ` [PATCH 7/9] KEYS: Permit in-place link replacement in keyring list David Howells
@ 2012-02-08 11:04 ` David Howells
  2012-02-08 11:04 ` [PATCH 9/9] KEYS: Add invalidation support David Howells
  7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2012-02-08 11:04 UTC (permalink / raw)
  To: steved, jmorris; +Cc: keyrings, linux-nfs, linux-security-module, linux-kernel

Do an LRU discard in keyrings that are full rather than returning ENFILE.  To
perform this, a time_t is added to the key struct and updated by the creation
of a link to a key and by a key being found as the result of a search.  At the
completion of a successful search, the keyrings in the path between the root of
the search and the first found link to it also have their last-used times
updated.

Note that discarding a link to a key from a keyring does not necessarily
destroy the key as there may be references held by other places.

An alternate discard method that might suffice is to perform FIFO discard from
the keyring, using the spare 2-byte hole in the keylist header as the index of
the next link to be discarded.

This is useful when using a keyring as a cache for DNS results or foreign
filesystem IDs.


This can be tested by the following.  As root do:

	echo 1000 >/proc/sys/kernel/keys/root_maxkeys

	kr=`keyctl newring foo @s`
	for ((i=0; i<2000; i++)); do keyctl add user a$i a $kr; done

Without this patch ENFILE should be reported when the keyring fills up.  With
this patch, the keyring discards keys in an LRU fashion.  Note that the stored
LRU time has a granularity of 1s.

After doing this, /proc/key-users can be observed and should show that most of
the 2000 keys have been discarded:

	[root@andromeda ~]# cat /proc/key-users
	    0:   517 516/516 513/1000 5249/20000

The "513/1000" here is the number of quota-accounted keys present for this user
out of the maximum permitted.

In /proc/keys, the keyring shows the number of keys it has and the number of
slots it has allocated:

	[root@andromeda ~]# grep foo /proc/keys
	200c64c4 I--Q--     1 perm 3b3f0000     0     0 keyring   foo: 509/509

The maximum is (PAGE_SIZE - header) / key pointer size.  That's typically 509
on a 64-bit system and 1020 on a 32-bit system.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/key.h          |    1 +
 security/keys/keyring.c      |   47 ++++++++++++++++++++++++++++++++++++------
 security/keys/process_keys.c |    2 ++
 3 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 9832399..3ed27b7 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -136,6 +136,7 @@ struct key {
 		time_t		expiry;		/* time at which key expires (or 0) */
 		time_t		revoked_at;	/* time at which key was revoked */
 	};
+	time_t			last_used_at;	/* last time used for LRU keyring discard */
 	uid_t			uid;
 	gid_t			gid;
 	key_perm_t		perm;		/* access permissions */
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 459b3cc..89d02cf 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -30,6 +30,10 @@
 		(klist)->keys[index],					\
 		rwsem_is_locked((struct rw_semaphore *)&(keyring)->sem)))
 
+#define MAX_KEYRING_LINKS						\
+	min_t(size_t, USHRT_MAX - 1,					\
+	      ((PAGE_SIZE - sizeof(struct keyring_list)) / sizeof(struct key *)))
+
 #define KEY_LINK_FIXQUOTA 1UL
 
 /*
@@ -319,6 +323,8 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref,
 			     bool no_state_check)
 {
 	struct {
+		/* Need a separate keylist pointer for RCU purposes */
+		struct key *keyring;
 		struct keyring_list *keylist;
 		int kix;
 	} stack[KEYRING_SEARCH_MAX_DEPTH];
@@ -451,6 +457,7 @@ ascend:
 			continue;
 
 		/* stack the current position */
+		stack[sp].keyring = keyring;
 		stack[sp].keylist = keylist;
 		stack[sp].kix = kix;
 		sp++;
@@ -466,6 +473,7 @@ not_this_keyring:
 	if (sp > 0) {
 		/* resume the processing of a keyring higher up in the tree */
 		sp--;
+		keyring = stack[sp].keyring;
 		keylist = stack[sp].keylist;
 		kix = stack[sp].kix + 1;
 		goto ascend;
@@ -477,6 +485,10 @@ not_this_keyring:
 	/* we found a viable match */
 found:
 	atomic_inc(&key->usage);
+	key->last_used_at = now.tv_sec;
+	keyring->last_used_at = now.tv_sec;
+	while (sp > 0)
+		stack[--sp].keyring->last_used_at = now.tv_sec;
 	key_check(key);
 	key_ref = make_key_ref(key, possessed);
 error_2:
@@ -558,6 +570,8 @@ key_ref_t __keyring_search_one(key_ref_t keyring_ref,
 
 found:
 	atomic_inc(&key->usage);
+	keyring->last_used_at = key->last_used_at =
+		current_kernel_time().tv_sec;
 	rcu_read_unlock();
 	return make_key_ref(key, possessed);
 }
@@ -611,6 +625,7 @@ struct key *find_keyring_by_name(const char *name, bool skip_perm_check)
 			 * (ie. it has a zero usage count) */
 			if (!atomic_inc_not_zero(&keyring->usage))
 				continue;
+			keyring->last_used_at = current_kernel_time().tv_sec;
 			goto out;
 		}
 	}
@@ -734,8 +749,9 @@ int __key_link_begin(struct key *keyring, const struct key_type *type,
 	struct keyring_list *klist, *nklist;
 	unsigned long prealloc;
 	unsigned max;
+	time_t lowest_lru;
 	size_t size;
-	int loop, ret;
+	int loop, lru, ret;
 
 	kenter("%d,%s,%s,", key_serial(keyring), type->name, description);
 
@@ -756,7 +772,9 @@ int __key_link_begin(struct key *keyring, const struct key_type *type,
 	klist = rcu_dereference_locked_keyring(keyring);
 
 	/* see if there's a matching key we can displace */
+	lru = -1;
 	if (klist && klist->nkeys > 0) {
+		lowest_lru = TIME_T_MAX;
 		for (loop = klist->nkeys - 1; loop >= 0; loop--) {
 			struct key *key = rcu_deref_link_locked(klist, loop,
 								keyring);
@@ -770,9 +788,23 @@ int __key_link_begin(struct key *keyring, const struct key_type *type,
 				prealloc = 0;
 				goto done;
 			}
+			if (key->last_used_at < lowest_lru) {
+				lowest_lru = key->last_used_at;
+				lru = loop;
+			}
 		}
 	}
 
+	/* If the keyring is full then do an LRU discard */
+	if (klist &&
+	    klist->nkeys == klist->maxkeys &&
+	    klist->maxkeys >= MAX_KEYRING_LINKS) {
+		kdebug("LRU discard %d\n", lru);
+		klist->delkey = lru;
+		prealloc = 0;
+		goto done;
+	}
+
 	/* check that we aren't going to overrun the user's quota */
 	ret = key_payload_reserve(keyring,
 				  keyring->datalen + KEYQUOTA_LINK_BYTES);
@@ -786,15 +818,14 @@ int __key_link_begin(struct key *keyring, const struct key_type *type,
 	} else {
 		/* grow the key list */
 		max = 4;
-		if (klist)
+		if (klist) {
 			max += klist->maxkeys;
+			if (max > MAX_KEYRING_LINKS)
+				max = MAX_KEYRING_LINKS;
+			BUG_ON(max <= klist->maxkeys);
+		}
 
-		ret = -ENFILE;
-		if (max > USHRT_MAX - 1)
-			goto error_quota;
 		size = sizeof(*klist) + sizeof(struct key *) * max;
-		if (size > PAGE_SIZE)
-			goto error_quota;
 
 		ret = -ENOMEM;
 		nklist = kmalloc(size, GFP_KERNEL);
@@ -873,6 +904,8 @@ void __key_link(struct key *keyring, struct key *key,
 	klist = rcu_dereference_locked_keyring(keyring);
 
 	atomic_inc(&key->usage);
+	keyring->last_used_at = key->last_used_at =
+		current_kernel_time().tv_sec;
 
 	/* there's a matching key we can displace or an empty slot in a newly
 	 * allocated list we can fill */
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 1068cb1..b1cdb56 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -731,6 +731,8 @@ try_again:
 	if (ret < 0)
 		goto invalid_key;
 
+	key->last_used_at = current_kernel_time().tv_sec;
+
 error:
 	put_cred(cred);
 	return key_ref;


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 9/9] KEYS: Add invalidation support
  2012-02-08 11:02 [PATCH 1/9] KEYS: Allow special keyrings to be cleared David Howells
                   ` (6 preceding siblings ...)
  2012-02-08 11:04 ` [PATCH 8/9] KEYS: Do LRU discard in full keyrings David Howells
@ 2012-02-08 11:04 ` David Howells
  7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2012-02-08 11:04 UTC (permalink / raw)
  To: steved, jmorris; +Cc: keyrings, linux-nfs, linux-security-module, linux-kernel

Add support for invalidating a key - which renders it immediately invisible to
further searches and causes the garbage collector to immediately wake up,
remove it from keyrings and then destroy it when it's no longer referenced.

It's better not to do this with keyctl_revoke() as that marks the key to start
returning -EKEYREVOKED to searches when what is actually desired is to have the
key refetched.

To invalidate a key the caller must be granted SEARCH permission by the key.
This may be too strict.  It may be better to also permit invalidation if the
caller has any of READ, WRITE or SETATTR permission.

The primary use for this is to evict keys that are cached in special keyrings,
such as the DNS resolver or an ID mapper.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 Documentation/security/keys.txt |   17 +++++++++++++++++
 include/linux/key.h             |    3 +++
 include/linux/keyctl.h          |    1 +
 security/keys/compat.c          |    3 +++
 security/keys/gc.c              |   21 ++++++++++++++-------
 security/keys/internal.h        |   15 ++++++++++++++-
 security/keys/key.c             |   22 ++++++++++++++++++++++
 security/keys/keyctl.c          |   34 ++++++++++++++++++++++++++++++++++
 security/keys/keyring.c         |   25 +++++++++++--------------
 security/keys/permission.c      |   15 ++++++++++-----
 security/keys/proc.c            |    3 ++-
 11 files changed, 131 insertions(+), 28 deletions(-)

diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index be3d229..69d4f48 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -806,6 +806,23 @@ The keyctl syscall functions are:
      kernel and resumes executing userspace.
 
 
+ (*) Invalidate a key.
+
+	long keyctl(KEYCTL_INVALIDATE, key_serial_t key);
+
+     This function marks a key as being invalidated and then wakes up the
+     garbage collector.  The garbage collector immediately removes invalidated
+     keys from all keyrings and deletes the key when its reference count
+     reaches zero.
+
+     Keys that are marked invalidated become invisible to normal key operations
+     immediately, though they are still visible in /proc/keys until deleted
+     (they're marked with an 'i' flag).
+
+     A process must have search permission on the key for this function to be
+     successful.
+
+
 ===============
 KERNEL SERVICES
 ===============
diff --git a/include/linux/key.h b/include/linux/key.h
index 3ed27b7..30dbf81 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -160,6 +160,7 @@ struct key {
 #define KEY_FLAG_USER_CONSTRUCT	4	/* set if key is being constructed in userspace */
 #define KEY_FLAG_NEGATIVE	5	/* set if key is negative */
 #define KEY_FLAG_ROOT_CAN_CLEAR	6	/* set if key can be cleared by root without permission */
+#define KEY_FLAG_INVALIDATED	7	/* set if key has been invalidated */
 
 	/* the description string
 	 * - this is used to match a key against search criteria
@@ -203,6 +204,7 @@ extern struct key *key_alloc(struct key_type *type,
 #define KEY_ALLOC_NOT_IN_QUOTA	0x0002	/* not in quota */
 
 extern void key_revoke(struct key *key);
+extern void key_invalidate(struct key *key);
 extern void key_put(struct key *key);
 
 static inline struct key *key_get(struct key *key)
@@ -321,6 +323,7 @@ extern void key_init(void);
 #define key_serial(k)			0
 #define key_get(k) 			({ NULL; })
 #define key_revoke(k)			do { } while(0)
+#define key_invalidate(k)		do { } while(0)
 #define key_put(k)			do { } while(0)
 #define key_ref_put(k)			do { } while(0)
 #define make_key_ref(k, p)		NULL
diff --git a/include/linux/keyctl.h b/include/linux/keyctl.h
index 9b0b865..c9b7f4fa 100644
--- a/include/linux/keyctl.h
+++ b/include/linux/keyctl.h
@@ -55,5 +55,6 @@
 #define KEYCTL_SESSION_TO_PARENT	18	/* apply session keyring to parent process */
 #define KEYCTL_REJECT			19	/* reject a partially constructed key */
 #define KEYCTL_INSTANTIATE_IOV		20	/* instantiate a partially constructed key */
+#define KEYCTL_INVALIDATE		21	/* invalidate a key */
 
 #endif /*  _LINUX_KEYCTL_H */
diff --git a/security/keys/compat.c b/security/keys/compat.c
index 4c48e13..fab4f8d 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -135,6 +135,9 @@ asmlinkage long compat_sys_keyctl(u32 option,
 		return compat_keyctl_instantiate_key_iov(
 			arg2, compat_ptr(arg3), arg4, arg5);
 
+	case KEYCTL_INVALIDATE:
+		return keyctl_invalidate_key(arg2);
+
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/security/keys/gc.c b/security/keys/gc.c
index adddaa2..61ab7c8 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -72,6 +72,15 @@ void key_schedule_gc(time_t gc_at)
 }
 
 /*
+ * Schedule a dead links collection run.
+ */
+void key_schedule_gc_links(void)
+{
+	set_bit(KEY_GC_KEY_EXPIRED, &key_gc_flags);
+	queue_work(system_nrt_wq, &key_gc_work);
+}
+
+/*
  * Some key's cleanup time was met after it expired, so we need to get the
  * reaper to go through a cycle finding expired keys.
  */
@@ -79,8 +88,7 @@ static void key_gc_timer_func(unsigned long data)
 {
 	kenter("");
 	key_gc_next_run = LONG_MAX;
-	set_bit(KEY_GC_KEY_EXPIRED, &key_gc_flags);
-	queue_work(system_nrt_wq, &key_gc_work);
+	key_schedule_gc_links();
 }
 
 /*
@@ -131,12 +139,12 @@ void key_gc_keytype(struct key_type *ktype)
 static void key_gc_keyring(struct key *keyring, time_t limit)
 {
 	struct keyring_list *klist;
-	struct key *key;
 	int loop;
 
 	kenter("%x", key_serial(keyring));
 
-	if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
+	if (keyring->flags & ((1 << KEY_FLAG_INVALIDATED) |
+			      (1 << KEY_FLAG_REVOKED)))
 		goto dont_gc;
 
 	/* scan the keyring looking for dead keys */
@@ -148,9 +156,8 @@ static void key_gc_keyring(struct key *keyring, time_t limit)
 	loop = klist->nkeys;
 	smp_rmb();
 	for (loop--; loop >= 0; loop--) {
-		key = rcu_dereference(klist->keys[loop]);
-		if (test_bit(KEY_FLAG_DEAD, &key->flags) ||
-		    (key->expiry > 0 && key->expiry <= limit))
+		struct key *key = rcu_dereference(klist->keys[loop]);
+		if (key_is_dead(key, limit))
 			goto do_gc;
 	}
 
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 65647f8..f711b09 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -152,7 +152,8 @@ extern long join_session_keyring(const char *name);
 extern struct work_struct key_gc_work;
 extern unsigned key_gc_delay;
 extern void keyring_gc(struct key *keyring, time_t limit);
-extern void key_schedule_gc(time_t expiry_at);
+extern void key_schedule_gc(time_t gc_at);
+extern void key_schedule_gc_links(void);
 extern void key_gc_keytype(struct key_type *ktype);
 
 extern int key_task_permission(const key_ref_t key_ref,
@@ -197,6 +198,17 @@ extern struct key *request_key_auth_new(struct key *target,
 extern struct key *key_get_instantiation_authkey(key_serial_t target_id);
 
 /*
+ * Determine whether a key is dead.
+ */
+static inline bool key_is_dead(struct key *key, time_t limit)
+{
+	return
+		key->flags & ((1 << KEY_FLAG_DEAD) |
+			      (1 << KEY_FLAG_INVALIDATED)) ||
+		(key->expiry > 0 && key->expiry <= limit);
+}
+
+/*
  * keyctl() functions
  */
 extern long keyctl_get_keyring_ID(key_serial_t, int);
@@ -225,6 +237,7 @@ extern long keyctl_reject_key(key_serial_t, unsigned, unsigned, key_serial_t);
 extern long keyctl_instantiate_key_iov(key_serial_t,
 				       const struct iovec __user *,
 				       unsigned, key_serial_t);
+extern long keyctl_invalidate_key(key_serial_t);
 
 extern long keyctl_instantiate_key_common(key_serial_t,
 					  const struct iovec __user *,
diff --git a/security/keys/key.c b/security/keys/key.c
index 9173253..3f409f6 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -935,6 +935,28 @@ void key_revoke(struct key *key)
 EXPORT_SYMBOL(key_revoke);
 
 /**
+ * key_invalidate - Invalidate a key.
+ * @key: The key to be invalidated.
+ *
+ * Mark a key as being invalidated and have it cleaned up immediately.  The key
+ * is ignored by all searches and other operations from this point.
+ */
+void key_invalidate(struct key *key)
+{
+	kenter("%d", key_serial(key));
+
+	key_check(key);
+
+	if (!test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
+		down_write_nested(&key->sem, 1);
+		if (!test_and_set_bit(KEY_FLAG_INVALIDATED, &key->flags))
+			key_schedule_gc_links();
+		up_write(&key->sem);
+	}
+}
+EXPORT_SYMBOL(key_invalidate);
+
+/**
  * register_key_type - Register a type of key.
  * @ktype: The new key type.
  *
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 6523599..477201f 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -374,6 +374,37 @@ error:
 }
 
 /*
+ * Invalidate a key.
+ *
+ * The key must be grant the caller Invalidate permission for this to work.
+ * The key and any links to the key will be automatically garbage collected
+ * immediately.
+ *
+ * If successful, 0 is returned.
+ */
+long keyctl_invalidate_key(key_serial_t id)
+{
+	key_ref_t key_ref;
+	long ret;
+
+	kenter("%d", id);
+
+	key_ref = lookup_user_key(id, 0, KEY_SEARCH);
+	if (IS_ERR(key_ref)) {
+		ret = PTR_ERR(key_ref);
+		goto error;
+	}
+
+	key_invalidate(key_ref_to_ptr(key_ref));
+	ret = 0;
+
+	key_ref_put(key_ref);
+error:
+	kleave(" = %ld", ret);
+	return ret;
+}
+
+/*
  * Clear the specified keyring, creating an empty process keyring if one of the
  * special keyring IDs is used.
  *
@@ -1636,6 +1667,9 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
 			(unsigned) arg4,
 			(key_serial_t) arg5);
 
+	case KEYCTL_INVALIDATE:
+		return keyctl_invalidate_key((key_serial_t) arg2);
+
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 89d02cf..7445875 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -382,13 +382,17 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref,
 	/* otherwise, the top keyring must not be revoked, expired, or
 	 * negatively instantiated if we are to search it */
 	key_ref = ERR_PTR(-EAGAIN);
-	if (kflags & ((1 << KEY_FLAG_REVOKED) | (1 << KEY_FLAG_NEGATIVE)) ||
+	if (kflags & ((1 << KEY_FLAG_INVALIDATED) |
+		      (1 << KEY_FLAG_REVOKED) |
+		      (1 << KEY_FLAG_NEGATIVE)) ||
 	    (keyring->expiry && now.tv_sec >= keyring->expiry))
 		goto error_2;
 
 	/* start processing a new keyring */
 descend:
-	if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
+	kflags = keyring->flags;
+	if (kflags & ((1 << KEY_FLAG_INVALIDATED) |
+		      (1 << KEY_FLAG_REVOKED)))
 		goto not_this_keyring;
 
 	keylist = rcu_dereference(keyring->payload.subscriptions);
@@ -406,9 +410,10 @@ descend:
 		if (key->type != type)
 			continue;
 
-		/* skip revoked keys and expired keys */
+		/* skip invalidated, revoked and expired keys */
 		if (!no_state_check) {
-			if (kflags & (1 << KEY_FLAG_REVOKED))
+			if (kflags & ((1 << KEY_FLAG_INVALIDATED) |
+				      (1 << KEY_FLAG_REVOKED)))
 				continue;
 
 			if (key->expiry && now.tv_sec >= key->expiry)
@@ -559,7 +564,8 @@ key_ref_t __keyring_search_one(key_ref_t keyring_ref,
 			     key->type->match(key, description)) &&
 			    key_permission(make_key_ref(key, possessed),
 					   perm) == 0 &&
-			    !test_bit(KEY_FLAG_REVOKED, &key->flags)
+			    !(key->flags & ((1 << KEY_FLAG_INVALIDATED) |
+					    (1 << KEY_FLAG_REVOKED)))
 			    )
 				goto found;
 		}
@@ -1177,15 +1183,6 @@ static void keyring_revoke(struct key *keyring)
 }
 
 /*
- * Determine whether a key is dead.
- */
-static bool key_is_dead(struct key *key, time_t limit)
-{
-	return test_bit(KEY_FLAG_DEAD, &key->flags) ||
-		(key->expiry > 0 && key->expiry <= limit);
-}
-
-/*
  * Collect garbage from the contents of a keyring, replacing the old list with
  * a new one with the pointers all shuffled down.
  *
diff --git a/security/keys/permission.c b/security/keys/permission.c
index c35b522..5f4c00c 100644
--- a/security/keys/permission.c
+++ b/security/keys/permission.c
@@ -87,20 +87,25 @@ EXPORT_SYMBOL(key_task_permission);
  * key_validate - Validate a key.
  * @key: The key to be validated.
  *
- * Check that a key is valid, returning 0 if the key is okay, -EKEYREVOKED if
- * the key's type has been removed or if the key has been revoked or
- * -EKEYEXPIRED if the key has expired.
+ * Check that a key is valid, returning 0 if the key is okay, -ENOKEY if the
+ * key is invalidated, -EKEYREVOKED if the key's type has been removed or if
+ * the key has been revoked or -EKEYEXPIRED if the key has expired.
  */
 int key_validate(struct key *key)
 {
 	struct timespec now;
+	unsigned long flags = key->flags;
 	int ret = 0;
 
 	if (key) {
+		ret = -ENOKEY;
+		if (flags & (1 << KEY_FLAG_INVALIDATED))
+			goto error;
+
 		/* check it's still accessible */
 		ret = -EKEYREVOKED;
-		if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
-		    test_bit(KEY_FLAG_DEAD, &key->flags))
+		if (flags & ((1 << KEY_FLAG_REVOKED) |
+			     (1 << KEY_FLAG_DEAD)))
 			goto error;
 
 		/* check it hasn't expired */
diff --git a/security/keys/proc.c b/security/keys/proc.c
index 49bbc97..30d1ddf 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -242,7 +242,7 @@ static int proc_keys_show(struct seq_file *m, void *v)
 #define showflag(KEY, LETTER, FLAG) \
 	(test_bit(FLAG,	&(KEY)->flags) ? LETTER : '-')
 
-	seq_printf(m, "%08x %c%c%c%c%c%c %5d %4s %08x %5d %5d %-9.9s ",
+	seq_printf(m, "%08x %c%c%c%c%c%c%c %5d %4s %08x %5d %5d %-9.9s ",
 		   key->serial,
 		   showflag(key, 'I', KEY_FLAG_INSTANTIATED),
 		   showflag(key, 'R', KEY_FLAG_REVOKED),
@@ -250,6 +250,7 @@ static int proc_keys_show(struct seq_file *m, void *v)
 		   showflag(key, 'Q', KEY_FLAG_IN_QUOTA),
 		   showflag(key, 'U', KEY_FLAG_USER_CONSTRUCT),
 		   showflag(key, 'N', KEY_FLAG_NEGATIVE),
+		   showflag(key, 'i', KEY_FLAG_INVALIDATED),
 		   atomic_read(&key->usage),
 		   xbuf,
 		   key->perm,


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Keyrings] [PATCH 6/9] KEYS: Perform RCU synchronisation on keys prior to key destruction
  2012-02-08 11:03 ` [PATCH 6/9] KEYS: Perform RCU synchronisation on keys prior to key destruction David Howells
@ 2012-03-19 14:31   ` Jeff Layton
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2012-03-19 14:31 UTC (permalink / raw)
  To: David Howells
  Cc: steved, jmorris, linux-security-module, keyrings, linux-nfs,
	linux-kernel

On Wed, 08 Feb 2012 11:03:49 +0000
David Howells <dhowells@redhat.com> wrote:

> Make the keys garbage collector invoke synchronize_rcu() prior to destroying
> keys with a zero usage count.  This means that a key can be examined under the
> RCU read lock in the safe knowledge that it won't get deallocated until after
> the lock is released - even if its usage count becomes zero whilst we're
> looking at it.
> 
> This is useful in keyring search vs key link.  Consider a keyring containing a
> link to a key.  That link can be replaced in-place in the keyring without
> requiring an RCU copy-and-replace on the keyring contents without breaking a
> search underway on that keyring when the displaced key is released, provided
> the key is actually destroyed only after the RCU read lock held by the search
> algorithm is released.
> 
> This permits __key_link() to replace a key without having to reallocate the key
> payload.  A key gets replaced if a new key being linked into a keyring has the
> same type and description.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  include/linux/key.h |    5 +++
>  security/keys/gc.c  |   73 +++++++++++++++++++++++++++++++--------------------
>  2 files changed, 48 insertions(+), 30 deletions(-)
> 
> diff --git a/include/linux/key.h b/include/linux/key.h
> index 1600ebf..9832399 100644
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -124,7 +124,10 @@ static inline unsigned long is_key_possessed(const key_ref_t key_ref)
>  struct key {
>  	atomic_t		usage;		/* number of references */
>  	key_serial_t		serial;		/* key serial number */
> -	struct rb_node		serial_node;
> +	union {
> +		struct list_head graveyard_link;
> +		struct rb_node	serial_node;
> +	};
>  	struct key_type		*type;		/* type of key */
>  	struct rw_semaphore	sem;		/* change vs change sem */
>  	struct key_user		*user;		/* owner of this key */
> diff --git a/security/keys/gc.c b/security/keys/gc.c
> index a42b455..27610bf 100644
> --- a/security/keys/gc.c
> +++ b/security/keys/gc.c
> @@ -168,38 +168,45 @@ do_gc:
>  }
>  
>  /*
> - * Garbage collect an unreferenced, detached key
> + * Garbage collect a list of unreferenced, detached keys
>   */
> -static noinline void key_gc_unused_key(struct key *key)
> +static noinline void key_gc_unused_keys(struct list_head *keys)
>  {
> -	key_check(key);
> -
> -	security_key_free(key);
> -
> -	/* deal with the user's key tracking and quota */
> -	if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
> -		spin_lock(&key->user->lock);
> -		key->user->qnkeys--;
> -		key->user->qnbytes -= key->quotalen;
> -		spin_unlock(&key->user->lock);
> -	}
> +	while (!list_empty(keys)) {
> +		struct key *key =
> +			list_entry(keys->next, struct key, graveyard_link);
> +		list_del(&key->graveyard_link);
> +
> +		kdebug("- %u", key->serial);
> +		key_check(key);
> +
> +		security_key_free(key);
> +
> +		/* deal with the user's key tracking and quota */
> +		if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
> +			spin_lock(&key->user->lock);
> +			key->user->qnkeys--;
> +			key->user->qnbytes -= key->quotalen;
> +			spin_unlock(&key->user->lock);
> +		}
>  
> -	atomic_dec(&key->user->nkeys);
> -	if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
> -		atomic_dec(&key->user->nikeys);
> +		atomic_dec(&key->user->nkeys);
> +		if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
> +			atomic_dec(&key->user->nikeys);
>  
> -	key_user_put(key->user);
> +		key_user_put(key->user);
>  
> -	/* now throw away the key memory */
> -	if (key->type->destroy)
> -		key->type->destroy(key);
> +		/* now throw away the key memory */
> +		if (key->type->destroy)
> +			key->type->destroy(key);
>  
> -	kfree(key->description);
> +		kfree(key->description);
>  
>  #ifdef KEY_DEBUGGING
> -	key->magic = KEY_DEBUG_MAGIC_X;
> +		key->magic = KEY_DEBUG_MAGIC_X;
>  #endif
> -	kmem_cache_free(key_jar, key);
> +		kmem_cache_free(key_jar, key);
> +	}
>  }
>  
>  /*
> @@ -211,6 +218,7 @@ static noinline void key_gc_unused_key(struct key *key)
>   */
>  static void key_garbage_collector(struct work_struct *work)
>  {
> +	static LIST_HEAD(graveyard);
>  	static u8 gc_state;		/* Internal persistent state */
>  #define KEY_GC_REAP_AGAIN	0x01	/* - Need another cycle */
>  #define KEY_GC_REAPING_LINKS	0x02	/* - We need to reap links */
> @@ -316,15 +324,22 @@ maybe_resched:
>  		key_schedule_gc(new_timer);
>  	}
>  
> -	if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2)) {
> -		/* Make sure everyone revalidates their keys if we marked a
> -		 * bunch as being dead and make sure all keyring ex-payloads
> -		 * are destroyed.
> +	if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2) ||
> +	    !list_empty(&graveyard)) {
> +		/* Make sure that all pending keyring payload destructions are
> +		 * fulfilled and that people aren't now looking at dead or
> +		 * dying keys that they don't have a reference upon or a link
> +		 * to.
>  		 */
> -		kdebug("dead sync");
> +		kdebug("gc sync");
>  		synchronize_rcu();
>  	}
>  
> +	if (!list_empty(&graveyard)) {
> +		kdebug("gc keys");
> +		key_gc_unused_keys(&graveyard);
> +	}
> +
>  	if (unlikely(gc_state & (KEY_GC_REAPING_DEAD_1 |
>  				 KEY_GC_REAPING_DEAD_2))) {
>  		if (!(gc_state & KEY_GC_FOUND_DEAD_KEY)) {
> @@ -359,7 +374,7 @@ found_unreferenced_key:
>  	rb_erase(&key->serial_node, &key_serial_tree);
>  	spin_unlock(&key_serial_lock);
>  
> -	key_gc_unused_key(key);
> +	list_add_tail(&key->graveyard_link, &graveyard);
>  	gc_state |= KEY_GC_REAP_AGAIN;
>  	goto maybe_resched;
>  
> 
> _______________________________________________
> Keyrings mailing list
> Keyrings@linux-nfs.org
> To change your subscription to this list, please see http://linux-nfs.org/cgi-bin/mailman/listinfo/keyrings

Not what I'd call straightforward code, but I think it looks correct...

Acked-by: Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Keyrings] [PATCH 7/9] KEYS: Permit in-place link replacement in keyring list
  2012-02-08 11:04 ` [PATCH 7/9] KEYS: Permit in-place link replacement in keyring list David Howells
@ 2012-03-19 14:44   ` Jeff Layton
  2012-03-19 15:39   ` David Howells
  1 sibling, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2012-03-19 14:44 UTC (permalink / raw)
  To: David Howells
  Cc: steved, jmorris, linux-security-module, keyrings, linux-nfs,
	linux-kernel

On Wed, 08 Feb 2012 11:04:00 +0000
David Howells <dhowells@redhat.com> wrote:

> @@ -154,11 +164,10 @@ static void keyring_destroy(struct key *keyring)
>  		write_unlock(&keyring_name_lock);
>  	}
>  
> -	klist = rcu_dereference_check(keyring->payload.subscriptions,
> -				      atomic_read(&keyring->usage) == 0);
> +	klist = rcu_access_pointer(keyring->payload.subscriptions);
>  	if (klist) {
>  		for (loop = klist->nkeys - 1; loop >= 0; loop--)
> -			key_put(klist->keys[loop]);
> +			key_put(rcu_access_pointer(klist->keys[loop]));
>  		kfree(klist);
>  	}
>  }

Why is it safe to use key_put(rcu_access_pointer(...)) ? Clearly that
pointer will end up being dereferenced, right?

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Keyrings] [PATCH 7/9] KEYS: Permit in-place link replacement in keyring list
  2012-02-08 11:04 ` [PATCH 7/9] KEYS: Permit in-place link replacement in keyring list David Howells
  2012-03-19 14:44   ` [Keyrings] " Jeff Layton
@ 2012-03-19 15:39   ` David Howells
  1 sibling, 0 replies; 13+ messages in thread
From: David Howells @ 2012-03-19 15:39 UTC (permalink / raw)
  To: Jeff Layton
  Cc: dhowells, steved, jmorris, linux-security-module, keyrings,
	linux-nfs, linux-kernel

Jeff Layton <jlayton@redhat.com> wrote:

> Why is it safe to use key_put(rcu_access_pointer(...)) ? Clearly that
> pointer will end up being dereferenced, right?

Simple.  This key_put() is called from within keyring_destroy() and that is
only ever called from the garbage collector - which is non-reentrant.  Add to
that that key_put() never actually destroys a now-unused key, but merely
schedules the gc for a future run, and it's *that* that searches out unused
keys and deletes them.

David

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 5/9] KEYS: Announce key type (un)registration
  2012-03-28 10:46 [PATCH 1/9] KEYS: Use the compat keyctl() syscall wrapper on Sparc64 for Sparc32 compat David Howells
@ 2012-03-28 10:46 ` David Howells
  0 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2012-03-28 10:46 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security-module, keyrings, linux-kernel, David Howells, Mimi Zohar

Announce the (un)registration of a key type in the core key code rather than
in the callers.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
---

 net/dns_resolver/dns_key.c |    5 -----
 security/keys/key.c        |    3 +++
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index c73bba3..14b2c3d 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -249,9 +249,6 @@ static int __init init_dns_resolver(void)
 	struct key *keyring;
 	int ret;
 
-	printk(KERN_NOTICE "Registering the %s key type\n",
-	       key_type_dns_resolver.name);
-
 	/* create an override credential set with a special thread keyring in
 	 * which DNS requests are cached
 	 *
@@ -301,8 +298,6 @@ static void __exit exit_dns_resolver(void)
 	key_revoke(dns_resolver_cache->thread_keyring);
 	unregister_key_type(&key_type_dns_resolver);
 	put_cred(dns_resolver_cache);
-	printk(KERN_NOTICE "Unregistered %s key type\n",
-	       key_type_dns_resolver.name);
 }
 
 module_init(init_dns_resolver)
diff --git a/security/keys/key.c b/security/keys/key.c
index 06783cf..dc62894 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -980,6 +980,8 @@ int register_key_type(struct key_type *ktype)
 
 	/* store the type */
 	list_add(&ktype->link, &key_types_list);
+
+	pr_notice("Key type %s registered\n", ktype->name);
 	ret = 0;
 
 out:
@@ -1002,6 +1004,7 @@ void unregister_key_type(struct key_type *ktype)
 	list_del_init(&ktype->link);
 	downgrade_write(&key_types_sem);
 	key_gc_keytype(ktype);
+	pr_notice("Key type %s unregistered\n", ktype->name);
 	up_read(&key_types_sem);
 }
 EXPORT_SYMBOL(unregister_key_type);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-03-28 10:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-08 11:02 [PATCH 1/9] KEYS: Allow special keyrings to be cleared David Howells
2012-02-08 11:03 ` [PATCH 2/9] keys: update the description with info about "logon" keys David Howells
2012-02-08 11:03 ` [PATCH 3/9] KEYS: Move the key config into security/keys/Kconfig David Howells
2012-02-08 11:03 ` [PATCH 4/9] KEYS: Reorganise keys Makefile David Howells
2012-02-08 11:03 ` [PATCH 5/9] KEYS: Announce key type (un)registration David Howells
2012-02-08 11:03 ` [PATCH 6/9] KEYS: Perform RCU synchronisation on keys prior to key destruction David Howells
2012-03-19 14:31   ` [Keyrings] " Jeff Layton
2012-02-08 11:04 ` [PATCH 7/9] KEYS: Permit in-place link replacement in keyring list David Howells
2012-03-19 14:44   ` [Keyrings] " Jeff Layton
2012-03-19 15:39   ` David Howells
2012-02-08 11:04 ` [PATCH 8/9] KEYS: Do LRU discard in full keyrings David Howells
2012-02-08 11:04 ` [PATCH 9/9] KEYS: Add invalidation support David Howells
2012-03-28 10:46 [PATCH 1/9] KEYS: Use the compat keyctl() syscall wrapper on Sparc64 for Sparc32 compat David Howells
2012-03-28 10:46 ` [PATCH 5/9] KEYS: Announce key type (un)registration David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).