Netdev List
 help / color / mirror / Atom feed
* [PATCH 8/9] keys: Network namespace domain tag [ver #4]
From: David Howells @ 2019-06-19 16:47 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096279115.28733.8761881995303698232.stgit@warthog.procyon.org.uk>

Create key domain tags for network namespaces and make it possible to
automatically tag keys that are used by networked services (e.g. AF_RXRPC,
AFS, DNS) with the default network namespace if not set by the caller.

This allows keys with the same description but in different namespaces to
coexist within a keyring.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: netdev@vger.kernel.org
cc: linux-nfs@vger.kernel.org
cc: linux-cifs@vger.kernel.org
cc: linux-afs@lists.infradead.org
---

 include/linux/key-type.h    |    3 +++
 include/net/net_namespace.h |    3 +++
 net/core/net_namespace.c    |   19 +++++++++++++++++++
 net/dns_resolver/dns_key.c  |    1 +
 net/rxrpc/key.c             |    2 ++
 security/keys/keyring.c     |    7 ++++++-
 6 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index e49d1de0614e..2148a6bf58f1 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -74,6 +74,9 @@ struct key_type {
 	 */
 	size_t def_datalen;
 
+	unsigned int flags;
+#define KEY_TYPE_NET_DOMAIN	0x00000001 /* Keys of this type have a net namespace domain */
+
 	/* vet a description */
 	int (*vet_description)(const char *description);
 
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 12689ddfc24c..a56bf7fc7c2b 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -71,6 +71,9 @@ struct net {
 						 */
 	struct llist_node	cleanup_list;	/* namespaces on death row */
 
+#ifdef CONFIG_KEYS
+	struct key_tag		*key_domain;	/* Key domain of operation tag */
+#endif
 	struct user_namespace   *user_ns;	/* Owning user namespace */
 	struct ucounts		*ucounts;
 	spinlock_t		nsid_lock;
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 711b161505ac..076a75c73c9e 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -38,9 +38,16 @@ EXPORT_SYMBOL_GPL(net_namespace_list);
 DECLARE_RWSEM(net_rwsem);
 EXPORT_SYMBOL_GPL(net_rwsem);
 
+#ifdef CONFIG_KEYS
+static struct key_tag init_net_key_domain = { .usage = REFCOUNT_INIT(1) };
+#endif
+
 struct net init_net = {
 	.count		= REFCOUNT_INIT(1),
 	.dev_base_head	= LIST_HEAD_INIT(init_net.dev_base_head),
+#ifdef CONFIG_KEYS
+	.key_domain	= &init_net_key_domain,
+#endif
 };
 EXPORT_SYMBOL(init_net);
 
@@ -386,10 +393,21 @@ static struct net *net_alloc(void)
 	if (!net)
 		goto out_free;
 
+#ifdef CONFIG_KEYS
+	net->key_domain = kzalloc(sizeof(struct key_tag), GFP_KERNEL);
+	if (!net->key_domain)
+		goto out_free_2;
+	refcount_set(&net->key_domain->usage, 1);
+#endif
+
 	rcu_assign_pointer(net->gen, ng);
 out:
 	return net;
 
+#ifdef CONFIG_KEYS
+out_free_2:
+	kmem_cache_free(net_cachep, net);
+#endif
 out_free:
 	kfree(ng);
 	goto out;
@@ -566,6 +584,7 @@ static void cleanup_net(struct work_struct *work)
 	list_for_each_entry_safe(net, tmp, &net_exit_list, exit_list) {
 		list_del_init(&net->exit_list);
 		dec_net_namespaces(net->ucounts);
+		key_remove_domain(net->key_domain);
 		put_user_ns(net->user_ns);
 		net_drop_ns(net);
 	}
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index a65d553e730d..3e1a90669006 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -314,6 +314,7 @@ static long dns_resolver_read(const struct key *key,
 
 struct key_type key_type_dns_resolver = {
 	.name		= "dns_resolver",
+	.flags		= KEY_TYPE_NET_DOMAIN,
 	.preparse	= dns_resolver_preparse,
 	.free_preparse	= dns_resolver_free_preparse,
 	.instantiate	= generic_key_instantiate,
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index e7f6b8823eb6..2722189ec273 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -43,6 +43,7 @@ static long rxrpc_read(const struct key *, char __user *, size_t);
  */
 struct key_type key_type_rxrpc = {
 	.name		= "rxrpc",
+	.flags		= KEY_TYPE_NET_DOMAIN,
 	.preparse	= rxrpc_preparse,
 	.free_preparse	= rxrpc_free_preparse,
 	.instantiate	= generic_key_instantiate,
@@ -58,6 +59,7 @@ EXPORT_SYMBOL(key_type_rxrpc);
  */
 struct key_type key_type_rxrpc_s = {
 	.name		= "rxrpc_s",
+	.flags		= KEY_TYPE_NET_DOMAIN,
 	.vet_description = rxrpc_vet_description_s,
 	.preparse	= rxrpc_preparse_s,
 	.free_preparse	= rxrpc_free_preparse_s,
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index d3c86fda1510..bca070f6ab46 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -17,10 +17,12 @@
 #include <linux/seq_file.h>
 #include <linux/err.h>
 #include <linux/user_namespace.h>
+#include <linux/nsproxy.h>
 #include <keys/keyring-type.h>
 #include <keys/user-type.h>
 #include <linux/assoc_array_priv.h>
 #include <linux/uaccess.h>
+#include <net/net_namespace.h>
 #include "internal.h"
 
 /*
@@ -220,7 +222,10 @@ void key_set_index_key(struct keyring_index_key *index_key)
 
 	memcpy(index_key->desc, index_key->description, n);
 
-	index_key->domain_tag = &default_domain_tag;
+	if (index_key->type->flags & KEY_TYPE_NET_DOMAIN)
+		index_key->domain_tag = current->nsproxy->net_ns->key_domain;
+	else
+		index_key->domain_tag = &default_domain_tag;
 	hash_key_type_and_desc(index_key);
 }
 


^ permalink raw reply related

* [PATCH 7/9] keys: Garbage collect keys for which the domain has been removed [ver #4]
From: David Howells @ 2019-06-19 16:47 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096279115.28733.8761881995303698232.stgit@warthog.procyon.org.uk>

If a key operation domain (such as a network namespace) has been removed
then attempt to garbage collect all the keys that use it.

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

 include/linux/key.h      |    2 ++
 security/keys/internal.h |    3 ++-
 security/keys/keyring.c  |   15 +++++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index abc68555bac3..60c076c6e47f 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -278,6 +278,7 @@ extern void key_revoke(struct key *key);
 extern void key_invalidate(struct key *key);
 extern void key_put(struct key *key);
 extern bool key_put_tag(struct key_tag *tag);
+extern void key_remove_domain(struct key_tag *domain_tag);
 
 static inline struct key *__key_get(struct key *key)
 {
@@ -446,6 +447,7 @@ extern void key_init(void);
 #define key_fsgid_changed(c)		do { } while(0)
 #define key_init()			do { } while(0)
 #define key_free_user_ns(ns)		do { } while(0)
+#define key_remove_domain(d)		do { } while(0)
 
 #endif /* CONFIG_KEYS */
 #endif /* __KERNEL__ */
diff --git a/security/keys/internal.h b/security/keys/internal.h
index d3a9439e2386..5a561f5f199e 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -209,7 +209,8 @@ static inline bool key_is_dead(const struct key *key, time64_t limit)
 	return
 		key->flags & ((1 << KEY_FLAG_DEAD) |
 			      (1 << KEY_FLAG_INVALIDATED)) ||
-		(key->expiry > 0 && key->expiry <= limit);
+		(key->expiry > 0 && key->expiry <= limit) ||
+		key->domain_tag->removed;
 }
 
 /*
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 0da8fa282d56..d3c86fda1510 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -241,6 +241,21 @@ bool key_put_tag(struct key_tag *tag)
 	return false;
 }
 
+/**
+ * key_remove_domain - Kill off a key domain and gc its keys
+ * @domain_tag: The domain tag to release.
+ *
+ * This marks a domain tag as being dead and releases a ref on it.  If that
+ * wasn't the last reference, the garbage collector is poked to try and delete
+ * all keys that were in the domain.
+ */
+void key_remove_domain(struct key_tag *domain_tag)
+{
+	domain_tag->removed = true;
+	if (!key_put_tag(domain_tag))
+		key_schedule_gc_links();
+}
+
 /*
  * Build the next index key chunk.
  *


^ permalink raw reply related

* [PATCH 6/9] keys: Include target namespace in match criteria [ver #4]
From: David Howells @ 2019-06-19 16:47 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096279115.28733.8761881995303698232.stgit@warthog.procyon.org.uk>

Currently a key has a standard matching criteria of { type, description }
and this is used to only allow keys with unique criteria in a keyring.
This means, however, that you cannot have keys with the same type and
description but a different target namespace in the same keyring.

This is a potential problem for a containerised environment where, say, a
container is made up of some parts of its mount space involving netfs
superblocks from two different network namespaces.

This is also a problem for shared system management keyrings such as the
DNS records keyring or the NFS idmapper keyring that might contain keys
from different network namespaces.

Fix this by including a namespace component in a key's matching criteria.
Keyring types are marked to indicate which, if any, namespace is relevant
to keys of that type, and that namespace is set when the key is created
from the current task's namespace set.

The capability bit KEYCTL_CAPS1_NS_KEY_TAG is set if the kernel is
employing this feature.

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

 include/linux/key.h         |   10 ++++++++++
 include/uapi/linux/keyctl.h |    1 +
 security/keys/gc.c          |    2 +-
 security/keys/key.c         |    1 +
 security/keys/keyctl.c      |    3 ++-
 security/keys/keyring.c     |   36 ++++++++++++++++++++++++++++++++++--
 security/keys/persistent.c  |    1 +
 7 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index ae1177302d70..abc68555bac3 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -82,9 +82,16 @@ struct cred;
 
 struct key_type;
 struct key_owner;
+struct key_tag;
 struct keyring_list;
 struct keyring_name;
 
+struct key_tag {
+	struct rcu_head		rcu;
+	refcount_t		usage;
+	bool			removed;	/* T when subject removed */
+};
+
 struct keyring_index_key {
 	/* [!] If this structure is altered, the union in struct key must change too! */
 	unsigned long		hash;			/* Hash value */
@@ -101,6 +108,7 @@ struct keyring_index_key {
 		unsigned long x;
 	};
 	struct key_type		*type;
+	struct key_tag		*domain_tag;	/* Domain of operation */
 	const char		*description;
 };
 
@@ -218,6 +226,7 @@ struct key {
 			unsigned long	hash;
 			unsigned long	len_desc;
 			struct key_type	*type;		/* type of key */
+			struct key_tag	*domain_tag;	/* Domain of operation */
 			char		*description;
 		};
 	};
@@ -268,6 +277,7 @@ extern struct key *key_alloc(struct key_type *type,
 extern void key_revoke(struct key *key);
 extern void key_invalidate(struct key *key);
 extern void key_put(struct key *key);
+extern bool key_put_tag(struct key_tag *tag);
 
 static inline struct key *__key_get(struct key *key)
 {
diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index 35b405034674..3bb5324d514f 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -129,5 +129,6 @@ struct keyctl_pkey_params {
 #define KEYCTL_CAPS0_RESTRICT_KEYRING	0x40 /* KEYCTL_RESTRICT_KEYRING supported */
 #define KEYCTL_CAPS0_MOVE		0x80 /* KEYCTL_MOVE supported */
 #define KEYCTL_CAPS1_NS_KEYRING_NAME	0x01 /* Keyring names are per-user_namespace */
+#define KEYCTL_CAPS2_NS_KEY_TAG		0x02 /* Key indexing can include a namespace tag */
 
 #endif /*  _LINUX_KEYCTL_H */
diff --git a/security/keys/gc.c b/security/keys/gc.c
index 634e96b380e8..83d279fb7793 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -154,7 +154,7 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 			atomic_dec(&key->user->nikeys);
 
 		key_user_put(key->user);
-
+		key_put_tag(key->domain_tag);
 		kfree(key->description);
 
 		memzero_explicit(key, sizeof(*key));
diff --git a/security/keys/key.c b/security/keys/key.c
index 9d52f2472a09..85fdc2ea6c14 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -317,6 +317,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 		goto security_error;
 
 	/* publish the key by giving it a serial number */
+	refcount_inc(&key->domain_tag->usage);
 	atomic_inc(&user->nkeys);
 	key_alloc_serial(key);
 
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 8a813220f269..aa9be531e5f5 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -40,7 +40,8 @@ static const unsigned char keyrings_capabilities[2] = {
 	       KEYCTL_CAPS0_RESTRICT_KEYRING |
 	       KEYCTL_CAPS0_MOVE
 	       ),
-	[1] = (KEYCTL_CAPS1_NS_KEYRING_NAME),
+	[1] = (KEYCTL_CAPS1_NS_KEYRING_NAME |
+	       KEYCTL_CAPS2_NS_KEY_TAG),
 };
 
 static int key_get_type_from_user(char *type,
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 3663e5168583..0da8fa282d56 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -175,6 +175,9 @@ static void hash_key_type_and_desc(struct keyring_index_key *index_key)
 	type = (unsigned long)index_key->type;
 	acc = mult_64x32_and_fold(type, desc_len + 13);
 	acc = mult_64x32_and_fold(acc, 9207);
+	piece = (unsigned long)index_key->domain_tag;
+	acc = mult_64x32_and_fold(acc, piece);
+	acc = mult_64x32_and_fold(acc, 9207);
 
 	for (;;) {
 		n = desc_len;
@@ -208,16 +211,36 @@ static void hash_key_type_and_desc(struct keyring_index_key *index_key)
 
 /*
  * Finalise an index key to include a part of the description actually in the
- * index key and to add in the hash too.
+ * index key, to set the domain tag and to calculate the hash.
  */
 void key_set_index_key(struct keyring_index_key *index_key)
 {
+	static struct key_tag default_domain_tag = { .usage = REFCOUNT_INIT(1), };
 	size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
+
 	memcpy(index_key->desc, index_key->description, n);
 
+	index_key->domain_tag = &default_domain_tag;
 	hash_key_type_and_desc(index_key);
 }
 
+/**
+ * key_put_tag - Release a ref on a tag.
+ * @tag: The tag to release.
+ *
+ * This releases a reference the given tag and returns true if that ref was the
+ * last one.
+ */
+bool key_put_tag(struct key_tag *tag)
+{
+	if (refcount_dec_and_test(&tag->usage)) {
+		kfree_rcu(tag, rcu);
+		return true;
+	}
+
+	return false;
+}
+
 /*
  * Build the next index key chunk.
  *
@@ -238,8 +261,10 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
 		return index_key->x;
 	case 2:
 		return (unsigned long)index_key->type;
+	case 3:
+		return (unsigned long)index_key->domain_tag;
 	default:
-		level -= 3;
+		level -= 4;
 		if (desc_len <= sizeof(index_key->desc))
 			return 0;
 
@@ -268,6 +293,7 @@ static bool keyring_compare_object(const void *object, const void *data)
 	const struct key *key = keyring_ptr_to_key(object);
 
 	return key->index_key.type == index_key->type &&
+		key->index_key.domain_tag == index_key->domain_tag &&
 		key->index_key.desc_len == index_key->desc_len &&
 		memcmp(key->index_key.description, index_key->description,
 		       index_key->desc_len) == 0;
@@ -309,6 +335,12 @@ static int keyring_diff_objects(const void *object, const void *data)
 		goto differ;
 	level += sizeof(unsigned long);
 
+	seg_a = (unsigned long)a->domain_tag;
+	seg_b = (unsigned long)b->domain_tag;
+	if ((seg_a ^ seg_b) != 0)
+		goto differ;
+	level += sizeof(unsigned long);
+
 	i = sizeof(a->desc);
 	if (a->desc_len <= i)
 		goto same;
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index 90303fe4a394..9944d855a28d 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -84,6 +84,7 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
 	long ret;
 
 	/* Look in the register if it exists */
+	memset(&index_key, 0, sizeof(index_key));
 	index_key.type = &key_type_keyring;
 	index_key.description = buf;
 	index_key.desc_len = sprintf(buf, "_persistent.%u", from_kuid(ns, uid));


^ permalink raw reply related

* [PATCH 5/9] keys: Move the user and user-session keyrings to the user_namespace [ver #4]
From: David Howells @ 2019-06-19 16:47 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096279115.28733.8761881995303698232.stgit@warthog.procyon.org.uk>

Move the user and user-session keyrings to the user_namespace struct rather
than pinning them from the user_struct struct.  This prevents these
keyrings from propagating across user-namespaces boundaries with regard to
the KEY_SPEC_* flags, thereby making them more useful in a containerised
environment.

The issue is that a single user_struct may be represent UIDs in several
different namespaces.

The way the patch does this is by attaching a 'register keyring' in each
user_namespace and then sticking the user and user-session keyrings into
that.  It can then be searched to retrieve them.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jann Horn <jannh@google.com>
---

 include/linux/sched/user.h     |   14 --
 include/linux/user_namespace.h |    9 +
 kernel/user.c                  |    7 -
 kernel/user_namespace.c        |    4 -
 security/keys/internal.h       |    3 
 security/keys/keyring.c        |    1 
 security/keys/persistent.c     |    8 +
 security/keys/process_keys.c   |  259 ++++++++++++++++++++++++++--------------
 security/keys/request_key.c    |   20 ++-
 9 files changed, 196 insertions(+), 129 deletions(-)

diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h
index 468d2565a9fe..917d88edb7b9 100644
--- a/include/linux/sched/user.h
+++ b/include/linux/sched/user.h
@@ -7,8 +7,6 @@
 #include <linux/refcount.h>
 #include <linux/ratelimit.h>
 
-struct key;
-
 /*
  * Some day this will be a full-fledged user tracking system..
  */
@@ -30,18 +28,6 @@ struct user_struct {
 	unsigned long unix_inflight;	/* How many files in flight in unix sockets */
 	atomic_long_t pipe_bufs;  /* how many pages are allocated in pipe buffers */
 
-#ifdef CONFIG_KEYS
-	/*
-	 * These pointers can only change from NULL to a non-NULL value once.
-	 * Writes are protected by key_user_keyring_mutex.
-	 * Unlocked readers should use READ_ONCE() unless they know that
-	 * install_user_keyrings() has been called successfully (which sets
-	 * these members to non-NULL values, preventing further modifications).
-	 */
-	struct key *uid_keyring;	/* UID specific keyring */
-	struct key *session_keyring;	/* UID's default session keyring */
-#endif
-
 	/* Hash table maintenance information */
 	struct hlist_node uidhash_node;
 	kuid_t uid;
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 90457015fa3f..fb9f4f799554 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -65,14 +65,19 @@ struct user_namespace {
 	unsigned long		flags;
 
 #ifdef CONFIG_KEYS
-	/* List of joinable keyrings in this namespace */
+	/* List of joinable keyrings in this namespace.  Modification access of
+	 * these pointers is controlled by keyring_sem.  Once
+	 * user_keyring_register is set, it won't be changed, so it can be
+	 * accessed directly with READ_ONCE().
+	 */
 	struct list_head	keyring_name_list;
+	struct key		*user_keyring_register;
+	struct rw_semaphore	keyring_sem;
 #endif
 
 	/* Register of per-UID persistent keyrings for this namespace */
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	struct key		*persistent_keyring_register;
-	struct rw_semaphore	persistent_keyring_register_sem;
 #endif
 	struct work_struct	work;
 #ifdef CONFIG_SYSCTL
diff --git a/kernel/user.c b/kernel/user.c
index 50979fd1b7aa..f8519b62cf9a 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -64,10 +64,7 @@ struct user_namespace init_user_ns = {
 	.flags = USERNS_INIT_FLAGS,
 #ifdef CONFIG_KEYS
 	.keyring_name_list = LIST_HEAD_INIT(init_user_ns.keyring_name_list),
-#endif
-#ifdef CONFIG_PERSISTENT_KEYRINGS
-	.persistent_keyring_register_sem =
-	__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
+	.keyring_sem = __RWSEM_INITIALIZER(init_user_ns.keyring_sem),
 #endif
 };
 EXPORT_SYMBOL_GPL(init_user_ns);
@@ -143,8 +140,6 @@ static void free_user(struct user_struct *up, unsigned long flags)
 {
 	uid_hash_remove(up);
 	spin_unlock_irqrestore(&uidhash_lock, flags);
-	key_put(up->uid_keyring);
-	key_put(up->session_keyring);
 	kmem_cache_free(uid_cachep, up);
 }
 
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index bda6e890ad88..c87c2ecc7085 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -135,9 +135,7 @@ int create_user_ns(struct cred *new)
 
 #ifdef CONFIG_KEYS
 	INIT_LIST_HEAD(&ns->keyring_name_list);
-#endif
-#ifdef CONFIG_PERSISTENT_KEYRINGS
-	init_rwsem(&ns->persistent_keyring_register_sem);
+	init_rwsem(&ns->keyring_sem);
 #endif
 	ret = -ENOMEM;
 	if (!setup_userns_sysctls(ns))
diff --git a/security/keys/internal.h b/security/keys/internal.h
index aa361299a3ec..d3a9439e2386 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -148,7 +148,8 @@ extern key_ref_t search_process_keyrings_rcu(struct keyring_search_context *ctx)
 
 extern struct key *find_keyring_by_name(const char *name, bool uid_keyring);
 
-extern int install_user_keyrings(void);
+extern int look_up_user_keyrings(struct key **, struct key **);
+extern struct key *get_user_session_keyring_rcu(const struct cred *);
 extern int install_thread_keyring_to_cred(struct cred *);
 extern int install_process_keyring_to_cred(struct cred *);
 extern int install_session_keyring_to_cred(struct cred *, struct key *);
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index fe851292509e..3663e5168583 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -62,6 +62,7 @@ void key_free_user_ns(struct user_namespace *ns)
 	list_del_init(&ns->keyring_name_list);
 	write_unlock(&keyring_name_lock);
 
+	key_put(ns->user_keyring_register);
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	key_put(ns->persistent_keyring_register);
 #endif
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index fc29ec59efa7..90303fe4a394 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -91,9 +91,9 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
 
 	if (ns->persistent_keyring_register) {
 		reg_ref = make_key_ref(ns->persistent_keyring_register, true);
-		down_read(&ns->persistent_keyring_register_sem);
+		down_read(&ns->keyring_sem);
 		persistent_ref = find_key_to_update(reg_ref, &index_key);
-		up_read(&ns->persistent_keyring_register_sem);
+		up_read(&ns->keyring_sem);
 
 		if (persistent_ref)
 			goto found;
@@ -102,9 +102,9 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
 	/* It wasn't in the register, so we'll need to create it.  We might
 	 * also need to create the register.
 	 */
-	down_write(&ns->persistent_keyring_register_sem);
+	down_write(&ns->keyring_sem);
 	persistent_ref = key_create_persistent(ns, uid, &index_key);
-	up_write(&ns->persistent_keyring_register_sem);
+	up_write(&ns->keyring_sem);
 	if (!IS_ERR(persistent_ref))
 		goto found;
 
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index b07f768d23dc..f74d64215942 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -19,15 +19,13 @@
 #include <linux/security.h>
 #include <linux/user_namespace.h>
 #include <linux/uaccess.h>
+#include <linux/init_task.h>
 #include <keys/request_key_auth-type.h>
 #include "internal.h"
 
 /* Session keyring create vs join semaphore */
 static DEFINE_MUTEX(key_session_mutex);
 
-/* User keyring creation semaphore */
-static DEFINE_MUTEX(key_user_keyring_mutex);
-
 /* The root user's tracking struct */
 struct key_user root_key_user = {
 	.usage		= REFCOUNT_INIT(3),
@@ -39,98 +37,185 @@ struct key_user root_key_user = {
 };
 
 /*
- * Install the user and user session keyrings for the current process's UID.
+ * Get or create a user register keyring.
+ */
+static struct key *get_user_register(struct user_namespace *user_ns)
+{
+	struct key *reg_keyring = READ_ONCE(user_ns->user_keyring_register);
+
+	if (reg_keyring)
+		return reg_keyring;
+
+	down_write(&user_ns->keyring_sem);
+
+	/* Make sure there's a register keyring.  It gets owned by the
+	 * user_namespace's owner.
+	 */
+	reg_keyring = user_ns->user_keyring_register;
+	if (!reg_keyring) {
+		reg_keyring = keyring_alloc(".user_reg",
+					    user_ns->owner, INVALID_GID,
+					    &init_cred,
+					    KEY_POS_WRITE | KEY_POS_SEARCH |
+					    KEY_USR_VIEW | KEY_USR_READ,
+					    0,
+					    NULL, NULL);
+		if (!IS_ERR(reg_keyring))
+			smp_store_release(&user_ns->user_keyring_register,
+					  reg_keyring);
+	}
+
+	up_write(&user_ns->keyring_sem);
+
+	/* We don't return a ref since the keyring is pinned by the user_ns */
+	return reg_keyring;
+}
+
+/*
+ * Look up the user and user session keyrings for the current process's UID,
+ * creating them if they don't exist.
  */
-int install_user_keyrings(void)
+int look_up_user_keyrings(struct key **_user_keyring,
+			  struct key **_user_session_keyring)
 {
-	struct user_struct *user;
-	const struct cred *cred;
-	struct key *uid_keyring, *session_keyring;
+	const struct cred *cred = current_cred();
+	struct user_namespace *user_ns = current_user_ns();
+	struct key *reg_keyring, *uid_keyring, *session_keyring;
 	key_perm_t user_keyring_perm;
+	key_ref_t uid_keyring_r, session_keyring_r;
+	uid_t uid = from_kuid(user_ns, cred->user->uid);
 	char buf[20];
 	int ret;
-	uid_t uid;
 
 	user_keyring_perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_ALL;
-	cred = current_cred();
-	user = cred->user;
-	uid = from_kuid(cred->user_ns, user->uid);
 
-	kenter("%p{%u}", user, uid);
+	kenter("%u", uid);
 
-	if (READ_ONCE(user->uid_keyring) && READ_ONCE(user->session_keyring)) {
-		kleave(" = 0 [exist]");
-		return 0;
-	}
+	reg_keyring = get_user_register(user_ns);
+	if (IS_ERR(reg_keyring))
+		return PTR_ERR(reg_keyring);
 
-	mutex_lock(&key_user_keyring_mutex);
+	down_write(&user_ns->keyring_sem);
 	ret = 0;
 
-	if (!user->uid_keyring) {
-		/* get the UID-specific keyring
-		 * - there may be one in existence already as it may have been
-		 *   pinned by a session, but the user_struct pointing to it
-		 *   may have been destroyed by setuid */
-		sprintf(buf, "_uid.%u", uid);
-
-		uid_keyring = find_keyring_by_name(buf, true);
+	/* Get the user keyring.  Note that there may be one in existence
+	 * already as it may have been pinned by a session, but the user_struct
+	 * pointing to it may have been destroyed by setuid.
+	 */
+	snprintf(buf, sizeof(buf), "_uid.%u", uid);
+	uid_keyring_r = keyring_search(make_key_ref(reg_keyring, true),
+				       &key_type_keyring, buf, false);
+	kdebug("_uid %p", uid_keyring_r);
+	if (uid_keyring_r == ERR_PTR(-EAGAIN)) {
+		uid_keyring = keyring_alloc(buf, cred->user->uid, INVALID_GID,
+					    cred, user_keyring_perm,
+					    KEY_ALLOC_UID_KEYRING |
+					    KEY_ALLOC_IN_QUOTA,
+					    NULL, reg_keyring);
 		if (IS_ERR(uid_keyring)) {
-			uid_keyring = keyring_alloc(buf, user->uid, INVALID_GID,
-						    cred, user_keyring_perm,
-						    KEY_ALLOC_UID_KEYRING |
-							KEY_ALLOC_IN_QUOTA,
-						    NULL, NULL);
-			if (IS_ERR(uid_keyring)) {
-				ret = PTR_ERR(uid_keyring);
-				goto error;
-			}
+			ret = PTR_ERR(uid_keyring);
+			goto error;
 		}
+	} else if (IS_ERR(uid_keyring_r)) {
+		ret = PTR_ERR(uid_keyring_r);
+		goto error;
+	} else {
+		uid_keyring = key_ref_to_ptr(uid_keyring_r);
+	}
 
-		/* get a default session keyring (which might also exist
-		 * already) */
-		sprintf(buf, "_uid_ses.%u", uid);
-
-		session_keyring = find_keyring_by_name(buf, true);
+	/* Get a default session keyring (which might also exist already) */
+	snprintf(buf, sizeof(buf), "_uid_ses.%u", uid);
+	session_keyring_r = keyring_search(make_key_ref(reg_keyring, true),
+					   &key_type_keyring, buf, false);
+	kdebug("_uid_ses %p", session_keyring_r);
+	if (session_keyring_r == ERR_PTR(-EAGAIN)) {
+		session_keyring = keyring_alloc(buf, cred->user->uid, INVALID_GID,
+						cred, user_keyring_perm,
+						KEY_ALLOC_UID_KEYRING |
+						KEY_ALLOC_IN_QUOTA,
+						NULL, NULL);
 		if (IS_ERR(session_keyring)) {
-			session_keyring =
-				keyring_alloc(buf, user->uid, INVALID_GID,
-					      cred, user_keyring_perm,
-					      KEY_ALLOC_UID_KEYRING |
-						  KEY_ALLOC_IN_QUOTA,
-					      NULL, NULL);
-			if (IS_ERR(session_keyring)) {
-				ret = PTR_ERR(session_keyring);
-				goto error_release;
-			}
-
-			/* we install a link from the user session keyring to
-			 * the user keyring */
-			ret = key_link(session_keyring, uid_keyring);
-			if (ret < 0)
-				goto error_release_both;
+			ret = PTR_ERR(session_keyring);
+			goto error_release;
 		}
 
-		/* install the keyrings */
-		/* paired with READ_ONCE() */
-		smp_store_release(&user->uid_keyring, uid_keyring);
-		/* paired with READ_ONCE() */
-		smp_store_release(&user->session_keyring, session_keyring);
+		/* We install a link from the user session keyring to
+		 * the user keyring.
+		 */
+		ret = key_link(session_keyring, uid_keyring);
+		if (ret < 0)
+			goto error_release_session;
+
+		/* And only then link the user-session keyring to the
+		 * register.
+		 */
+		ret = key_link(reg_keyring, session_keyring);
+		if (ret < 0)
+			goto error_release_session;
+	} else if (IS_ERR(session_keyring_r)) {
+		ret = PTR_ERR(session_keyring_r);
+		goto error_release;
+	} else {
+		session_keyring = key_ref_to_ptr(session_keyring_r);
 	}
 
-	mutex_unlock(&key_user_keyring_mutex);
+	up_write(&user_ns->keyring_sem);
+
+	if (_user_session_keyring)
+		*_user_session_keyring = session_keyring;
+	else
+		key_put(session_keyring);
+	if (_user_keyring)
+		*_user_keyring = uid_keyring;
+	else
+		key_put(uid_keyring);
 	kleave(" = 0");
 	return 0;
 
-error_release_both:
+error_release_session:
 	key_put(session_keyring);
 error_release:
 	key_put(uid_keyring);
 error:
-	mutex_unlock(&key_user_keyring_mutex);
+	up_write(&user_ns->keyring_sem);
 	kleave(" = %d", ret);
 	return ret;
 }
 
+/*
+ * Get the user session keyring if it exists, but don't create it if it
+ * doesn't.
+ */
+struct key *get_user_session_keyring_rcu(const struct cred *cred)
+{
+	struct key *reg_keyring = READ_ONCE(cred->user_ns->user_keyring_register);
+	key_ref_t session_keyring_r;
+	char buf[20];
+
+	struct keyring_search_context ctx = {
+		.index_key.type		= &key_type_keyring,
+		.index_key.description	= buf,
+		.cred			= cred,
+		.match_data.cmp		= key_default_cmp,
+		.match_data.raw_data	= buf,
+		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
+	};
+
+	if (!reg_keyring)
+		return NULL;
+
+	ctx.index_key.desc_len = snprintf(buf, sizeof(buf), "_uid_ses.%u",
+					  from_kuid(cred->user_ns,
+						    cred->user->uid));
+
+	session_keyring_r = keyring_search_rcu(make_key_ref(reg_keyring, true),
+					       &ctx);
+	if (IS_ERR(session_keyring_r))
+		return NULL;
+	return key_ref_to_ptr(session_keyring_r);
+}
+
 /*
  * Install a thread keyring to the given credentials struct if it didn't have
  * one already.  This is allowed to overrun the quota.
@@ -340,6 +425,7 @@ void key_fsgid_changed(struct cred *new_cred)
  */
 key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx)
 {
+	struct key *user_session;
 	key_ref_t key_ref, ret, err;
 	const struct cred *cred = ctx->cred;
 
@@ -415,10 +501,11 @@ key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx)
 		}
 	}
 	/* or search the user-session keyring */
-	else if (READ_ONCE(cred->user->session_keyring)) {
-		key_ref = keyring_search_rcu(
-			make_key_ref(READ_ONCE(cred->user->session_keyring), 1),
-			ctx);
+	else if ((user_session = get_user_session_keyring_rcu(cred))) {
+		key_ref = keyring_search_rcu(make_key_ref(user_session, 1),
+					     ctx);
+		key_put(user_session);
+
 		if (!IS_ERR(key_ref))
 			goto found;
 
@@ -535,7 +622,7 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 					   KEYRING_SEARCH_RECURSE),
 	};
 	struct request_key_auth *rka;
-	struct key *key;
+	struct key *key, *user_session;
 	key_ref_t key_ref, skey_ref;
 	int ret;
 
@@ -584,20 +671,20 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 		if (!ctx.cred->session_keyring) {
 			/* always install a session keyring upon access if one
 			 * doesn't exist yet */
-			ret = install_user_keyrings();
+			ret = look_up_user_keyrings(NULL, &user_session);
 			if (ret < 0)
 				goto error;
 			if (lflags & KEY_LOOKUP_CREATE)
 				ret = join_session_keyring(NULL);
 			else
-				ret = install_session_keyring(
-					ctx.cred->user->session_keyring);
+				ret = install_session_keyring(user_session);
 
+			key_put(user_session);
 			if (ret < 0)
 				goto error;
 			goto reget_creds;
-		} else if (ctx.cred->session_keyring ==
-			   READ_ONCE(ctx.cred->user->session_keyring) &&
+		} else if (test_bit(KEY_FLAG_UID_KEYRING,
+				    &ctx.cred->session_keyring->flags) &&
 			   lflags & KEY_LOOKUP_CREATE) {
 			ret = join_session_keyring(NULL);
 			if (ret < 0)
@@ -611,26 +698,16 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 		break;
 
 	case KEY_SPEC_USER_KEYRING:
-		if (!READ_ONCE(ctx.cred->user->uid_keyring)) {
-			ret = install_user_keyrings();
-			if (ret < 0)
-				goto error;
-		}
-
-		key = ctx.cred->user->uid_keyring;
-		__key_get(key);
+		ret = look_up_user_keyrings(&key, NULL);
+		if (ret < 0)
+			goto error;
 		key_ref = make_key_ref(key, 1);
 		break;
 
 	case KEY_SPEC_USER_SESSION_KEYRING:
-		if (!READ_ONCE(ctx.cred->user->session_keyring)) {
-			ret = install_user_keyrings();
-			if (ret < 0)
-				goto error;
-		}
-
-		key = ctx.cred->user->session_keyring;
-		__key_get(key);
+		ret = look_up_user_keyrings(NULL, &key);
+		if (ret < 0)
+			goto error;
 		key_ref = make_key_ref(key, 1);
 		break;
 
@@ -879,7 +956,7 @@ void key_change_session_keyring(struct callback_head *twork)
  */
 static int __init init_root_keyring(void)
 {
-	return install_user_keyrings();
+	return look_up_user_keyrings(NULL, NULL);
 }
 
 late_initcall(init_root_keyring);
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 1ffd3803ce29..9201ca96c4df 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -121,7 +121,7 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
 	struct request_key_auth *rka = get_request_key_auth(authkey);
 	const struct cred *cred = current_cred();
 	key_serial_t prkey, sskey;
-	struct key *key = rka->target_key, *keyring, *session;
+	struct key *key = rka->target_key, *keyring, *session, *user_session;
 	char *argv[9], *envp[3], uid_str[12], gid_str[12];
 	char key_str[12], keyring_str[3][12];
 	char desc[20];
@@ -129,9 +129,9 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
 
 	kenter("{%d},{%d},%s", key->serial, authkey->serial, rka->op);
 
-	ret = install_user_keyrings();
+	ret = look_up_user_keyrings(NULL, &user_session);
 	if (ret < 0)
-		goto error_alloc;
+		goto error_us;
 
 	/* allocate a new session keyring */
 	sprintf(desc, "_req.%u", key->serial);
@@ -169,7 +169,7 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
 
 	session = cred->session_keyring;
 	if (!session)
-		session = cred->user->session_keyring;
+		session = user_session;
 	sskey = session->serial;
 
 	sprintf(keyring_str[2], "%d", sskey);
@@ -211,6 +211,8 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
 	key_put(keyring);
 
 error_alloc:
+	key_put(user_session);
+error_us:
 	complete_request_key(authkey, ret);
 	kleave(" = %d", ret);
 	return ret;
@@ -317,13 +319,15 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
 
 			/* fall through */
 		case KEY_REQKEY_DEFL_USER_SESSION_KEYRING:
-			dest_keyring =
-				key_get(READ_ONCE(cred->user->session_keyring));
+			ret = look_up_user_keyrings(NULL, &dest_keyring);
+			if (ret < 0)
+				return ret;
 			break;
 
 		case KEY_REQKEY_DEFL_USER_KEYRING:
-			dest_keyring =
-				key_get(READ_ONCE(cred->user->uid_keyring));
+			ret = look_up_user_keyrings(&dest_keyring, NULL);
+			if (ret < 0)
+				return ret;
 			break;
 
 		case KEY_REQKEY_DEFL_GROUP_KEYRING:


^ permalink raw reply related

* [PATCH 4/9] keys: Namespace keyring names [ver #4]
From: David Howells @ 2019-06-19 16:47 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096279115.28733.8761881995303698232.stgit@warthog.procyon.org.uk>

Keyring names are held in a single global list that any process can pick
from by means of keyctl_join_session_keyring (provided the keyring grants
Search permission).  This isn't very container friendly, however.

Make the following changes:

 (1) Make default session, process and thread keyring names begin with a
     '.' instead of '_'.

 (2) Keyrings whose names begin with a '.' aren't added to the list.  Such
     keyrings are system specials.

 (3) Replace the global list with per-user_namespace lists.  A keyring adds
     its name to the list for the user_namespace that it is currently in.

 (4) When a user_namespace is deleted, it just removes itself from the
     keyring name list.

The global keyring_name_lock is retained for accessing the name lists.
This allows (4) to work.

This can be tested by:

	# keyctl newring foo @s
	995906392
	# unshare -U
	$ keyctl show
	...
	 995906392 --alswrv  65534 65534   \_ keyring: foo
	...
	$ keyctl session foo
	Joined session keyring: 935622349

As can be seen, a new session keyring was created.

The capability bit KEYCTL_CAPS1_NS_KEYRING_NAME is set if the kernel is
employing this feature.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Eric W. Biederman <ebiederm@xmission.com>
---

 include/linux/key.h            |    2 +
 include/linux/user_namespace.h |    5 ++
 include/uapi/linux/keyctl.h    |    1 
 kernel/user.c                  |    3 +
 kernel/user_namespace.c        |    7 ++-
 security/keys/keyctl.c         |    3 +
 security/keys/keyring.c        |   99 +++++++++++++++++-----------------------
 7 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index ff102731b3db..ae1177302d70 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -361,6 +361,7 @@ extern void key_set_timeout(struct key *, unsigned);
 
 extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
 				 key_perm_t perm);
+extern void key_free_user_ns(struct user_namespace *);
 
 /*
  * The permissions required on a key that we're looking up.
@@ -434,6 +435,7 @@ extern void key_init(void);
 #define key_fsuid_changed(c)		do { } while(0)
 #define key_fsgid_changed(c)		do { } while(0)
 #define key_init()			do { } while(0)
+#define key_free_user_ns(ns)		do { } while(0)
 
 #endif /* CONFIG_KEYS */
 #endif /* __KERNEL__ */
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index d6b74b91096b..90457015fa3f 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -64,6 +64,11 @@ struct user_namespace {
 	struct ns_common	ns;
 	unsigned long		flags;
 
+#ifdef CONFIG_KEYS
+	/* List of joinable keyrings in this namespace */
+	struct list_head	keyring_name_list;
+#endif
+
 	/* Register of per-UID persistent keyrings for this namespace */
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	struct key		*persistent_keyring_register;
diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index 551b5814f53e..35b405034674 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -128,5 +128,6 @@ struct keyctl_pkey_params {
 #define KEYCTL_CAPS0_INVALIDATE		0x20 /* KEYCTL_INVALIDATE supported */
 #define KEYCTL_CAPS0_RESTRICT_KEYRING	0x40 /* KEYCTL_RESTRICT_KEYRING supported */
 #define KEYCTL_CAPS0_MOVE		0x80 /* KEYCTL_MOVE supported */
+#define KEYCTL_CAPS1_NS_KEYRING_NAME	0x01 /* Keyring names are per-user_namespace */
 
 #endif /*  _LINUX_KEYCTL_H */
diff --git a/kernel/user.c b/kernel/user.c
index 88b834f0eebc..50979fd1b7aa 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -62,6 +62,9 @@ struct user_namespace init_user_ns = {
 	.ns.ops = &userns_operations,
 #endif
 	.flags = USERNS_INIT_FLAGS,
+#ifdef CONFIG_KEYS
+	.keyring_name_list = LIST_HEAD_INIT(init_user_ns.keyring_name_list),
+#endif
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	.persistent_keyring_register_sem =
 	__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 923414a246e9..bda6e890ad88 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -133,6 +133,9 @@ int create_user_ns(struct cred *new)
 	ns->flags = parent_ns->flags;
 	mutex_unlock(&userns_state_mutex);
 
+#ifdef CONFIG_KEYS
+	INIT_LIST_HEAD(&ns->keyring_name_list);
+#endif
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	init_rwsem(&ns->persistent_keyring_register_sem);
 #endif
@@ -196,9 +199,7 @@ static void free_user_ns(struct work_struct *work)
 			kfree(ns->projid_map.reverse);
 		}
 		retire_userns_sysctls(ns);
-#ifdef CONFIG_PERSISTENT_KEYRINGS
-		key_put(ns->persistent_keyring_register);
-#endif
+		key_free_user_ns(ns);
 		ns_free_inum(&ns->ns);
 		kmem_cache_free(user_ns_cachep, ns);
 		dec_user_namespaces(ucounts);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 169409b611b0..8a813220f269 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -30,7 +30,7 @@
 
 #define KEY_MAX_DESC_SIZE 4096
 
-static const unsigned char keyrings_capabilities[1] = {
+static const unsigned char keyrings_capabilities[2] = {
 	[0] = (KEYCTL_CAPS0_CAPABILITIES |
 	       (IS_ENABLED(CONFIG_PERSISTENT_KEYRINGS)	? KEYCTL_CAPS0_PERSISTENT_KEYRINGS : 0) |
 	       (IS_ENABLED(CONFIG_KEY_DH_OPERATIONS)	? KEYCTL_CAPS0_DIFFIE_HELLMAN : 0) |
@@ -40,6 +40,7 @@ static const unsigned char keyrings_capabilities[1] = {
 	       KEYCTL_CAPS0_RESTRICT_KEYRING |
 	       KEYCTL_CAPS0_MOVE
 	       ),
+	[1] = (KEYCTL_CAPS1_NS_KEYRING_NAME),
 };
 
 static int key_get_type_from_user(char *type,
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 20891cd198f0..fe851292509e 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -16,6 +16,7 @@
 #include <linux/security.h>
 #include <linux/seq_file.h>
 #include <linux/err.h>
+#include <linux/user_namespace.h>
 #include <keys/keyring-type.h>
 #include <keys/user-type.h>
 #include <linux/assoc_array_priv.h>
@@ -28,11 +29,6 @@
  */
 #define KEYRING_SEARCH_MAX_DEPTH 6
 
-/*
- * We keep all named keyrings in a hash to speed looking them up.
- */
-#define KEYRING_NAME_HASH_SIZE	(1 << 5)
-
 /*
  * We mark pointers we pass to the associative array with bit 1 set if
  * they're keyrings and clear otherwise.
@@ -55,17 +51,20 @@ static inline void *keyring_key_to_ptr(struct key *key)
 	return key;
 }
 
-static struct list_head	keyring_name_hash[KEYRING_NAME_HASH_SIZE];
 static DEFINE_RWLOCK(keyring_name_lock);
 
-static inline unsigned keyring_hash(const char *desc)
+/*
+ * Clean up the bits of user_namespace that belong to us.
+ */
+void key_free_user_ns(struct user_namespace *ns)
 {
-	unsigned bucket = 0;
-
-	for (; *desc; desc++)
-		bucket += (unsigned char)*desc;
+	write_lock(&keyring_name_lock);
+	list_del_init(&ns->keyring_name_list);
+	write_unlock(&keyring_name_lock);
 
-	return bucket & (KEYRING_NAME_HASH_SIZE - 1);
+#ifdef CONFIG_PERSISTENT_KEYRINGS
+	key_put(ns->persistent_keyring_register);
+#endif
 }
 
 /*
@@ -104,23 +103,17 @@ static DEFINE_MUTEX(keyring_serialise_link_lock);
 
 /*
  * Publish the name of a keyring so that it can be found by name (if it has
- * one).
+ * one and it doesn't begin with a dot).
  */
 static void keyring_publish_name(struct key *keyring)
 {
-	int bucket;
-
-	if (keyring->description) {
-		bucket = keyring_hash(keyring->description);
+	struct user_namespace *ns = current_user_ns();
 
+	if (keyring->description &&
+	    keyring->description[0] &&
+	    keyring->description[0] != '.') {
 		write_lock(&keyring_name_lock);
-
-		if (!keyring_name_hash[bucket].next)
-			INIT_LIST_HEAD(&keyring_name_hash[bucket]);
-
-		list_add_tail(&keyring->name_link,
-			      &keyring_name_hash[bucket]);
-
+		list_add_tail(&keyring->name_link, &ns->keyring_name_list);
 		write_unlock(&keyring_name_lock);
 	}
 }
@@ -1097,50 +1090,44 @@ key_ref_t find_key_to_update(key_ref_t keyring_ref,
  */
 struct key *find_keyring_by_name(const char *name, bool uid_keyring)
 {
+	struct user_namespace *ns = current_user_ns();
 	struct key *keyring;
-	int bucket;
 
 	if (!name)
 		return ERR_PTR(-EINVAL);
 
-	bucket = keyring_hash(name);
-
 	read_lock(&keyring_name_lock);
 
-	if (keyring_name_hash[bucket].next) {
-		/* search this hash bucket for a keyring with a matching name
-		 * that's readable and that hasn't been revoked */
-		list_for_each_entry(keyring,
-				    &keyring_name_hash[bucket],
-				    name_link
-				    ) {
-			if (!kuid_has_mapping(current_user_ns(), keyring->user->uid))
-				continue;
-
-			if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
-				continue;
+	/* Search this hash bucket for a keyring with a matching name that
+	 * grants Search permission and that hasn't been revoked
+	 */
+	list_for_each_entry(keyring, &ns->keyring_name_list, name_link) {
+		if (!kuid_has_mapping(ns, keyring->user->uid))
+			continue;
 
-			if (strcmp(keyring->description, name) != 0)
-				continue;
+		if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
+			continue;
 
-			if (uid_keyring) {
-				if (!test_bit(KEY_FLAG_UID_KEYRING,
-					      &keyring->flags))
-					continue;
-			} else {
-				if (key_permission(make_key_ref(keyring, 0),
-						   KEY_NEED_SEARCH) < 0)
-					continue;
-			}
+		if (strcmp(keyring->description, name) != 0)
+			continue;
 
-			/* we've got a match but we might end up racing with
-			 * key_cleanup() if the keyring is currently 'dead'
-			 * (ie. it has a zero usage count) */
-			if (!refcount_inc_not_zero(&keyring->usage))
+		if (uid_keyring) {
+			if (!test_bit(KEY_FLAG_UID_KEYRING,
+				      &keyring->flags))
+				continue;
+		} else {
+			if (key_permission(make_key_ref(keyring, 0),
+					   KEY_NEED_SEARCH) < 0)
 				continue;
-			keyring->last_used_at = ktime_get_real_seconds();
-			goto out;
 		}
+
+		/* we've got a match but we might end up racing with
+		 * key_cleanup() if the keyring is currently 'dead'
+		 * (ie. it has a zero usage count) */
+		if (!refcount_inc_not_zero(&keyring->usage))
+			continue;
+		keyring->last_used_at = ktime_get_real_seconds();
+		goto out;
 	}
 
 	keyring = ERR_PTR(-ENOKEY);


^ permalink raw reply related

* [PATCH 3/9] keys: Add a 'recurse' flag for keyring searches [ver #4]
From: David Howells @ 2019-06-19 16:47 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096279115.28733.8761881995303698232.stgit@warthog.procyon.org.uk>

Add a 'recurse' flag for keyring searches so that the flag can be omitted
and recursion disabled, thereby allowing just the nominated keyring to be
searched and none of the children.

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

 Documentation/security/keys/core.rst     |   10 ++++++----
 certs/blacklist.c                        |    2 +-
 crypto/asymmetric_keys/asymmetric_type.c |    2 +-
 include/linux/key.h                      |    3 ++-
 lib/digsig.c                             |    2 +-
 net/rxrpc/security.c                     |    2 +-
 security/integrity/digsig_asymmetric.c   |    4 ++--
 security/keys/internal.h                 |    1 +
 security/keys/keyctl.c                   |    2 +-
 security/keys/keyring.c                  |   12 ++++++++++--
 security/keys/proc.c                     |    3 ++-
 security/keys/process_keys.c             |    3 ++-
 security/keys/request_key.c              |    3 ++-
 security/keys/request_key_auth.c         |    3 ++-
 14 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst
index a0e245f9576f..ae930ae9d590 100644
--- a/Documentation/security/keys/core.rst
+++ b/Documentation/security/keys/core.rst
@@ -1162,11 +1162,13 @@ payload contents" for more information.
 
 	key_ref_t keyring_search(key_ref_t keyring_ref,
 				 const struct key_type *type,
-				 const char *description)
+				 const char *description,
+				 bool recurse)
 
-    This searches the keyring tree specified for a matching key. Error ENOKEY
-    is returned upon failure (use IS_ERR/PTR_ERR to determine). If successful,
-    the returned key will need to be released.
+    This searches the specified keyring only (recurse == false) or keyring tree
+    (recurse == true) specified for a matching key. Error ENOKEY is returned
+    upon failure (use IS_ERR/PTR_ERR to determine). If successful, the returned
+    key will need to be released.
 
     The possession attribute from the keyring reference is used to control
     access through the permissions mask and is propagated to the returned key
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 3a507b9e2568..181cb7fa9540 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -128,7 +128,7 @@ int is_hash_blacklisted(const u8 *hash, size_t hash_len, const char *type)
 	*p = 0;
 
 	kref = keyring_search(make_key_ref(blacklist_keyring, true),
-			      &key_type_blacklist, buffer);
+			      &key_type_blacklist, buffer, false);
 	if (!IS_ERR(kref)) {
 		key_ref_put(kref);
 		ret = -EKEYREJECTED;
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 69a0788a7de5..084027ef3121 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -87,7 +87,7 @@ struct key *find_asymmetric_key(struct key *keyring,
 	pr_debug("Look up: \"%s\"\n", req);
 
 	ref = keyring_search(make_key_ref(keyring, 1),
-			     &key_type_asymmetric, req);
+			     &key_type_asymmetric, req, true);
 	if (IS_ERR(ref))
 		pr_debug("Request for key '%s' err %ld\n", req, PTR_ERR(ref));
 	kfree(req);
diff --git a/include/linux/key.h b/include/linux/key.h
index fb2debcacea0..ff102731b3db 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -341,7 +341,8 @@ extern int keyring_clear(struct key *keyring);
 
 extern key_ref_t keyring_search(key_ref_t keyring,
 				struct key_type *type,
-				const char *description);
+				const char *description,
+				bool recurse);
 
 extern int keyring_add_key(struct key *keyring,
 			   struct key *key);
diff --git a/lib/digsig.c b/lib/digsig.c
index 3b0a579bdcdf..3782af401c68 100644
--- a/lib/digsig.c
+++ b/lib/digsig.c
@@ -221,7 +221,7 @@ int digsig_verify(struct key *keyring, const char *sig, int siglen,
 		/* search in specific keyring */
 		key_ref_t kref;
 		kref = keyring_search(make_key_ref(keyring, 1UL),
-						&key_type_user, name);
+				      &key_type_user, name, true);
 		if (IS_ERR(kref))
 			key = ERR_CAST(kref);
 		else
diff --git a/net/rxrpc/security.c b/net/rxrpc/security.c
index c4479afe8ae7..2cfc7125bc41 100644
--- a/net/rxrpc/security.c
+++ b/net/rxrpc/security.c
@@ -148,7 +148,7 @@ int rxrpc_init_server_conn_security(struct rxrpc_connection *conn)
 
 	/* look through the service's keyring */
 	kref = keyring_search(make_key_ref(rx->securities, 1UL),
-			      &key_type_rxrpc_s, kdesc);
+			      &key_type_rxrpc_s, kdesc, true);
 	if (IS_ERR(kref)) {
 		read_unlock(&local->services_lock);
 		_leave(" = %ld [search]", PTR_ERR(kref));
diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
index 99080871eb9f..358f614811e8 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -39,7 +39,7 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
 		key_ref_t kref;
 
 		kref = keyring_search(make_key_ref(key, 1),
-				     &key_type_asymmetric, name);
+				      &key_type_asymmetric, name, true);
 		if (!IS_ERR(kref)) {
 			pr_err("Key '%s' is in ima_blacklist_keyring\n", name);
 			return ERR_PTR(-EKEYREJECTED);
@@ -51,7 +51,7 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
 		key_ref_t kref;
 
 		kref = keyring_search(make_key_ref(keyring, 1),
-				      &key_type_asymmetric, name);
+				      &key_type_asymmetric, name, true);
 		if (IS_ERR(kref))
 			key = ERR_CAST(kref);
 		else
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 4305414795ae..aa361299a3ec 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -127,6 +127,7 @@ struct keyring_search_context {
 #define KEYRING_SEARCH_NO_CHECK_PERM	0x0008	/* Don't check permissions */
 #define KEYRING_SEARCH_DETECT_TOO_DEEP	0x0010	/* Give an error on excessive depth */
 #define KEYRING_SEARCH_SKIP_EXPIRED	0x0020	/* Ignore expired keys (intention to replace) */
+#define KEYRING_SEARCH_RECURSE		0x0040	/* Search child keyrings also */
 
 	int (*iterator)(const void *object, void *iterator_data);
 
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 9f418e66f067..169409b611b0 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -762,7 +762,7 @@ long keyctl_keyring_search(key_serial_t ringid,
 	}
 
 	/* do the search */
-	key_ref = keyring_search(keyring_ref, ktype, description);
+	key_ref = keyring_search(keyring_ref, ktype, description, true);
 	if (IS_ERR(key_ref)) {
 		ret = PTR_ERR(key_ref);
 
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index a5ee3b4d2eb8..20891cd198f0 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -685,6 +685,9 @@ static bool search_nested_keyrings(struct key *keyring,
 	 * Non-keyrings avoid the leftmost branch of the root entirely (root
 	 * slots 1-15).
 	 */
+	if (!(ctx->flags & KEYRING_SEARCH_RECURSE))
+		goto not_this_keyring;
+
 	ptr = READ_ONCE(keyring->keys.root);
 	if (!ptr)
 		goto not_this_keyring;
@@ -885,13 +888,15 @@ key_ref_t keyring_search_rcu(key_ref_t keyring_ref,
  * @keyring: The root of the keyring tree to be searched.
  * @type: The type of keyring we want to find.
  * @description: The name of the keyring we want to find.
+ * @recurse: True to search the children of @keyring also
  *
  * As keyring_search_rcu() above, but using the current task's credentials and
  * type's default matching function and preferred search method.
  */
 key_ref_t keyring_search(key_ref_t keyring,
 			 struct key_type *type,
-			 const char *description)
+			 const char *description,
+			 bool recurse)
 {
 	struct keyring_search_context ctx = {
 		.index_key.type		= type,
@@ -906,6 +911,8 @@ key_ref_t keyring_search(key_ref_t keyring,
 	key_ref_t key;
 	int ret;
 
+	if (recurse)
+		ctx.flags |= KEYRING_SEARCH_RECURSE;
 	if (type->match_preparse) {
 		ret = type->match_preparse(&ctx.match_data);
 		if (ret < 0)
@@ -1176,7 +1183,8 @@ static int keyring_detect_cycle(struct key *A, struct key *B)
 		.flags			= (KEYRING_SEARCH_NO_STATE_CHECK |
 					   KEYRING_SEARCH_NO_UPDATE_TIME |
 					   KEYRING_SEARCH_NO_CHECK_PERM |
-					   KEYRING_SEARCH_DETECT_TOO_DEEP),
+					   KEYRING_SEARCH_DETECT_TOO_DEEP |
+					   KEYRING_SEARCH_RECURSE),
 	};
 
 	rcu_read_lock();
diff --git a/security/keys/proc.c b/security/keys/proc.c
index f081dceae3b9..b4f5ba56b9cb 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -170,7 +170,8 @@ static int proc_keys_show(struct seq_file *m, void *v)
 		.match_data.cmp		= lookup_user_key_possessed,
 		.match_data.raw_data	= key,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
-		.flags			= KEYRING_SEARCH_NO_STATE_CHECK,
+		.flags			= (KEYRING_SEARCH_NO_STATE_CHECK |
+					   KEYRING_SEARCH_RECURSE),
 	};
 
 	key_ref = make_key_ref(key, 0);
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index f8ffb06d0297..b07f768d23dc 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -531,7 +531,8 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 	struct keyring_search_context ctx = {
 		.match_data.cmp		= lookup_user_key_possessed,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
-		.flags			= KEYRING_SEARCH_NO_STATE_CHECK,
+		.flags			= (KEYRING_SEARCH_NO_STATE_CHECK |
+					   KEYRING_SEARCH_RECURSE),
 	};
 	struct request_key_auth *rka;
 	struct key *key;
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 36c55ef47b9e..1ffd3803ce29 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -569,7 +569,8 @@ struct key *request_key_and_link(struct key_type *type,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 		.flags			= (KEYRING_SEARCH_DO_STATE_CHECK |
-					   KEYRING_SEARCH_SKIP_EXPIRED),
+					   KEYRING_SEARCH_SKIP_EXPIRED |
+					   KEYRING_SEARCH_RECURSE),
 	};
 	struct key *key;
 	key_ref_t key_ref;
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 99ed7a8a273d..f613987e8a63 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -252,7 +252,8 @@ struct key *key_get_instantiation_authkey(key_serial_t target_id)
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
-		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
+		.flags			= (KEYRING_SEARCH_DO_STATE_CHECK |
+					   KEYRING_SEARCH_RECURSE),
 	};
 	struct key *authkey;
 	key_ref_t authkey_ref;


^ permalink raw reply related

* [PATCH 2/9] keys: Cache the hash value to avoid lots of recalculation [ver #4]
From: David Howells @ 2019-06-19 16:46 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096279115.28733.8761881995303698232.stgit@warthog.procyon.org.uk>

Cache the hash of the key's type and description in the index key so that
we're not recalculating it every time we look at a key during a search.
The hash function does a bunch of multiplications, so evading those is
probably worthwhile - especially as this is done for every key examined
during a search.

This also allows the methods used by assoc_array to get chunks of index-key
to be simplified.

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

 include/linux/key.h      |    3 +++
 security/keys/internal.h |    8 +-------
 security/keys/key.c      |    2 +-
 security/keys/keyring.c  |   28 ++++++++++++++++++++--------
 4 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 86ccc2d010f6..fb2debcacea0 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -86,6 +86,8 @@ struct keyring_list;
 struct keyring_name;
 
 struct keyring_index_key {
+	/* [!] If this structure is altered, the union in struct key must change too! */
+	unsigned long		hash;			/* Hash value */
 	union {
 		struct {
 #ifdef __LITTLE_ENDIAN /* Put desc_len at the LSB of x */
@@ -213,6 +215,7 @@ struct key {
 	union {
 		struct keyring_index_key index_key;
 		struct {
+			unsigned long	hash;
 			unsigned long	len_desc;
 			struct key_type	*type;		/* type of key */
 			char		*description;
diff --git a/security/keys/internal.h b/security/keys/internal.h
index ee71c72fc5f0..4305414795ae 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -89,13 +89,7 @@ extern spinlock_t key_serial_lock;
 extern struct mutex key_construction_mutex;
 extern wait_queue_head_t request_key_conswq;
 
-
-static inline void key_set_index_key(struct keyring_index_key *index_key)
-{
-	size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
-	memcpy(index_key->desc, index_key->description, n);
-}
-
+extern void key_set_index_key(struct keyring_index_key *index_key);
 extern struct key_type *key_type_lookup(const char *type);
 extern void key_type_put(struct key_type *ktype);
 
diff --git a/security/keys/key.c b/security/keys/key.c
index 0a3828f15f57..9d52f2472a09 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -285,12 +285,12 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
 	if (!key->index_key.description)
 		goto no_memory_3;
+	key->index_key.type = type;
 	key_set_index_key(&key->index_key);
 
 	refcount_set(&key->usage, 1);
 	init_rwsem(&key->sem);
 	lockdep_set_class(&key->sem, &type->lock_class);
-	key->index_key.type = type;
 	key->user = user;
 	key->quotalen = quotalen;
 	key->datalen = type->def_datalen;
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index ebf52077598f..a5ee3b4d2eb8 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -168,7 +168,7 @@ static u64 mult_64x32_and_fold(u64 x, u32 y)
 /*
  * Hash a key type and description.
  */
-static unsigned long hash_key_type_and_desc(const struct keyring_index_key *index_key)
+static void hash_key_type_and_desc(struct keyring_index_key *index_key)
 {
 	const unsigned level_shift = ASSOC_ARRAY_LEVEL_STEP;
 	const unsigned long fan_mask = ASSOC_ARRAY_FAN_MASK;
@@ -206,10 +206,22 @@ static unsigned long hash_key_type_and_desc(const struct keyring_index_key *inde
 	 * zero for keyrings and non-zero otherwise.
 	 */
 	if (index_key->type != &key_type_keyring && (hash & fan_mask) == 0)
-		return hash | (hash >> (ASSOC_ARRAY_KEY_CHUNK_SIZE - level_shift)) | 1;
-	if (index_key->type == &key_type_keyring && (hash & fan_mask) != 0)
-		return (hash + (hash << level_shift)) & ~fan_mask;
-	return hash;
+		hash |= (hash >> (ASSOC_ARRAY_KEY_CHUNK_SIZE - level_shift)) | 1;
+	else if (index_key->type == &key_type_keyring && (hash & fan_mask) != 0)
+		hash = (hash + (hash << level_shift)) & ~fan_mask;
+	index_key->hash = hash;
+}
+
+/*
+ * Finalise an index key to include a part of the description actually in the
+ * index key and to add in the hash too.
+ */
+void key_set_index_key(struct keyring_index_key *index_key)
+{
+	size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
+	memcpy(index_key->desc, index_key->description, n);
+
+	hash_key_type_and_desc(index_key);
 }
 
 /*
@@ -227,7 +239,7 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
 	level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
 	switch (level) {
 	case 0:
-		return hash_key_type_and_desc(index_key);
+		return index_key->hash;
 	case 1:
 		return index_key->x;
 	case 2:
@@ -280,8 +292,8 @@ static int keyring_diff_objects(const void *object, const void *data)
 	int level, i;
 
 	level = 0;
-	seg_a = hash_key_type_and_desc(a);
-	seg_b = hash_key_type_and_desc(b);
+	seg_a = a->hash;
+	seg_b = b->hash;
 	if ((seg_a ^ seg_b) != 0)
 		goto differ;
 	level += ASSOC_ARRAY_KEY_CHUNK_SIZE / 8;


^ permalink raw reply related

* [PATCH 1/9] keys: Simplify key description management [ver #4]
From: David Howells @ 2019-06-19 16:46 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096279115.28733.8761881995303698232.stgit@warthog.procyon.org.uk>

Simplify key description management by cramming the word containing the
length with the first few chars of the description also.  This simplifies
the code that generates the index-key used by assoc_array.  It should speed
up key searching a bit too.

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

 include/linux/key.h        |   14 ++++++++-
 security/keys/internal.h   |    6 ++++
 security/keys/key.c        |    2 +
 security/keys/keyring.c    |   70 +++++++++++++-------------------------------
 security/keys/persistent.c |    1 +
 5 files changed, 43 insertions(+), 50 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 4cd5669184f3..86ccc2d010f6 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -86,9 +86,20 @@ struct keyring_list;
 struct keyring_name;
 
 struct keyring_index_key {
+	union {
+		struct {
+#ifdef __LITTLE_ENDIAN /* Put desc_len at the LSB of x */
+			u8	desc_len;
+			char	desc[sizeof(long) - 1];	/* First few chars of description */
+#else
+			char	desc[sizeof(long) - 1];	/* First few chars of description */
+			u8	desc_len;
+#endif
+		};
+		unsigned long x;
+	};
 	struct key_type		*type;
 	const char		*description;
-	size_t			desc_len;
 };
 
 union key_payload {
@@ -202,6 +213,7 @@ struct key {
 	union {
 		struct keyring_index_key index_key;
 		struct {
+			unsigned long	len_desc;
 			struct key_type	*type;		/* type of key */
 			char		*description;
 		};
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 3d5c08db74d2..ee71c72fc5f0 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -90,6 +90,12 @@ extern struct mutex key_construction_mutex;
 extern wait_queue_head_t request_key_conswq;
 
 
+static inline void key_set_index_key(struct keyring_index_key *index_key)
+{
+	size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
+	memcpy(index_key->desc, index_key->description, n);
+}
+
 extern struct key_type *key_type_lookup(const char *type);
 extern void key_type_put(struct key_type *ktype);
 
diff --git a/security/keys/key.c b/security/keys/key.c
index e792d65c0af8..0a3828f15f57 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -285,6 +285,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
 	if (!key->index_key.description)
 		goto no_memory_3;
+	key_set_index_key(&key->index_key);
 
 	refcount_set(&key->usage, 1);
 	init_rwsem(&key->sem);
@@ -868,6 +869,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 			goto error_free_prep;
 	}
 	index_key.desc_len = strlen(index_key.description);
+	key_set_index_key(&index_key);
 
 	ret = __key_link_lock(keyring, &index_key);
 	if (ret < 0) {
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index afa6d4024c67..ebf52077598f 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -179,9 +179,9 @@ static unsigned long hash_key_type_and_desc(const struct keyring_index_key *inde
 	int n, desc_len = index_key->desc_len;
 
 	type = (unsigned long)index_key->type;
-
 	acc = mult_64x32_and_fold(type, desc_len + 13);
 	acc = mult_64x32_and_fold(acc, 9207);
+
 	for (;;) {
 		n = desc_len;
 		if (n <= 0)
@@ -215,23 +215,13 @@ static unsigned long hash_key_type_and_desc(const struct keyring_index_key *inde
 /*
  * Build the next index key chunk.
  *
- * On 32-bit systems the index key is laid out as:
- *
- *	0	4	5	9...
- *	hash	desclen	typeptr	desc[]
- *
- * On 64-bit systems:
- *
- *	0	8	9	17...
- *	hash	desclen	typeptr	desc[]
- *
  * We return it one word-sized chunk at a time.
  */
 static unsigned long keyring_get_key_chunk(const void *data, int level)
 {
 	const struct keyring_index_key *index_key = data;
 	unsigned long chunk = 0;
-	long offset = 0;
+	const u8 *d;
 	int desc_len = index_key->desc_len, n = sizeof(chunk);
 
 	level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
@@ -239,33 +229,23 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
 	case 0:
 		return hash_key_type_and_desc(index_key);
 	case 1:
-		return ((unsigned long)index_key->type << 8) | desc_len;
+		return index_key->x;
 	case 2:
-		if (desc_len == 0)
-			return (u8)((unsigned long)index_key->type >>
-				    (ASSOC_ARRAY_KEY_CHUNK_SIZE - 8));
-		n--;
-		offset = 1;
-		/* fall through */
+		return (unsigned long)index_key->type;
 	default:
-		offset += sizeof(chunk) - 1;
-		offset += (level - 3) * sizeof(chunk);
-		if (offset >= desc_len)
+		level -= 3;
+		if (desc_len <= sizeof(index_key->desc))
 			return 0;
-		desc_len -= offset;
+
+		d = index_key->description + sizeof(index_key->desc);
+		d += level * sizeof(long);
+		desc_len -= sizeof(index_key->desc);
 		if (desc_len > n)
 			desc_len = n;
-		offset += desc_len;
 		do {
 			chunk <<= 8;
-			chunk |= ((u8*)index_key->description)[--offset];
+			chunk |= *d++;
 		} while (--desc_len > 0);
-
-		if (level == 2) {
-			chunk <<= 8;
-			chunk |= (u8)((unsigned long)index_key->type >>
-				      (ASSOC_ARRAY_KEY_CHUNK_SIZE - 8));
-		}
 		return chunk;
 	}
 }
@@ -304,39 +284,28 @@ static int keyring_diff_objects(const void *object, const void *data)
 	seg_b = hash_key_type_and_desc(b);
 	if ((seg_a ^ seg_b) != 0)
 		goto differ;
+	level += ASSOC_ARRAY_KEY_CHUNK_SIZE / 8;
 
 	/* The number of bits contributed by the hash is controlled by a
 	 * constant in the assoc_array headers.  Everything else thereafter we
 	 * can deal with as being machine word-size dependent.
 	 */
-	level += ASSOC_ARRAY_KEY_CHUNK_SIZE / 8;
-	seg_a = a->desc_len;
-	seg_b = b->desc_len;
+	seg_a = a->x;
+	seg_b = b->x;
 	if ((seg_a ^ seg_b) != 0)
 		goto differ;
+	level += sizeof(unsigned long);
 
 	/* The next bit may not work on big endian */
-	level++;
 	seg_a = (unsigned long)a->type;
 	seg_b = (unsigned long)b->type;
 	if ((seg_a ^ seg_b) != 0)
 		goto differ;
-
 	level += sizeof(unsigned long);
-	if (a->desc_len == 0)
-		goto same;
 
-	i = 0;
-	if (((unsigned long)a->description | (unsigned long)b->description) &
-	    (sizeof(unsigned long) - 1)) {
-		do {
-			seg_a = *(unsigned long *)(a->description + i);
-			seg_b = *(unsigned long *)(b->description + i);
-			if ((seg_a ^ seg_b) != 0)
-				goto differ_plus_i;
-			i += sizeof(unsigned long);
-		} while (i < (a->desc_len & (sizeof(unsigned long) - 1)));
-	}
+	i = sizeof(a->desc);
+	if (a->desc_len <= i)
+		goto same;
 
 	for (; i < a->desc_len; i++) {
 		seg_a = *(unsigned char *)(a->description + i);
@@ -662,6 +631,9 @@ static bool search_nested_keyrings(struct key *keyring,
 	BUG_ON((ctx->flags & STATE_CHECKS) == 0 ||
 	       (ctx->flags & STATE_CHECKS) == STATE_CHECKS);
 
+	if (ctx->index_key.description)
+		key_set_index_key(&ctx->index_key);
+
 	/* Check to see if this top-level keyring is what we are looking for
 	 * and whether it is valid or not.
 	 */
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index d0cb5b32eff7..fc29ec59efa7 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -87,6 +87,7 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
 	index_key.type = &key_type_keyring;
 	index_key.description = buf;
 	index_key.desc_len = sprintf(buf, "_persistent.%u", from_kuid(ns, uid));
+	key_set_index_key(&index_key);
 
 	if (ns->persistent_keyring_register) {
 		reg_ref = make_key_ref(ns->persistent_keyring_register, true);


^ permalink raw reply related

* [PATCH 0/9] keys: Namespacing [ver #4]
From: David Howells @ 2019-06-19 16:46 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel


Here are some patches to make keys and keyrings more namespace aware.

Firstly some miscellaneous patches to make the process easier:

 (1) Simplify key index_key handling so that the word-sized chunks
     assoc_array requires don't have to be shifted about, making it easier
     to add more bits into the key.

 (2) Cache the hash value in the key so that we don't have to calculate on
     every key we examine during a search (it involves a bunch of
     multiplications).

 (3) Allow keying_search() to search non-recursively.

Then the main patches:

 (4) Make it so that keyring names are per-user_namespace from the point of
     view of KEYCTL_JOIN_SESSION_KEYRING so that they're not accessible
     cross-user_namespace.

     keyctl_capabilities() shows KEYCTL_CAPS1_NS_KEYRING_NAME for this.

 (5) Move the user and user-session keyrings to the user_namespace rather
     than the user_struct.  This prevents them propagating directly across
     user_namespaces boundaries (ie. the KEY_SPEC_* flags will only pick
     from the current user_namespace).

 (6) Make it possible to include the target namespace in which the key shall
     operate in the index_key.  This will allow the possibility of multiple
     keys with the same description, but different target domains to be held
     in the same keyring.

     keyctl_capabilities() shows KEYCTL_CAPS1_NS_KEY_TAG for this.

 (7) Make it so that keys are implicitly invalidated by removal of a domain
     tag, causing them to be garbage collected.

 (8) Institute a network namespace domain tag that allows keys to be
     differentiated by the network namespace in which they operate.  New keys
     that are of a type marked 'KEY_TYPE_NET_DOMAIN' are assigned the network
     domain in force when they are created.

 (9) Make it so that the desired network namespace can be handed down into the
     request_key() mechanism.  This allows AFS, NFS, etc. to request keys
     specific to the network namespace of the superblock.

     This also means that the keys in the DNS record cache are thenceforth
     namespaced, provided network filesystems pass the appropriate network
     namespace down into dns_query().

     For DNS, AFS and NFS are good; CIFS and Ceph are not.  Other cache
     keyrings, such as idmapper keyrings, also need to set the domain tag -
     for which they need access to the network namespace of the superblock.

The patches can be found on the following branch:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-namespace

David
---
David Howells (9):
      keys: Simplify key description management
      keys: Cache the hash value to avoid lots of recalculation
      keys: Add a 'recurse' flag for keyring searches
      keys: Namespace keyring names
      keys: Move the user and user-session keyrings to the user_namespace
      keys: Include target namespace in match criteria
      keys: Garbage collect keys for which the domain has been removed
      keys: Network namespace domain tag
      keys: Pass the network namespace into request_key mechanism


 Documentation/security/keys/core.rst        |   38 +++-
 Documentation/security/keys/request-key.rst |   29 ++-
 certs/blacklist.c                           |    2 
 crypto/asymmetric_keys/asymmetric_type.c    |    2 
 fs/afs/addr_list.c                          |    4 
 fs/afs/dynroot.c                            |    8 +
 fs/cifs/dns_resolve.c                       |    3 
 fs/nfs/dns_resolve.c                        |    3 
 fs/nfs/nfs4idmap.c                          |    2 
 include/linux/dns_resolver.h                |    3 
 include/linux/key-type.h                    |    3 
 include/linux/key.h                         |   81 ++++++++
 include/linux/sched/user.h                  |   14 -
 include/linux/user_namespace.h              |   12 +
 include/net/net_namespace.h                 |    3 
 include/uapi/linux/keyctl.h                 |    2 
 kernel/user.c                               |    8 -
 kernel/user_namespace.c                     |    9 -
 lib/digsig.c                                |    2 
 net/ceph/messenger.c                        |    3 
 net/core/net_namespace.c                    |   19 ++
 net/dns_resolver/dns_key.c                  |    1 
 net/dns_resolver/dns_query.c                |    7 +
 net/rxrpc/key.c                             |    6 -
 net/rxrpc/security.c                        |    2 
 security/integrity/digsig_asymmetric.c      |    4 
 security/keys/gc.c                          |    2 
 security/keys/internal.h                    |   10 +
 security/keys/key.c                         |    5 -
 security/keys/keyctl.c                      |    8 +
 security/keys/keyring.c                     |  263 +++++++++++++++------------
 security/keys/persistent.c                  |   10 +
 security/keys/proc.c                        |    3 
 security/keys/process_keys.c                |  262 +++++++++++++++++----------
 security/keys/request_key.c                 |   62 ++++--
 security/keys/request_key_auth.c            |    3 
 36 files changed, 588 insertions(+), 310 deletions(-)


^ permalink raw reply

* Re: [PATCH net] inet: clear num_timeout reqsk_alloc()
From: Soheil Hassas Yeganeh @ 2019-06-19 16:45 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, netdev, Eric Dumazet, Yuchung Cheng,
	Neal Cardwell, syzbot
In-Reply-To: <20190619163838.150971-1-edumazet@google.com>

On Wed, Jun 19, 2019 at 12:38 PM Eric Dumazet <edumazet@google.com> wrote:
>
> KMSAN caught uninit-value in tcp_create_openreq_child() [1]
> This is caused by a recent change, combined by the fact
> that TCP cleared num_timeout, num_retrans and sk fields only
> when a request socket was about to be queued.
>
> Under syncookie mode, a temporary request socket is used,
> and req->num_timeout could contain garbage.
>
> Lets clear these three fields sooner, there is really no
> point trying to defer this and risk other bugs.
>
> [1]
>
> BUG: KMSAN: uninit-value in tcp_create_openreq_child+0x157f/0x1cc0 net/ipv4/tcp_minisocks.c:526
> CPU: 1 PID: 13357 Comm: syz-executor591 Not tainted 5.2.0-rc4+ #3
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  <IRQ>
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x191/0x1f0 lib/dump_stack.c:113
>  kmsan_report+0x162/0x2d0 mm/kmsan/kmsan.c:611
>  __msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:304
>  tcp_create_openreq_child+0x157f/0x1cc0 net/ipv4/tcp_minisocks.c:526
>  tcp_v6_syn_recv_sock+0x761/0x2d80 net/ipv6/tcp_ipv6.c:1152
>  tcp_get_cookie_sock+0x16e/0x6b0 net/ipv4/syncookies.c:209
>  cookie_v6_check+0x27e0/0x29a0 net/ipv6/syncookies.c:252
>  tcp_v6_cookie_check net/ipv6/tcp_ipv6.c:1039 [inline]
>  tcp_v6_do_rcv+0xf1c/0x1ce0 net/ipv6/tcp_ipv6.c:1344
>  tcp_v6_rcv+0x60b7/0x6a30 net/ipv6/tcp_ipv6.c:1554
>  ip6_protocol_deliver_rcu+0x1433/0x22f0 net/ipv6/ip6_input.c:397
>  ip6_input_finish net/ipv6/ip6_input.c:438 [inline]
>  NF_HOOK include/linux/netfilter.h:305 [inline]
>  ip6_input+0x2af/0x340 net/ipv6/ip6_input.c:447
>  dst_input include/net/dst.h:439 [inline]
>  ip6_rcv_finish net/ipv6/ip6_input.c:76 [inline]
>  NF_HOOK include/linux/netfilter.h:305 [inline]
>  ipv6_rcv+0x683/0x710 net/ipv6/ip6_input.c:272
>  __netif_receive_skb_one_core net/core/dev.c:4981 [inline]
>  __netif_receive_skb net/core/dev.c:5095 [inline]
>  process_backlog+0x721/0x1410 net/core/dev.c:5906
>  napi_poll net/core/dev.c:6329 [inline]
>  net_rx_action+0x738/0x1940 net/core/dev.c:6395
>  __do_softirq+0x4ad/0x858 kernel/softirq.c:293
>  do_softirq_own_stack+0x49/0x80 arch/x86/entry/entry_64.S:1052
>  </IRQ>
>  do_softirq kernel/softirq.c:338 [inline]
>  __local_bh_enable_ip+0x199/0x1e0 kernel/softirq.c:190
>  local_bh_enable+0x36/0x40 include/linux/bottom_half.h:32
>  rcu_read_unlock_bh include/linux/rcupdate.h:682 [inline]
>  ip6_finish_output2+0x213f/0x2670 net/ipv6/ip6_output.c:117
>  ip6_finish_output+0xae4/0xbc0 net/ipv6/ip6_output.c:150
>  NF_HOOK_COND include/linux/netfilter.h:294 [inline]
>  ip6_output+0x5d3/0x720 net/ipv6/ip6_output.c:167
>  dst_output include/net/dst.h:433 [inline]
>  NF_HOOK include/linux/netfilter.h:305 [inline]
>  ip6_xmit+0x1f53/0x2650 net/ipv6/ip6_output.c:271
>  inet6_csk_xmit+0x3df/0x4f0 net/ipv6/inet6_connection_sock.c:135
>  __tcp_transmit_skb+0x4076/0x5b40 net/ipv4/tcp_output.c:1156
>  tcp_transmit_skb net/ipv4/tcp_output.c:1172 [inline]
>  tcp_write_xmit+0x39a9/0xa730 net/ipv4/tcp_output.c:2397
>  __tcp_push_pending_frames+0x124/0x4e0 net/ipv4/tcp_output.c:2573
>  tcp_send_fin+0xd43/0x1540 net/ipv4/tcp_output.c:3118
>  tcp_close+0x16ba/0x1860 net/ipv4/tcp.c:2403
>  inet_release+0x1f7/0x270 net/ipv4/af_inet.c:427
>  inet6_release+0xaf/0x100 net/ipv6/af_inet6.c:470
>  __sock_release net/socket.c:601 [inline]
>  sock_close+0x156/0x490 net/socket.c:1273
>  __fput+0x4c9/0xba0 fs/file_table.c:280
>  ____fput+0x37/0x40 fs/file_table.c:313
>  task_work_run+0x22e/0x2a0 kernel/task_work.c:113
>  tracehook_notify_resume include/linux/tracehook.h:185 [inline]
>  exit_to_usermode_loop arch/x86/entry/common.c:168 [inline]
>  prepare_exit_to_usermode+0x39d/0x4d0 arch/x86/entry/common.c:199
>  syscall_return_slowpath+0x90/0x5c0 arch/x86/entry/common.c:279
>  do_syscall_64+0xe2/0xf0 arch/x86/entry/common.c:305
>  entry_SYSCALL_64_after_hwframe+0x63/0xe7
> RIP: 0033:0x401d50
> Code: 01 f0 ff ff 0f 83 40 0d 00 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 83 3d dd 8d 2d 00 00 75 14 b8 03 00 00 00 0f 05 <48> 3d 01 f0 ff ff 0f 83 14 0d 00 00 c3 48 83 ec 08 e8 7a 02 00 00
> RSP: 002b:00007fff1cf58cf8 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
> RAX: 0000000000000000 RBX: 0000000000000004 RCX: 0000000000401d50
> RDX: 000000000000001c RSI: 0000000000000000 RDI: 0000000000000003
> RBP: 00000000004a9050 R08: 0000000020000040 R09: 000000000000001c
> R10: 0000000020004004 R11: 0000000000000246 R12: 0000000000402ef0
> R13: 0000000000402f80 R14: 0000000000000000 R15: 0000000000000000
>
> Uninit was created at:
>  kmsan_save_stack_with_flags mm/kmsan/kmsan.c:201 [inline]
>  kmsan_internal_poison_shadow+0x53/0xa0 mm/kmsan/kmsan.c:160
>  kmsan_kmalloc+0xa4/0x130 mm/kmsan/kmsan_hooks.c:177
>  kmem_cache_alloc+0x534/0xb00 mm/slub.c:2781
>  reqsk_alloc include/net/request_sock.h:84 [inline]
>  inet_reqsk_alloc+0xa8/0x600 net/ipv4/tcp_input.c:6384
>  cookie_v6_check+0xadb/0x29a0 net/ipv6/syncookies.c:173
>  tcp_v6_cookie_check net/ipv6/tcp_ipv6.c:1039 [inline]
>  tcp_v6_do_rcv+0xf1c/0x1ce0 net/ipv6/tcp_ipv6.c:1344
>  tcp_v6_rcv+0x60b7/0x6a30 net/ipv6/tcp_ipv6.c:1554
>  ip6_protocol_deliver_rcu+0x1433/0x22f0 net/ipv6/ip6_input.c:397
>  ip6_input_finish net/ipv6/ip6_input.c:438 [inline]
>  NF_HOOK include/linux/netfilter.h:305 [inline]
>  ip6_input+0x2af/0x340 net/ipv6/ip6_input.c:447
>  dst_input include/net/dst.h:439 [inline]
>  ip6_rcv_finish net/ipv6/ip6_input.c:76 [inline]
>  NF_HOOK include/linux/netfilter.h:305 [inline]
>  ipv6_rcv+0x683/0x710 net/ipv6/ip6_input.c:272
>  __netif_receive_skb_one_core net/core/dev.c:4981 [inline]
>  __netif_receive_skb net/core/dev.c:5095 [inline]
>  process_backlog+0x721/0x1410 net/core/dev.c:5906
>  napi_poll net/core/dev.c:6329 [inline]
>  net_rx_action+0x738/0x1940 net/core/dev.c:6395
>  __do_softirq+0x4ad/0x858 kernel/softirq.c:293
>  do_softirq_own_stack+0x49/0x80 arch/x86/entry/entry_64.S:1052
>  do_softirq kernel/softirq.c:338 [inline]
>  __local_bh_enable_ip+0x199/0x1e0 kernel/softirq.c:190
>  local_bh_enable+0x36/0x40 include/linux/bottom_half.h:32
>  rcu_read_unlock_bh include/linux/rcupdate.h:682 [inline]
>  ip6_finish_output2+0x213f/0x2670 net/ipv6/ip6_output.c:117
>  ip6_finish_output+0xae4/0xbc0 net/ipv6/ip6_output.c:150
>  NF_HOOK_COND include/linux/netfilter.h:294 [inline]
>  ip6_output+0x5d3/0x720 net/ipv6/ip6_output.c:167
>  dst_output include/net/dst.h:433 [inline]
>  NF_HOOK include/linux/netfilter.h:305 [inline]
>  ip6_xmit+0x1f53/0x2650 net/ipv6/ip6_output.c:271
>  inet6_csk_xmit+0x3df/0x4f0 net/ipv6/inet6_connection_sock.c:135
>  __tcp_transmit_skb+0x4076/0x5b40 net/ipv4/tcp_output.c:1156
>  tcp_transmit_skb net/ipv4/tcp_output.c:1172 [inline]
>  tcp_write_xmit+0x39a9/0xa730 net/ipv4/tcp_output.c:2397
>  __tcp_push_pending_frames+0x124/0x4e0 net/ipv4/tcp_output.c:2573
>  tcp_send_fin+0xd43/0x1540 net/ipv4/tcp_output.c:3118
>  tcp_close+0x16ba/0x1860 net/ipv4/tcp.c:2403
>  inet_release+0x1f7/0x270 net/ipv4/af_inet.c:427
>  inet6_release+0xaf/0x100 net/ipv6/af_inet6.c:470
>  __sock_release net/socket.c:601 [inline]
>  sock_close+0x156/0x490 net/socket.c:1273
>  __fput+0x4c9/0xba0 fs/file_table.c:280
>  ____fput+0x37/0x40 fs/file_table.c:313
>  task_work_run+0x22e/0x2a0 kernel/task_work.c:113
>  tracehook_notify_resume include/linux/tracehook.h:185 [inline]
>  exit_to_usermode_loop arch/x86/entry/common.c:168 [inline]
>  prepare_exit_to_usermode+0x39d/0x4d0 arch/x86/entry/common.c:199
>  syscall_return_slowpath+0x90/0x5c0 arch/x86/entry/common.c:279
>  do_syscall_64+0xe2/0xf0 arch/x86/entry/common.c:305
>  entry_SYSCALL_64_after_hwframe+0x63/0xe7
>
> Fixes: 336c39a03151 ("tcp: undo init congestion window on false SYNACK timeout")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

Thank you for the fix!

> ---
>  include/net/request_sock.h      | 3 +++
>  net/ipv4/inet_connection_sock.c | 4 ----
>  net/ipv4/tcp_fastopen.c         | 4 ----
>  3 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
> index b3ea21f2732ef509d47bfcecc38f78d9178a3d5c..fd178d58fa84e7ae7abdeff5be2ba7b1ec790889 100644
> --- a/include/net/request_sock.h
> +++ b/include/net/request_sock.h
> @@ -97,6 +97,9 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener,
>         sk_node_init(&req_to_sk(req)->sk_node);
>         sk_tx_queue_clear(req_to_sk(req));
>         req->saved_syn = NULL;
> +       req->num_timeout = 0;
> +       req->num_retrans = 0;
> +       req->sk = NULL;
>         refcount_set(&req->rsk_refcnt, 0);
>
>         return req;
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 13ec7c3a9c49f02ed359f3f941c8ff70f4699695..7fd6db3fe3664b02fa0279a6aef08144588de6e4 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -752,10 +752,6 @@ static void reqsk_timer_handler(struct timer_list *t)
>  static void reqsk_queue_hash_req(struct request_sock *req,
>                                  unsigned long timeout)
>  {
> -       req->num_retrans = 0;
> -       req->num_timeout = 0;
> -       req->sk = NULL;
> -
>         timer_setup(&req->rsk_timer, reqsk_timer_handler, TIMER_PINNED);
>         mod_timer(&req->rsk_timer, jiffies + timeout);
>
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index 018a484773551f8e67fe20be97b04f6d02104a56..f5a45e1e118224966e336725de71d3636b207892 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -221,10 +221,6 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
>         struct sock *child;
>         bool own_req;
>
> -       req->num_retrans = 0;
> -       req->num_timeout = 0;
> -       req->sk = NULL;
> -
>         child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
>                                                          NULL, &own_req);
>         if (!child)
> --
> 2.22.0.410.gd8fdbe21b5-goog
>

^ permalink raw reply

* [PATCH] can: xilinx_can: clean up indentation issue
From: Colin King @ 2019-06-19 16:45 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, David S . Miller,
	Michal Simek, linux-can, netdev, linux-arm-kernel
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

A statement is indented one level too deep, fix this.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/can/xilinx_can.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 63203ff452b5..a3940141555c 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -924,7 +924,7 @@ static void xcan_err_interrupt(struct net_device *ndev, u32 isr)
 				cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
 			}
 		}
-			priv->can.can_stats.bus_error++;
+		priv->can.can_stats.bus_error++;
 	}
 
 	if (skb) {
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] mm: mempolicy: handle vma with unmovable pages mapped correctly in mbind
From: Yang Shi @ 2019-06-19 16:39 UTC (permalink / raw)
  To: Vlastimil Babka, Michal Hocko
  Cc: akpm, mgorman, linux-mm, linux-kernel, Eric Dumazet,
	David S. Miller, netdev
In-Reply-To: <2c30d86f-43e4-f43c-411d-c916fb1de44e@suse.cz>



On 6/19/19 1:18 AM, Vlastimil Babka wrote:
> On 6/18/19 7:06 PM, Yang Shi wrote:
>> The BUG_ON was removed by commit
>> d44d363f65780f2ac2ec672164555af54896d40d ("mm: don't assume anonymous
>> pages have SwapBacked flag") since 4.12.
> Perhaps that commit should be sent to stable@ ? Although with
> VM_BUG_ON() this is less critical than plain BUG_ON().

I don't think we have to. I agree it is less critical,  VM_DEBUG should 
be not enabled for production environment.

And, it doesn't actually break anything since split_huge_page would just 
return error, and those unmovable pages are silently ignored by isolate.



^ permalink raw reply

* [PATCH net] inet: clear num_timeout reqsk_alloc()
From: Eric Dumazet @ 2019-06-19 16:38 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Eric Dumazet, Yuchung Cheng, Neal Cardwell,
	Soheil Hassas Yeganeh, syzbot

KMSAN caught uninit-value in tcp_create_openreq_child() [1]
This is caused by a recent change, combined by the fact
that TCP cleared num_timeout, num_retrans and sk fields only
when a request socket was about to be queued.

Under syncookie mode, a temporary request socket is used,
and req->num_timeout could contain garbage.

Lets clear these three fields sooner, there is really no
point trying to defer this and risk other bugs.

[1]

BUG: KMSAN: uninit-value in tcp_create_openreq_child+0x157f/0x1cc0 net/ipv4/tcp_minisocks.c:526
CPU: 1 PID: 13357 Comm: syz-executor591 Not tainted 5.2.0-rc4+ #3
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 <IRQ>
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x191/0x1f0 lib/dump_stack.c:113
 kmsan_report+0x162/0x2d0 mm/kmsan/kmsan.c:611
 __msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:304
 tcp_create_openreq_child+0x157f/0x1cc0 net/ipv4/tcp_minisocks.c:526
 tcp_v6_syn_recv_sock+0x761/0x2d80 net/ipv6/tcp_ipv6.c:1152
 tcp_get_cookie_sock+0x16e/0x6b0 net/ipv4/syncookies.c:209
 cookie_v6_check+0x27e0/0x29a0 net/ipv6/syncookies.c:252
 tcp_v6_cookie_check net/ipv6/tcp_ipv6.c:1039 [inline]
 tcp_v6_do_rcv+0xf1c/0x1ce0 net/ipv6/tcp_ipv6.c:1344
 tcp_v6_rcv+0x60b7/0x6a30 net/ipv6/tcp_ipv6.c:1554
 ip6_protocol_deliver_rcu+0x1433/0x22f0 net/ipv6/ip6_input.c:397
 ip6_input_finish net/ipv6/ip6_input.c:438 [inline]
 NF_HOOK include/linux/netfilter.h:305 [inline]
 ip6_input+0x2af/0x340 net/ipv6/ip6_input.c:447
 dst_input include/net/dst.h:439 [inline]
 ip6_rcv_finish net/ipv6/ip6_input.c:76 [inline]
 NF_HOOK include/linux/netfilter.h:305 [inline]
 ipv6_rcv+0x683/0x710 net/ipv6/ip6_input.c:272
 __netif_receive_skb_one_core net/core/dev.c:4981 [inline]
 __netif_receive_skb net/core/dev.c:5095 [inline]
 process_backlog+0x721/0x1410 net/core/dev.c:5906
 napi_poll net/core/dev.c:6329 [inline]
 net_rx_action+0x738/0x1940 net/core/dev.c:6395
 __do_softirq+0x4ad/0x858 kernel/softirq.c:293
 do_softirq_own_stack+0x49/0x80 arch/x86/entry/entry_64.S:1052
 </IRQ>
 do_softirq kernel/softirq.c:338 [inline]
 __local_bh_enable_ip+0x199/0x1e0 kernel/softirq.c:190
 local_bh_enable+0x36/0x40 include/linux/bottom_half.h:32
 rcu_read_unlock_bh include/linux/rcupdate.h:682 [inline]
 ip6_finish_output2+0x213f/0x2670 net/ipv6/ip6_output.c:117
 ip6_finish_output+0xae4/0xbc0 net/ipv6/ip6_output.c:150
 NF_HOOK_COND include/linux/netfilter.h:294 [inline]
 ip6_output+0x5d3/0x720 net/ipv6/ip6_output.c:167
 dst_output include/net/dst.h:433 [inline]
 NF_HOOK include/linux/netfilter.h:305 [inline]
 ip6_xmit+0x1f53/0x2650 net/ipv6/ip6_output.c:271
 inet6_csk_xmit+0x3df/0x4f0 net/ipv6/inet6_connection_sock.c:135
 __tcp_transmit_skb+0x4076/0x5b40 net/ipv4/tcp_output.c:1156
 tcp_transmit_skb net/ipv4/tcp_output.c:1172 [inline]
 tcp_write_xmit+0x39a9/0xa730 net/ipv4/tcp_output.c:2397
 __tcp_push_pending_frames+0x124/0x4e0 net/ipv4/tcp_output.c:2573
 tcp_send_fin+0xd43/0x1540 net/ipv4/tcp_output.c:3118
 tcp_close+0x16ba/0x1860 net/ipv4/tcp.c:2403
 inet_release+0x1f7/0x270 net/ipv4/af_inet.c:427
 inet6_release+0xaf/0x100 net/ipv6/af_inet6.c:470
 __sock_release net/socket.c:601 [inline]
 sock_close+0x156/0x490 net/socket.c:1273
 __fput+0x4c9/0xba0 fs/file_table.c:280
 ____fput+0x37/0x40 fs/file_table.c:313
 task_work_run+0x22e/0x2a0 kernel/task_work.c:113
 tracehook_notify_resume include/linux/tracehook.h:185 [inline]
 exit_to_usermode_loop arch/x86/entry/common.c:168 [inline]
 prepare_exit_to_usermode+0x39d/0x4d0 arch/x86/entry/common.c:199
 syscall_return_slowpath+0x90/0x5c0 arch/x86/entry/common.c:279
 do_syscall_64+0xe2/0xf0 arch/x86/entry/common.c:305
 entry_SYSCALL_64_after_hwframe+0x63/0xe7
RIP: 0033:0x401d50
Code: 01 f0 ff ff 0f 83 40 0d 00 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 83 3d dd 8d 2d 00 00 75 14 b8 03 00 00 00 0f 05 <48> 3d 01 f0 ff ff 0f 83 14 0d 00 00 c3 48 83 ec 08 e8 7a 02 00 00
RSP: 002b:00007fff1cf58cf8 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 0000000000000004 RCX: 0000000000401d50
RDX: 000000000000001c RSI: 0000000000000000 RDI: 0000000000000003
RBP: 00000000004a9050 R08: 0000000020000040 R09: 000000000000001c
R10: 0000000020004004 R11: 0000000000000246 R12: 0000000000402ef0
R13: 0000000000402f80 R14: 0000000000000000 R15: 0000000000000000

Uninit was created at:
 kmsan_save_stack_with_flags mm/kmsan/kmsan.c:201 [inline]
 kmsan_internal_poison_shadow+0x53/0xa0 mm/kmsan/kmsan.c:160
 kmsan_kmalloc+0xa4/0x130 mm/kmsan/kmsan_hooks.c:177
 kmem_cache_alloc+0x534/0xb00 mm/slub.c:2781
 reqsk_alloc include/net/request_sock.h:84 [inline]
 inet_reqsk_alloc+0xa8/0x600 net/ipv4/tcp_input.c:6384
 cookie_v6_check+0xadb/0x29a0 net/ipv6/syncookies.c:173
 tcp_v6_cookie_check net/ipv6/tcp_ipv6.c:1039 [inline]
 tcp_v6_do_rcv+0xf1c/0x1ce0 net/ipv6/tcp_ipv6.c:1344
 tcp_v6_rcv+0x60b7/0x6a30 net/ipv6/tcp_ipv6.c:1554
 ip6_protocol_deliver_rcu+0x1433/0x22f0 net/ipv6/ip6_input.c:397
 ip6_input_finish net/ipv6/ip6_input.c:438 [inline]
 NF_HOOK include/linux/netfilter.h:305 [inline]
 ip6_input+0x2af/0x340 net/ipv6/ip6_input.c:447
 dst_input include/net/dst.h:439 [inline]
 ip6_rcv_finish net/ipv6/ip6_input.c:76 [inline]
 NF_HOOK include/linux/netfilter.h:305 [inline]
 ipv6_rcv+0x683/0x710 net/ipv6/ip6_input.c:272
 __netif_receive_skb_one_core net/core/dev.c:4981 [inline]
 __netif_receive_skb net/core/dev.c:5095 [inline]
 process_backlog+0x721/0x1410 net/core/dev.c:5906
 napi_poll net/core/dev.c:6329 [inline]
 net_rx_action+0x738/0x1940 net/core/dev.c:6395
 __do_softirq+0x4ad/0x858 kernel/softirq.c:293
 do_softirq_own_stack+0x49/0x80 arch/x86/entry/entry_64.S:1052
 do_softirq kernel/softirq.c:338 [inline]
 __local_bh_enable_ip+0x199/0x1e0 kernel/softirq.c:190
 local_bh_enable+0x36/0x40 include/linux/bottom_half.h:32
 rcu_read_unlock_bh include/linux/rcupdate.h:682 [inline]
 ip6_finish_output2+0x213f/0x2670 net/ipv6/ip6_output.c:117
 ip6_finish_output+0xae4/0xbc0 net/ipv6/ip6_output.c:150
 NF_HOOK_COND include/linux/netfilter.h:294 [inline]
 ip6_output+0x5d3/0x720 net/ipv6/ip6_output.c:167
 dst_output include/net/dst.h:433 [inline]
 NF_HOOK include/linux/netfilter.h:305 [inline]
 ip6_xmit+0x1f53/0x2650 net/ipv6/ip6_output.c:271
 inet6_csk_xmit+0x3df/0x4f0 net/ipv6/inet6_connection_sock.c:135
 __tcp_transmit_skb+0x4076/0x5b40 net/ipv4/tcp_output.c:1156
 tcp_transmit_skb net/ipv4/tcp_output.c:1172 [inline]
 tcp_write_xmit+0x39a9/0xa730 net/ipv4/tcp_output.c:2397
 __tcp_push_pending_frames+0x124/0x4e0 net/ipv4/tcp_output.c:2573
 tcp_send_fin+0xd43/0x1540 net/ipv4/tcp_output.c:3118
 tcp_close+0x16ba/0x1860 net/ipv4/tcp.c:2403
 inet_release+0x1f7/0x270 net/ipv4/af_inet.c:427
 inet6_release+0xaf/0x100 net/ipv6/af_inet6.c:470
 __sock_release net/socket.c:601 [inline]
 sock_close+0x156/0x490 net/socket.c:1273
 __fput+0x4c9/0xba0 fs/file_table.c:280
 ____fput+0x37/0x40 fs/file_table.c:313
 task_work_run+0x22e/0x2a0 kernel/task_work.c:113
 tracehook_notify_resume include/linux/tracehook.h:185 [inline]
 exit_to_usermode_loop arch/x86/entry/common.c:168 [inline]
 prepare_exit_to_usermode+0x39d/0x4d0 arch/x86/entry/common.c:199
 syscall_return_slowpath+0x90/0x5c0 arch/x86/entry/common.c:279
 do_syscall_64+0xe2/0xf0 arch/x86/entry/common.c:305
 entry_SYSCALL_64_after_hwframe+0x63/0xe7

Fixes: 336c39a03151 ("tcp: undo init congestion window on false SYNACK timeout")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
 include/net/request_sock.h      | 3 +++
 net/ipv4/inet_connection_sock.c | 4 ----
 net/ipv4/tcp_fastopen.c         | 4 ----
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index b3ea21f2732ef509d47bfcecc38f78d9178a3d5c..fd178d58fa84e7ae7abdeff5be2ba7b1ec790889 100644
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -97,6 +97,9 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener,
 	sk_node_init(&req_to_sk(req)->sk_node);
 	sk_tx_queue_clear(req_to_sk(req));
 	req->saved_syn = NULL;
+	req->num_timeout = 0;
+	req->num_retrans = 0;
+	req->sk = NULL;
 	refcount_set(&req->rsk_refcnt, 0);
 
 	return req;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 13ec7c3a9c49f02ed359f3f941c8ff70f4699695..7fd6db3fe3664b02fa0279a6aef08144588de6e4 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -752,10 +752,6 @@ static void reqsk_timer_handler(struct timer_list *t)
 static void reqsk_queue_hash_req(struct request_sock *req,
 				 unsigned long timeout)
 {
-	req->num_retrans = 0;
-	req->num_timeout = 0;
-	req->sk = NULL;
-
 	timer_setup(&req->rsk_timer, reqsk_timer_handler, TIMER_PINNED);
 	mod_timer(&req->rsk_timer, jiffies + timeout);
 
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 018a484773551f8e67fe20be97b04f6d02104a56..f5a45e1e118224966e336725de71d3636b207892 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -221,10 +221,6 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
 	struct sock *child;
 	bool own_req;
 
-	req->num_retrans = 0;
-	req->num_timeout = 0;
-	req->sk = NULL;
-
 	child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
 							 NULL, &own_req);
 	if (!child)
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* Re: general protection fault in call_fib6_multipath_entry_notifiers
From: Ido Schimmel @ 2019-06-19 16:35 UTC (permalink / raw)
  To: syzbot
  Cc: davem, dsahern, idosch, jiri, kuznet, linux-kernel, netdev,
	syzkaller-bugs, yoshfuji
In-Reply-To: <0000000000004237d7058baf24e3@google.com>

On Wed, Jun 19, 2019 at 08:47:07AM -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    39f58860 net/mlx5: add missing void argument to function m..
> git tree:       net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=115eb99ea00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=4937094fc643655f
> dashboard link: https://syzkaller.appspot.com/bug?extid=382566d339d52cd1a204
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=120c9e11a00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=120c3d21a00000
> 
> The bug was bisected to:
> 
> commit ebee3cad835f7fe7250213225cf6d62c7cf3b2ca
> Author: Ido Schimmel <idosch@mellanox.com>
> Date:   Tue Jun 18 15:12:48 2019 +0000
> 
>     ipv6: Add IPv6 multipath notifications for add / replace
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=1529970aa00000
> final crash:    https://syzkaller.appspot.com/x/report.txt?x=1729970aa00000
> console output: https://syzkaller.appspot.com/x/log.txt?x=1329970aa00000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+382566d339d52cd1a204@syzkaller.appspotmail.com
> Fixes: ebee3cad835f ("ipv6: Add IPv6 multipath notifications for add /
> replace")
> 
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] PREEMPT SMP KASAN
> CPU: 0 PID: 9190 Comm: syz-executor149 Not tainted 5.2.0-rc5+ #38
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:call_fib6_multipath_entry_notifiers+0xd1/0x1a0

Did not consider the case where RTA_MULTIPATH does not encode valid
nexthop information in which case 'rt_notif' can be NULL. Will send a
fix after I run some tests.

> net/ipv6/ip6_fib.c:396
> Code: 8b b5 30 ff ff ff 48 c7 85 68 ff ff ff 00 00 00 00 48 c7 85 70 ff ff
> ff 00 00 00 00 89 45 88 4c 89 e0 48 c1 e8 03 4c 89 65 80 <42> 80 3c 28 00 0f
> 85 9a 00 00 00 48 b8 00 00 00 00 00 fc ff df 4d
> RSP: 0018:ffff88809788f2c0 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: 1ffff11012f11e59 RCX: 00000000ffffffff
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: ffff88809788f390 R08: ffff88809788f8c0 R09: 000000000000000c
> R10: ffff88809788f5d8 R11: ffff88809788f527 R12: 0000000000000000
> R13: dffffc0000000000 R14: ffff88809788f8c0 R15: ffffffff89541d80
> FS:  000055555632c880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020000080 CR3: 000000009ba7c000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  ip6_route_multipath_add+0xc55/0x1490 net/ipv6/route.c:5094
>  inet6_rtm_newroute+0xed/0x180 net/ipv6/route.c:5208
>  rtnetlink_rcv_msg+0x463/0xb00 net/core/rtnetlink.c:5219
>  netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
>  rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5237
>  netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
>  netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
>  netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
>  sock_sendmsg_nosec net/socket.c:646 [inline]
>  sock_sendmsg+0xd7/0x130 net/socket.c:665
>  ___sys_sendmsg+0x803/0x920 net/socket.c:2286
>  __sys_sendmsg+0x105/0x1d0 net/socket.c:2324
>  __do_sys_sendmsg net/socket.c:2333 [inline]
>  __se_sys_sendmsg net/socket.c:2331 [inline]
>  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2331
>  do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x4401f9
> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff
> 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffc09fd0028 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004401f9
> RDX: 0000000000000000 RSI: 0000000020000080 RDI: 0000000000000003
> RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401a80
> R13: 0000000000401b10 R14: 0000000000000000 R15: 0000000000000000
> Modules linked in:
> ---[ end trace 77949df4cfac115c ]---
> RIP: 0010:call_fib6_multipath_entry_notifiers+0xd1/0x1a0
> net/ipv6/ip6_fib.c:396
> Code: 8b b5 30 ff ff ff 48 c7 85 68 ff ff ff 00 00 00 00 48 c7 85 70 ff ff
> ff 00 00 00 00 89 45 88 4c 89 e0 48 c1 e8 03 4c 89 65 80 <42> 80 3c 28 00 0f
> 85 9a 00 00 00 48 b8 00 00 00 00 00 fc ff df 4d
> RSP: 0018:ffff88809788f2c0 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: 1ffff11012f11e59 RCX: 00000000ffffffff
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: ffff88809788f390 R08: ffff88809788f8c0 R09: 000000000000000c
> R10: ffff88809788f5d8 R11: ffff88809788f527 R12: 0000000000000000
> R13: dffffc0000000000 R14: ffff88809788f8c0 R15: ffffffff89541d80
> FS:  000055555632c880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020000080 CR3: 000000009ba7c000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> 
> 
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
> 
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: [PATCH v3 0/7] Hexdump Enhancements
From: Joe Perches @ 2019-06-19 16:31 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
	Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
	Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
	Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
	James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
	Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <20190617020430.8708-1-alastair@au1.ibm.com>

On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Apologies for the large CC list, it's a heads up for those responsible
> for subsystems where a prototype change in generic code causes a change
> in those subsystems.
> 
> This series enhances hexdump.

Still not a fan of these patches.

> These improve the readability of the dumped data in certain situations
> (eg. wide terminals are available, many lines of empty bytes exist, etc).

Changing hexdump's last argument from bool to int is odd.

Perhaps a new function should be added instead of changing
the existing hexdump.

> The default behaviour of hexdump is unchanged, however, the prototype
> for hex_dump_to_buffer() has changed, and print_hex_dump() has been
> renamed to print_hex_dump_ext(), with a wrapper replacing it for
> compatibility with existing code, which would have been too invasive to
> change.
> 
> Hexdump selftests have be run & confirmed passed.



^ permalink raw reply

* [PATCH][bpf-next] bpf: verifier: add a break statement in switch
From: Gustavo A. R. Silva @ 2019-06-19 16:07 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, Lawrence Brakmo, Andrii Nakryiko
  Cc: netdev, bpf, linux-kernel, Gustavo A. R. Silva, Kees Cook

Notice that in this case, it's much clearer to explicitly add a break
rather than letting the code to fall through. It also avoid potential
future fall-through warnings[1].

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

[1] https://lore.kernel.org/patchwork/patch/1087056/

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 kernel/bpf/verifier.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 709ce4cef8ba..0b38cc917d21 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6066,6 +6066,7 @@ static int check_return_code(struct bpf_verifier_env *env)
 			range = tnum_range(0, 3);
 			enforce_attach_type_range = tnum_range(2, 3);
 		}
+		break;
 	case BPF_PROG_TYPE_CGROUP_SOCK:
 	case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
 	case BPF_PROG_TYPE_SOCK_OPS:
-- 
2.21.0


^ permalink raw reply related

* Re: use exact allocation for dma coherent memory
From: Jason Gunthorpe @ 2019-06-19 16:29 UTC (permalink / raw)
  To: Christoph Hellwig, Potnuri Bharat Teja
  Cc: Dan Carpenter, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Daniel Vetter, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Ian Abbott, H Hartley Sweeten, devel, linux-s390,
	Intel Linux Wireless, linux-rdma, netdev, intel-gfx,
	linux-wireless, linux-kernel, dri-devel, linux-mm, iommu,
	moderated list:ARM PORT, linux-media
In-Reply-To: <20190617083342.GA7883@lst.de>

On Mon, Jun 17, 2019 at 10:33:42AM +0200, Christoph Hellwig wrote:
> > drivers/infiniband/hw/cxgb4/qp.c
> >    129  static int alloc_host_sq(struct c4iw_rdev *rdev, struct t4_sq *sq)
> >    130  {
> >    131          sq->queue = dma_alloc_coherent(&(rdev->lldi.pdev->dev), sq->memsize,
> >    132                                         &(sq->dma_addr), GFP_KERNEL);
> >    133          if (!sq->queue)
> >    134                  return -ENOMEM;
> >    135          sq->phys_addr = virt_to_phys(sq->queue);
> >    136          dma_unmap_addr_set(sq, mapping, sq->dma_addr);
> >    137          return 0;
> >    138  }
> > 
> > Is this a bug?
> 
> Yes.  This will blow up badly on many platforms, as sq->queue
> might be vmapped, ioremapped, come from a pool without page backing.

Gah, this addr gets fed into io_remap_pfn_range/remap_pfn_range too..

Potnuri, you should fix this.. 

You probably need to use dma_mmap_from_dev_coherent() in the mmap ?

Jason

^ permalink raw reply

* [PATCH][next] libbpf: fix spelling mistake "conflictling" -> "conflicting"
From: Colin King @ 2019-06-19 16:27 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, netdev, bpf
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

There are several spelling mistakes in pr_warning messages. Fix these.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 tools/lib/bpf/libbpf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 4259c9f0cfe7..68f45a96769f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1169,7 +1169,7 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 			pr_debug("map '%s': found key_size = %u.\n",
 				 map_name, sz);
 			if (map->def.key_size && map->def.key_size != sz) {
-				pr_warning("map '%s': conflictling key size %u != %u.\n",
+				pr_warning("map '%s': conflicting key size %u != %u.\n",
 					   map_name, map->def.key_size, sz);
 				return -EINVAL;
 			}
@@ -1197,7 +1197,7 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 			pr_debug("map '%s': found key [%u], sz = %lld.\n",
 				 map_name, t->type, sz);
 			if (map->def.key_size && map->def.key_size != sz) {
-				pr_warning("map '%s': conflictling key size %u != %lld.\n",
+				pr_warning("map '%s': conflicting key size %u != %lld.\n",
 					   map_name, map->def.key_size, sz);
 				return -EINVAL;
 			}
@@ -1212,7 +1212,7 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 			pr_debug("map '%s': found value_size = %u.\n",
 				 map_name, sz);
 			if (map->def.value_size && map->def.value_size != sz) {
-				pr_warning("map '%s': conflictling value size %u != %u.\n",
+				pr_warning("map '%s': conflicting value size %u != %u.\n",
 					   map_name, map->def.value_size, sz);
 				return -EINVAL;
 			}
@@ -1240,7 +1240,7 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 			pr_debug("map '%s': found value [%u], sz = %lld.\n",
 				 map_name, t->type, sz);
 			if (map->def.value_size && map->def.value_size != sz) {
-				pr_warning("map '%s': conflictling value size %u != %lld.\n",
+				pr_warning("map '%s': conflicting value size %u != %lld.\n",
 					   map_name, map->def.value_size, sz);
 				return -EINVAL;
 			}
-- 
2.20.1


^ permalink raw reply related

* [PATCH][bpf] bpf: verifier: add break statement in switch
From: Gustavo A. R. Silva @ 2019-06-19 16:02 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, Lawrence Brakmo, Andrii Nakryiko
  Cc: netdev, bpf, linux-kernel, Gustavo A. R. Silva, Kees Cook

Notice that in this case, it's much clearer to explicitly add a break
rather than letting the code to fall through. It also avoid potential
future fall-through warnings[1].

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

[1] https://lore.kernel.org/patchwork/patch/1087056/

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 kernel/bpf/verifier.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d2c8a6677ac4..0acf7c569ec6 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5365,6 +5365,7 @@ static int check_return_code(struct bpf_verifier_env *env)
 		if (env->prog->expected_attach_type == BPF_CGROUP_UDP4_RECVMSG ||
 		    env->prog->expected_attach_type == BPF_CGROUP_UDP6_RECVMSG)
 			range = tnum_range(1, 1);
+		break;
 	case BPF_PROG_TYPE_CGROUP_SKB:
 	case BPF_PROG_TYPE_CGROUP_SOCK:
 	case BPF_PROG_TYPE_SOCK_OPS:
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH] mm: mempolicy: handle vma with unmovable pages mapped correctly in mbind
From: Yang Shi @ 2019-06-19 16:21 UTC (permalink / raw)
  To: Vlastimil Babka, Michal Hocko
  Cc: akpm, mgorman, linux-mm, linux-kernel, Eric Dumazet,
	David S. Miller, netdev
In-Reply-To: <21a0b20c-5b62-490e-ad8e-26b4b78ac095@suse.cz>



On 6/19/19 1:22 AM, Vlastimil Babka wrote:
> On 6/19/19 7:21 AM, Michal Hocko wrote:
>> On Tue 18-06-19 14:13:16, Yang Shi wrote:
>> [...]
>>> I used to have !__PageMovable(page), but it was removed since the
>>> aforementioned reason. I could add it back.
>>>
>>> For the temporary off LRU page, I did a quick search, it looks the most
>>> paths have to acquire mmap_sem, so it can't race with us here. Page
>>> reclaim/compaction looks like the only race. But, since the mapping should
>>> be preserved even though the page is off LRU temporarily unless the page is
>>> reclaimed, so we should be able to exclude temporary off LRU pages by
>>> calling page_mapping() and page_anon_vma().
>>>
>>> So, the fix may look like:
>>>
>>> if (!PageLRU(head) && !__PageMovable(page)) {
>>>      if (!(page_mapping(page) || page_anon_vma(page)))
>>>          return -EIO;
>> This is getting even more muddy TBH. Is there any reason that we have to
>> handle this problem during the isolation phase rather the migration?
> I think it was already said that if pages can't be isolated, then
> migration phase won't process them, so they're just ignored.

Yes,exactly.

> However I think the patch is wrong to abort immediately when
> encountering such page that cannot be isolated (AFAICS). IMHO it should
> still try to migrate everything it can, and only then return -EIO.

It is fine too. I don't see mbind semantics define how to handle such 
case other than returning -EIO.



^ permalink raw reply

* Re: linux-next: build failure after merge of the net-next tree
From: Kevin 'ldir' Darbyshire-Bryant @ 2019-06-19 16:11 UTC (permalink / raw)
  To: David Miller
  Cc: Stephen Rothwell, Networking, Linux Next Mailing List,
	Linux Kernel Mailing List, yamada.masahiro@socionext.com
In-Reply-To: <FBCC967B-578C-459A-95EF-4AC123511B12@darbyshire-bryant.me.uk>

[-- Attachment #1: Type: text/plain, Size: 2062 bytes --]



> On 19 Jun 2019, at 16:09, Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote:
> 
> 
> 
>> On 19 Jun 2019, at 15:13, David Miller <davem@davemloft.net> wrote:
>> 
>> 
>> I've fixed this as follows, thanks:
>> 
>> ====================
>> From 23cdf8752b26d4edbd60a6293bca492d83192d4d Mon Sep 17 00:00:00 2001
>> From: "David S. Miller" <davem@davemloft.net>
>> Date: Wed, 19 Jun 2019 10:12:58 -0400
>> Subject: [PATCH] act_ctinfo: Don't use BIT() in UAPI headers.
>> 
>> Use _BITUL() instead.
>> 
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>> ---
>> include/uapi/linux/tc_act/tc_ctinfo.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/include/uapi/linux/tc_act/tc_ctinfo.h b/include/uapi/linux/tc_act/tc_ctinfo.h
>> index da803e05a89b..32337304fbe5 100644
>> --- a/include/uapi/linux/tc_act/tc_ctinfo.h
>> +++ b/include/uapi/linux/tc_act/tc_ctinfo.h
>> @@ -27,8 +27,8 @@ enum {
>> #define TCA_CTINFO_MAX (__TCA_CTINFO_MAX - 1)
>> 
>> enum {
>> -	CTINFO_MODE_DSCP	= BIT(0),
>> -	CTINFO_MODE_CPMARK	= BIT(1)
>> +	CTINFO_MODE_DSCP	= _BITUL(0),
>> +	CTINFO_MODE_CPMARK	= _BITUL(1)
>> };
>> 
>> #endif
>> --
>> 2.20.1
>> 
> 
> Hi David,
> 
> Thanks for that.  Owe you a beer!
> 
> Thinking out loud, doesn’t this also require:
> 
> #include <linux/const.h>
> 
> 
> Or at least iproute2 would need to know about _BITUL as it doesn’t at present.
> Which also means iproute2’s Linux uapi shadow would also have to import
> linux/const.h.  Or have I got wrong end of stick?
> 
> Cheers,
> 
> Kevin
> 

Whilst out walking the dog I realised I’m a moron.

The CTINFO_MODE_FOO is only used within module, it shouldn’t even be exposed
to user space.

I’ll send a patch shortly.  Sorry, I’m pretty embarrassed at how something so
apparently simple on the surface has had so many issues.

Cheers,

Kevin D-B

gpg: 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 0/9] keys: Namespacing [ver #4]
From: David Howells @ 2019-06-19 16:09 UTC (permalink / raw)
  Cc: dhowells, ebiederm, keyrings, linux-cifs, linux-nfs, netdev,
	linux-afs, dwalsh, vgoyal, linux-security-module, linux-fsdevel,
	linux-kernel
In-Reply-To: <156096036064.6697.2432500504898119675.stgit@warthog.procyon.org.uk>

Apologies...  A couple of the patches don't compile because I forgot to commit
a change:-(

Will repush and repost.

David

^ permalink raw reply

* Re: [PATCH net-next 3/5] ipv6: honor RT6_LOOKUP_F_DST_NOREF in rule lookup logic
From: David Miller @ 2019-06-19 16:07 UTC (permalink / raw)
  To: tracywwnj; +Cc: netdev, kafai, edumazet, maheshb, dsahern, weiwan
In-Reply-To: <20190618182543.65477-4-tracywwnj@gmail.com>

From: Wei Wang <tracywwnj@gmail.com>
Date: Tue, 18 Jun 2019 11:25:41 -0700

> @@ -237,13 +240,16 @@ static int __fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
>  			goto out;
>  	}
>  again:
> -	ip6_rt_put(rt);
> +	if (!(flags & RT6_LOOKUP_F_DST_NOREF) ||
> +	    !list_empty(&rt->rt6i_uncached))
> +		ip6_rt_put(rt);

This conditional release logic, with the special treatment of uncache items
when using DST_NOREF, seems error prone.

Maybe you can put this logic into a helper like ip6_rt_put_any() and do the
list empty test etc. there?

	ip6_rt_put_any(struct rt6_info *rt, int flags);

What do you think?

^ permalink raw reply

* [PATCH 5/9] keys: Move the user and user-session keyrings to the user_namespace [ver #4]
From: David Howells @ 2019-06-19 16:06 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096036064.6697.2432500504898119675.stgit@warthog.procyon.org.uk>

Move the user and user-session keyrings to the user_namespace struct rather
than pinning them from the user_struct struct.  This prevents these
keyrings from propagating across user-namespaces boundaries with regard to
the KEY_SPEC_* flags, thereby making them more useful in a containerised
environment.

The issue is that a single user_struct may be represent UIDs in several
different namespaces.

The way the patch does this is by attaching a 'register keyring' in each
user_namespace and then sticking the user and user-session keyrings into
that.  It can then be searched to retrieve them.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jann Horn <jannh@google.com>
---

 include/linux/sched/user.h     |   14 --
 include/linux/user_namespace.h |    9 +
 kernel/user.c                  |    7 -
 kernel/user_namespace.c        |    4 -
 security/keys/internal.h       |    3 
 security/keys/keyring.c        |    1 
 security/keys/persistent.c     |    8 +
 security/keys/process_keys.c   |  259 ++++++++++++++++++++++++++--------------
 security/keys/request_key.c    |   20 ++-
 9 files changed, 196 insertions(+), 129 deletions(-)

diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h
index 468d2565a9fe..917d88edb7b9 100644
--- a/include/linux/sched/user.h
+++ b/include/linux/sched/user.h
@@ -7,8 +7,6 @@
 #include <linux/refcount.h>
 #include <linux/ratelimit.h>
 
-struct key;
-
 /*
  * Some day this will be a full-fledged user tracking system..
  */
@@ -30,18 +28,6 @@ struct user_struct {
 	unsigned long unix_inflight;	/* How many files in flight in unix sockets */
 	atomic_long_t pipe_bufs;  /* how many pages are allocated in pipe buffers */
 
-#ifdef CONFIG_KEYS
-	/*
-	 * These pointers can only change from NULL to a non-NULL value once.
-	 * Writes are protected by key_user_keyring_mutex.
-	 * Unlocked readers should use READ_ONCE() unless they know that
-	 * install_user_keyrings() has been called successfully (which sets
-	 * these members to non-NULL values, preventing further modifications).
-	 */
-	struct key *uid_keyring;	/* UID specific keyring */
-	struct key *session_keyring;	/* UID's default session keyring */
-#endif
-
 	/* Hash table maintenance information */
 	struct hlist_node uidhash_node;
 	kuid_t uid;
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 90457015fa3f..fb9f4f799554 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -65,14 +65,19 @@ struct user_namespace {
 	unsigned long		flags;
 
 #ifdef CONFIG_KEYS
-	/* List of joinable keyrings in this namespace */
+	/* List of joinable keyrings in this namespace.  Modification access of
+	 * these pointers is controlled by keyring_sem.  Once
+	 * user_keyring_register is set, it won't be changed, so it can be
+	 * accessed directly with READ_ONCE().
+	 */
 	struct list_head	keyring_name_list;
+	struct key		*user_keyring_register;
+	struct rw_semaphore	keyring_sem;
 #endif
 
 	/* Register of per-UID persistent keyrings for this namespace */
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	struct key		*persistent_keyring_register;
-	struct rw_semaphore	persistent_keyring_register_sem;
 #endif
 	struct work_struct	work;
 #ifdef CONFIG_SYSCTL
diff --git a/kernel/user.c b/kernel/user.c
index 50979fd1b7aa..f8519b62cf9a 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -64,10 +64,7 @@ struct user_namespace init_user_ns = {
 	.flags = USERNS_INIT_FLAGS,
 #ifdef CONFIG_KEYS
 	.keyring_name_list = LIST_HEAD_INIT(init_user_ns.keyring_name_list),
-#endif
-#ifdef CONFIG_PERSISTENT_KEYRINGS
-	.persistent_keyring_register_sem =
-	__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
+	.keyring_sem = __RWSEM_INITIALIZER(init_user_ns.keyring_sem),
 #endif
 };
 EXPORT_SYMBOL_GPL(init_user_ns);
@@ -143,8 +140,6 @@ static void free_user(struct user_struct *up, unsigned long flags)
 {
 	uid_hash_remove(up);
 	spin_unlock_irqrestore(&uidhash_lock, flags);
-	key_put(up->uid_keyring);
-	key_put(up->session_keyring);
 	kmem_cache_free(uid_cachep, up);
 }
 
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index bda6e890ad88..c87c2ecc7085 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -135,9 +135,7 @@ int create_user_ns(struct cred *new)
 
 #ifdef CONFIG_KEYS
 	INIT_LIST_HEAD(&ns->keyring_name_list);
-#endif
-#ifdef CONFIG_PERSISTENT_KEYRINGS
-	init_rwsem(&ns->persistent_keyring_register_sem);
+	init_rwsem(&ns->keyring_sem);
 #endif
 	ret = -ENOMEM;
 	if (!setup_userns_sysctls(ns))
diff --git a/security/keys/internal.h b/security/keys/internal.h
index aa361299a3ec..d3a9439e2386 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -148,7 +148,8 @@ extern key_ref_t search_process_keyrings_rcu(struct keyring_search_context *ctx)
 
 extern struct key *find_keyring_by_name(const char *name, bool uid_keyring);
 
-extern int install_user_keyrings(void);
+extern int look_up_user_keyrings(struct key **, struct key **);
+extern struct key *get_user_session_keyring_rcu(const struct cred *);
 extern int install_thread_keyring_to_cred(struct cred *);
 extern int install_process_keyring_to_cred(struct cred *);
 extern int install_session_keyring_to_cred(struct cred *, struct key *);
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index fe851292509e..3663e5168583 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -62,6 +62,7 @@ void key_free_user_ns(struct user_namespace *ns)
 	list_del_init(&ns->keyring_name_list);
 	write_unlock(&keyring_name_lock);
 
+	key_put(ns->user_keyring_register);
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	key_put(ns->persistent_keyring_register);
 #endif
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index fc29ec59efa7..90303fe4a394 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -91,9 +91,9 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
 
 	if (ns->persistent_keyring_register) {
 		reg_ref = make_key_ref(ns->persistent_keyring_register, true);
-		down_read(&ns->persistent_keyring_register_sem);
+		down_read(&ns->keyring_sem);
 		persistent_ref = find_key_to_update(reg_ref, &index_key);
-		up_read(&ns->persistent_keyring_register_sem);
+		up_read(&ns->keyring_sem);
 
 		if (persistent_ref)
 			goto found;
@@ -102,9 +102,9 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
 	/* It wasn't in the register, so we'll need to create it.  We might
 	 * also need to create the register.
 	 */
-	down_write(&ns->persistent_keyring_register_sem);
+	down_write(&ns->keyring_sem);
 	persistent_ref = key_create_persistent(ns, uid, &index_key);
-	up_write(&ns->persistent_keyring_register_sem);
+	up_write(&ns->keyring_sem);
 	if (!IS_ERR(persistent_ref))
 		goto found;
 
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index b07f768d23dc..f74d64215942 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -19,15 +19,13 @@
 #include <linux/security.h>
 #include <linux/user_namespace.h>
 #include <linux/uaccess.h>
+#include <linux/init_task.h>
 #include <keys/request_key_auth-type.h>
 #include "internal.h"
 
 /* Session keyring create vs join semaphore */
 static DEFINE_MUTEX(key_session_mutex);
 
-/* User keyring creation semaphore */
-static DEFINE_MUTEX(key_user_keyring_mutex);
-
 /* The root user's tracking struct */
 struct key_user root_key_user = {
 	.usage		= REFCOUNT_INIT(3),
@@ -39,98 +37,185 @@ struct key_user root_key_user = {
 };
 
 /*
- * Install the user and user session keyrings for the current process's UID.
+ * Get or create a user register keyring.
+ */
+static struct key *get_user_register(struct user_namespace *user_ns)
+{
+	struct key *reg_keyring = READ_ONCE(user_ns->user_keyring_register);
+
+	if (reg_keyring)
+		return reg_keyring;
+
+	down_write(&user_ns->keyring_sem);
+
+	/* Make sure there's a register keyring.  It gets owned by the
+	 * user_namespace's owner.
+	 */
+	reg_keyring = user_ns->user_keyring_register;
+	if (!reg_keyring) {
+		reg_keyring = keyring_alloc(".user_reg",
+					    user_ns->owner, INVALID_GID,
+					    &init_cred,
+					    KEY_POS_WRITE | KEY_POS_SEARCH |
+					    KEY_USR_VIEW | KEY_USR_READ,
+					    0,
+					    NULL, NULL);
+		if (!IS_ERR(reg_keyring))
+			smp_store_release(&user_ns->user_keyring_register,
+					  reg_keyring);
+	}
+
+	up_write(&user_ns->keyring_sem);
+
+	/* We don't return a ref since the keyring is pinned by the user_ns */
+	return reg_keyring;
+}
+
+/*
+ * Look up the user and user session keyrings for the current process's UID,
+ * creating them if they don't exist.
  */
-int install_user_keyrings(void)
+int look_up_user_keyrings(struct key **_user_keyring,
+			  struct key **_user_session_keyring)
 {
-	struct user_struct *user;
-	const struct cred *cred;
-	struct key *uid_keyring, *session_keyring;
+	const struct cred *cred = current_cred();
+	struct user_namespace *user_ns = current_user_ns();
+	struct key *reg_keyring, *uid_keyring, *session_keyring;
 	key_perm_t user_keyring_perm;
+	key_ref_t uid_keyring_r, session_keyring_r;
+	uid_t uid = from_kuid(user_ns, cred->user->uid);
 	char buf[20];
 	int ret;
-	uid_t uid;
 
 	user_keyring_perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_ALL;
-	cred = current_cred();
-	user = cred->user;
-	uid = from_kuid(cred->user_ns, user->uid);
 
-	kenter("%p{%u}", user, uid);
+	kenter("%u", uid);
 
-	if (READ_ONCE(user->uid_keyring) && READ_ONCE(user->session_keyring)) {
-		kleave(" = 0 [exist]");
-		return 0;
-	}
+	reg_keyring = get_user_register(user_ns);
+	if (IS_ERR(reg_keyring))
+		return PTR_ERR(reg_keyring);
 
-	mutex_lock(&key_user_keyring_mutex);
+	down_write(&user_ns->keyring_sem);
 	ret = 0;
 
-	if (!user->uid_keyring) {
-		/* get the UID-specific keyring
-		 * - there may be one in existence already as it may have been
-		 *   pinned by a session, but the user_struct pointing to it
-		 *   may have been destroyed by setuid */
-		sprintf(buf, "_uid.%u", uid);
-
-		uid_keyring = find_keyring_by_name(buf, true);
+	/* Get the user keyring.  Note that there may be one in existence
+	 * already as it may have been pinned by a session, but the user_struct
+	 * pointing to it may have been destroyed by setuid.
+	 */
+	snprintf(buf, sizeof(buf), "_uid.%u", uid);
+	uid_keyring_r = keyring_search(make_key_ref(reg_keyring, true),
+				       &key_type_keyring, buf, false);
+	kdebug("_uid %p", uid_keyring_r);
+	if (uid_keyring_r == ERR_PTR(-EAGAIN)) {
+		uid_keyring = keyring_alloc(buf, cred->user->uid, INVALID_GID,
+					    cred, user_keyring_perm,
+					    KEY_ALLOC_UID_KEYRING |
+					    KEY_ALLOC_IN_QUOTA,
+					    NULL, reg_keyring);
 		if (IS_ERR(uid_keyring)) {
-			uid_keyring = keyring_alloc(buf, user->uid, INVALID_GID,
-						    cred, user_keyring_perm,
-						    KEY_ALLOC_UID_KEYRING |
-							KEY_ALLOC_IN_QUOTA,
-						    NULL, NULL);
-			if (IS_ERR(uid_keyring)) {
-				ret = PTR_ERR(uid_keyring);
-				goto error;
-			}
+			ret = PTR_ERR(uid_keyring);
+			goto error;
 		}
+	} else if (IS_ERR(uid_keyring_r)) {
+		ret = PTR_ERR(uid_keyring_r);
+		goto error;
+	} else {
+		uid_keyring = key_ref_to_ptr(uid_keyring_r);
+	}
 
-		/* get a default session keyring (which might also exist
-		 * already) */
-		sprintf(buf, "_uid_ses.%u", uid);
-
-		session_keyring = find_keyring_by_name(buf, true);
+	/* Get a default session keyring (which might also exist already) */
+	snprintf(buf, sizeof(buf), "_uid_ses.%u", uid);
+	session_keyring_r = keyring_search(make_key_ref(reg_keyring, true),
+					   &key_type_keyring, buf, false);
+	kdebug("_uid_ses %p", session_keyring_r);
+	if (session_keyring_r == ERR_PTR(-EAGAIN)) {
+		session_keyring = keyring_alloc(buf, cred->user->uid, INVALID_GID,
+						cred, user_keyring_perm,
+						KEY_ALLOC_UID_KEYRING |
+						KEY_ALLOC_IN_QUOTA,
+						NULL, NULL);
 		if (IS_ERR(session_keyring)) {
-			session_keyring =
-				keyring_alloc(buf, user->uid, INVALID_GID,
-					      cred, user_keyring_perm,
-					      KEY_ALLOC_UID_KEYRING |
-						  KEY_ALLOC_IN_QUOTA,
-					      NULL, NULL);
-			if (IS_ERR(session_keyring)) {
-				ret = PTR_ERR(session_keyring);
-				goto error_release;
-			}
-
-			/* we install a link from the user session keyring to
-			 * the user keyring */
-			ret = key_link(session_keyring, uid_keyring);
-			if (ret < 0)
-				goto error_release_both;
+			ret = PTR_ERR(session_keyring);
+			goto error_release;
 		}
 
-		/* install the keyrings */
-		/* paired with READ_ONCE() */
-		smp_store_release(&user->uid_keyring, uid_keyring);
-		/* paired with READ_ONCE() */
-		smp_store_release(&user->session_keyring, session_keyring);
+		/* We install a link from the user session keyring to
+		 * the user keyring.
+		 */
+		ret = key_link(session_keyring, uid_keyring);
+		if (ret < 0)
+			goto error_release_session;
+
+		/* And only then link the user-session keyring to the
+		 * register.
+		 */
+		ret = key_link(reg_keyring, session_keyring);
+		if (ret < 0)
+			goto error_release_session;
+	} else if (IS_ERR(session_keyring_r)) {
+		ret = PTR_ERR(session_keyring_r);
+		goto error_release;
+	} else {
+		session_keyring = key_ref_to_ptr(session_keyring_r);
 	}
 
-	mutex_unlock(&key_user_keyring_mutex);
+	up_write(&user_ns->keyring_sem);
+
+	if (_user_session_keyring)
+		*_user_session_keyring = session_keyring;
+	else
+		key_put(session_keyring);
+	if (_user_keyring)
+		*_user_keyring = uid_keyring;
+	else
+		key_put(uid_keyring);
 	kleave(" = 0");
 	return 0;
 
-error_release_both:
+error_release_session:
 	key_put(session_keyring);
 error_release:
 	key_put(uid_keyring);
 error:
-	mutex_unlock(&key_user_keyring_mutex);
+	up_write(&user_ns->keyring_sem);
 	kleave(" = %d", ret);
 	return ret;
 }
 
+/*
+ * Get the user session keyring if it exists, but don't create it if it
+ * doesn't.
+ */
+struct key *get_user_session_keyring_rcu(const struct cred *cred)
+{
+	struct key *reg_keyring = READ_ONCE(cred->user_ns->user_keyring_register);
+	key_ref_t session_keyring_r;
+	char buf[20];
+
+	struct keyring_search_context ctx = {
+		.index_key.type		= &key_type_keyring,
+		.index_key.description	= buf,
+		.cred			= cred,
+		.match_data.cmp		= key_default_cmp,
+		.match_data.raw_data	= buf,
+		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
+	};
+
+	if (!reg_keyring)
+		return NULL;
+
+	ctx.index_key.desc_len = snprintf(buf, sizeof(buf), "_uid_ses.%u",
+					  from_kuid(cred->user_ns,
+						    cred->user->uid));
+
+	session_keyring_r = keyring_search_rcu(make_key_ref(reg_keyring, true),
+					       &ctx);
+	if (IS_ERR(session_keyring_r))
+		return NULL;
+	return key_ref_to_ptr(session_keyring_r);
+}
+
 /*
  * Install a thread keyring to the given credentials struct if it didn't have
  * one already.  This is allowed to overrun the quota.
@@ -340,6 +425,7 @@ void key_fsgid_changed(struct cred *new_cred)
  */
 key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx)
 {
+	struct key *user_session;
 	key_ref_t key_ref, ret, err;
 	const struct cred *cred = ctx->cred;
 
@@ -415,10 +501,11 @@ key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx)
 		}
 	}
 	/* or search the user-session keyring */
-	else if (READ_ONCE(cred->user->session_keyring)) {
-		key_ref = keyring_search_rcu(
-			make_key_ref(READ_ONCE(cred->user->session_keyring), 1),
-			ctx);
+	else if ((user_session = get_user_session_keyring_rcu(cred))) {
+		key_ref = keyring_search_rcu(make_key_ref(user_session, 1),
+					     ctx);
+		key_put(user_session);
+
 		if (!IS_ERR(key_ref))
 			goto found;
 
@@ -535,7 +622,7 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 					   KEYRING_SEARCH_RECURSE),
 	};
 	struct request_key_auth *rka;
-	struct key *key;
+	struct key *key, *user_session;
 	key_ref_t key_ref, skey_ref;
 	int ret;
 
@@ -584,20 +671,20 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 		if (!ctx.cred->session_keyring) {
 			/* always install a session keyring upon access if one
 			 * doesn't exist yet */
-			ret = install_user_keyrings();
+			ret = look_up_user_keyrings(NULL, &user_session);
 			if (ret < 0)
 				goto error;
 			if (lflags & KEY_LOOKUP_CREATE)
 				ret = join_session_keyring(NULL);
 			else
-				ret = install_session_keyring(
-					ctx.cred->user->session_keyring);
+				ret = install_session_keyring(user_session);
 
+			key_put(user_session);
 			if (ret < 0)
 				goto error;
 			goto reget_creds;
-		} else if (ctx.cred->session_keyring ==
-			   READ_ONCE(ctx.cred->user->session_keyring) &&
+		} else if (test_bit(KEY_FLAG_UID_KEYRING,
+				    &ctx.cred->session_keyring->flags) &&
 			   lflags & KEY_LOOKUP_CREATE) {
 			ret = join_session_keyring(NULL);
 			if (ret < 0)
@@ -611,26 +698,16 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 		break;
 
 	case KEY_SPEC_USER_KEYRING:
-		if (!READ_ONCE(ctx.cred->user->uid_keyring)) {
-			ret = install_user_keyrings();
-			if (ret < 0)
-				goto error;
-		}
-
-		key = ctx.cred->user->uid_keyring;
-		__key_get(key);
+		ret = look_up_user_keyrings(&key, NULL);
+		if (ret < 0)
+			goto error;
 		key_ref = make_key_ref(key, 1);
 		break;
 
 	case KEY_SPEC_USER_SESSION_KEYRING:
-		if (!READ_ONCE(ctx.cred->user->session_keyring)) {
-			ret = install_user_keyrings();
-			if (ret < 0)
-				goto error;
-		}
-
-		key = ctx.cred->user->session_keyring;
-		__key_get(key);
+		ret = look_up_user_keyrings(NULL, &key);
+		if (ret < 0)
+			goto error;
 		key_ref = make_key_ref(key, 1);
 		break;
 
@@ -879,7 +956,7 @@ void key_change_session_keyring(struct callback_head *twork)
  */
 static int __init init_root_keyring(void)
 {
-	return install_user_keyrings();
+	return look_up_user_keyrings(NULL, NULL);
 }
 
 late_initcall(init_root_keyring);
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 1ffd3803ce29..9201ca96c4df 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -121,7 +121,7 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
 	struct request_key_auth *rka = get_request_key_auth(authkey);
 	const struct cred *cred = current_cred();
 	key_serial_t prkey, sskey;
-	struct key *key = rka->target_key, *keyring, *session;
+	struct key *key = rka->target_key, *keyring, *session, *user_session;
 	char *argv[9], *envp[3], uid_str[12], gid_str[12];
 	char key_str[12], keyring_str[3][12];
 	char desc[20];
@@ -129,9 +129,9 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
 
 	kenter("{%d},{%d},%s", key->serial, authkey->serial, rka->op);
 
-	ret = install_user_keyrings();
+	ret = look_up_user_keyrings(NULL, &user_session);
 	if (ret < 0)
-		goto error_alloc;
+		goto error_us;
 
 	/* allocate a new session keyring */
 	sprintf(desc, "_req.%u", key->serial);
@@ -169,7 +169,7 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
 
 	session = cred->session_keyring;
 	if (!session)
-		session = cred->user->session_keyring;
+		session = user_session;
 	sskey = session->serial;
 
 	sprintf(keyring_str[2], "%d", sskey);
@@ -211,6 +211,8 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
 	key_put(keyring);
 
 error_alloc:
+	key_put(user_session);
+error_us:
 	complete_request_key(authkey, ret);
 	kleave(" = %d", ret);
 	return ret;
@@ -317,13 +319,15 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
 
 			/* fall through */
 		case KEY_REQKEY_DEFL_USER_SESSION_KEYRING:
-			dest_keyring =
-				key_get(READ_ONCE(cred->user->session_keyring));
+			ret = look_up_user_keyrings(NULL, &dest_keyring);
+			if (ret < 0)
+				return ret;
 			break;
 
 		case KEY_REQKEY_DEFL_USER_KEYRING:
-			dest_keyring =
-				key_get(READ_ONCE(cred->user->uid_keyring));
+			ret = look_up_user_keyrings(&dest_keyring, NULL);
+			if (ret < 0)
+				return ret;
 			break;
 
 		case KEY_REQKEY_DEFL_GROUP_KEYRING:


^ permalink raw reply related

* [PATCH 4/9] keys: Namespace keyring names [ver #4]
From: David Howells @ 2019-06-19 16:06 UTC (permalink / raw)
  To: ebiederm, keyrings
  Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
	vgoyal, linux-security-module, linux-fsdevel, linux-kernel
In-Reply-To: <156096036064.6697.2432500504898119675.stgit@warthog.procyon.org.uk>

Keyring names are held in a single global list that any process can pick
from by means of keyctl_join_session_keyring (provided the keyring grants
Search permission).  This isn't very container friendly, however.

Make the following changes:

 (1) Make default session, process and thread keyring names begin with a
     '.' instead of '_'.

 (2) Keyrings whose names begin with a '.' aren't added to the list.  Such
     keyrings are system specials.

 (3) Replace the global list with per-user_namespace lists.  A keyring adds
     its name to the list for the user_namespace that it is currently in.

 (4) When a user_namespace is deleted, it just removes itself from the
     keyring name list.

The global keyring_name_lock is retained for accessing the name lists.
This allows (4) to work.

This can be tested by:

	# keyctl newring foo @s
	995906392
	# unshare -U
	$ keyctl show
	...
	 995906392 --alswrv  65534 65534   \_ keyring: foo
	...
	$ keyctl session foo
	Joined session keyring: 935622349

As can be seen, a new session keyring was created.

The capability bit KEYCTL_CAPS1_NS_KEYRING_NAME is set if the kernel is
employing this feature.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Eric W. Biederman <ebiederm@xmission.com>
---

 include/linux/key.h            |    2 +
 include/linux/user_namespace.h |    5 ++
 include/uapi/linux/keyctl.h    |    1 
 kernel/user.c                  |    3 +
 kernel/user_namespace.c        |    7 ++-
 security/keys/keyctl.c         |    1 
 security/keys/keyring.c        |   99 +++++++++++++++++-----------------------
 7 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index ff102731b3db..ae1177302d70 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -361,6 +361,7 @@ extern void key_set_timeout(struct key *, unsigned);
 
 extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
 				 key_perm_t perm);
+extern void key_free_user_ns(struct user_namespace *);
 
 /*
  * The permissions required on a key that we're looking up.
@@ -434,6 +435,7 @@ extern void key_init(void);
 #define key_fsuid_changed(c)		do { } while(0)
 #define key_fsgid_changed(c)		do { } while(0)
 #define key_init()			do { } while(0)
+#define key_free_user_ns(ns)		do { } while(0)
 
 #endif /* CONFIG_KEYS */
 #endif /* __KERNEL__ */
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index d6b74b91096b..90457015fa3f 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -64,6 +64,11 @@ struct user_namespace {
 	struct ns_common	ns;
 	unsigned long		flags;
 
+#ifdef CONFIG_KEYS
+	/* List of joinable keyrings in this namespace */
+	struct list_head	keyring_name_list;
+#endif
+
 	/* Register of per-UID persistent keyrings for this namespace */
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	struct key		*persistent_keyring_register;
diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index 551b5814f53e..35b405034674 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -128,5 +128,6 @@ struct keyctl_pkey_params {
 #define KEYCTL_CAPS0_INVALIDATE		0x20 /* KEYCTL_INVALIDATE supported */
 #define KEYCTL_CAPS0_RESTRICT_KEYRING	0x40 /* KEYCTL_RESTRICT_KEYRING supported */
 #define KEYCTL_CAPS0_MOVE		0x80 /* KEYCTL_MOVE supported */
+#define KEYCTL_CAPS1_NS_KEYRING_NAME	0x01 /* Keyring names are per-user_namespace */
 
 #endif /*  _LINUX_KEYCTL_H */
diff --git a/kernel/user.c b/kernel/user.c
index 88b834f0eebc..50979fd1b7aa 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -62,6 +62,9 @@ struct user_namespace init_user_ns = {
 	.ns.ops = &userns_operations,
 #endif
 	.flags = USERNS_INIT_FLAGS,
+#ifdef CONFIG_KEYS
+	.keyring_name_list = LIST_HEAD_INIT(init_user_ns.keyring_name_list),
+#endif
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	.persistent_keyring_register_sem =
 	__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 923414a246e9..bda6e890ad88 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -133,6 +133,9 @@ int create_user_ns(struct cred *new)
 	ns->flags = parent_ns->flags;
 	mutex_unlock(&userns_state_mutex);
 
+#ifdef CONFIG_KEYS
+	INIT_LIST_HEAD(&ns->keyring_name_list);
+#endif
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	init_rwsem(&ns->persistent_keyring_register_sem);
 #endif
@@ -196,9 +199,7 @@ static void free_user_ns(struct work_struct *work)
 			kfree(ns->projid_map.reverse);
 		}
 		retire_userns_sysctls(ns);
-#ifdef CONFIG_PERSISTENT_KEYRINGS
-		key_put(ns->persistent_keyring_register);
-#endif
+		key_free_user_ns(ns);
 		ns_free_inum(&ns->ns);
 		kmem_cache_free(user_ns_cachep, ns);
 		dec_user_namespaces(ucounts);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 169409b611b0..37d2940cb10a 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -40,6 +40,7 @@ static const unsigned char keyrings_capabilities[1] = {
 	       KEYCTL_CAPS0_RESTRICT_KEYRING |
 	       KEYCTL_CAPS0_MOVE
 	       ),
+	[1] = (KEYCTL_CAPS1_NS_KEYRING_NAME),
 };
 
 static int key_get_type_from_user(char *type,
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 20891cd198f0..fe851292509e 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -16,6 +16,7 @@
 #include <linux/security.h>
 #include <linux/seq_file.h>
 #include <linux/err.h>
+#include <linux/user_namespace.h>
 #include <keys/keyring-type.h>
 #include <keys/user-type.h>
 #include <linux/assoc_array_priv.h>
@@ -28,11 +29,6 @@
  */
 #define KEYRING_SEARCH_MAX_DEPTH 6
 
-/*
- * We keep all named keyrings in a hash to speed looking them up.
- */
-#define KEYRING_NAME_HASH_SIZE	(1 << 5)
-
 /*
  * We mark pointers we pass to the associative array with bit 1 set if
  * they're keyrings and clear otherwise.
@@ -55,17 +51,20 @@ static inline void *keyring_key_to_ptr(struct key *key)
 	return key;
 }
 
-static struct list_head	keyring_name_hash[KEYRING_NAME_HASH_SIZE];
 static DEFINE_RWLOCK(keyring_name_lock);
 
-static inline unsigned keyring_hash(const char *desc)
+/*
+ * Clean up the bits of user_namespace that belong to us.
+ */
+void key_free_user_ns(struct user_namespace *ns)
 {
-	unsigned bucket = 0;
-
-	for (; *desc; desc++)
-		bucket += (unsigned char)*desc;
+	write_lock(&keyring_name_lock);
+	list_del_init(&ns->keyring_name_list);
+	write_unlock(&keyring_name_lock);
 
-	return bucket & (KEYRING_NAME_HASH_SIZE - 1);
+#ifdef CONFIG_PERSISTENT_KEYRINGS
+	key_put(ns->persistent_keyring_register);
+#endif
 }
 
 /*
@@ -104,23 +103,17 @@ static DEFINE_MUTEX(keyring_serialise_link_lock);
 
 /*
  * Publish the name of a keyring so that it can be found by name (if it has
- * one).
+ * one and it doesn't begin with a dot).
  */
 static void keyring_publish_name(struct key *keyring)
 {
-	int bucket;
-
-	if (keyring->description) {
-		bucket = keyring_hash(keyring->description);
+	struct user_namespace *ns = current_user_ns();
 
+	if (keyring->description &&
+	    keyring->description[0] &&
+	    keyring->description[0] != '.') {
 		write_lock(&keyring_name_lock);
-
-		if (!keyring_name_hash[bucket].next)
-			INIT_LIST_HEAD(&keyring_name_hash[bucket]);
-
-		list_add_tail(&keyring->name_link,
-			      &keyring_name_hash[bucket]);
-
+		list_add_tail(&keyring->name_link, &ns->keyring_name_list);
 		write_unlock(&keyring_name_lock);
 	}
 }
@@ -1097,50 +1090,44 @@ key_ref_t find_key_to_update(key_ref_t keyring_ref,
  */
 struct key *find_keyring_by_name(const char *name, bool uid_keyring)
 {
+	struct user_namespace *ns = current_user_ns();
 	struct key *keyring;
-	int bucket;
 
 	if (!name)
 		return ERR_PTR(-EINVAL);
 
-	bucket = keyring_hash(name);
-
 	read_lock(&keyring_name_lock);
 
-	if (keyring_name_hash[bucket].next) {
-		/* search this hash bucket for a keyring with a matching name
-		 * that's readable and that hasn't been revoked */
-		list_for_each_entry(keyring,
-				    &keyring_name_hash[bucket],
-				    name_link
-				    ) {
-			if (!kuid_has_mapping(current_user_ns(), keyring->user->uid))
-				continue;
-
-			if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
-				continue;
+	/* Search this hash bucket for a keyring with a matching name that
+	 * grants Search permission and that hasn't been revoked
+	 */
+	list_for_each_entry(keyring, &ns->keyring_name_list, name_link) {
+		if (!kuid_has_mapping(ns, keyring->user->uid))
+			continue;
 
-			if (strcmp(keyring->description, name) != 0)
-				continue;
+		if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
+			continue;
 
-			if (uid_keyring) {
-				if (!test_bit(KEY_FLAG_UID_KEYRING,
-					      &keyring->flags))
-					continue;
-			} else {
-				if (key_permission(make_key_ref(keyring, 0),
-						   KEY_NEED_SEARCH) < 0)
-					continue;
-			}
+		if (strcmp(keyring->description, name) != 0)
+			continue;
 
-			/* we've got a match but we might end up racing with
-			 * key_cleanup() if the keyring is currently 'dead'
-			 * (ie. it has a zero usage count) */
-			if (!refcount_inc_not_zero(&keyring->usage))
+		if (uid_keyring) {
+			if (!test_bit(KEY_FLAG_UID_KEYRING,
+				      &keyring->flags))
+				continue;
+		} else {
+			if (key_permission(make_key_ref(keyring, 0),
+					   KEY_NEED_SEARCH) < 0)
 				continue;
-			keyring->last_used_at = ktime_get_real_seconds();
-			goto out;
 		}
+
+		/* we've got a match but we might end up racing with
+		 * key_cleanup() if the keyring is currently 'dead'
+		 * (ie. it has a zero usage count) */
+		if (!refcount_inc_not_zero(&keyring->usage))
+			continue;
+		keyring->last_used_at = ktime_get_real_seconds();
+		goto out;
 	}
 
 	keyring = ERR_PTR(-ENOKEY);


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox