[PATCH 0/6] Miscellaneous kdbus fixes and code removal

[PATCH 0/6] Miscellaneous kdbus fixes and code removal

[David Herrmann]

Hi Greg

A set of 6 patches to drop unused code and fix some bugs:

 #1: simple fix for test-cases
 #2: drop redundant internal accounting
 #3: drop unused structure member
 #4: drop support for an HELLO-ioctl item that was never used. If this breaks
     some user-space we're not aware of, we will have to revert it. But I
     haven't seen anything using this so far. This does *not* break ABI, but
     only cause the ioctl to fail if you pass that item.
 #5: pin namespaces on kdbus-connections to simplify namespace handling, further
     described in the commit-msg
 #6: fix a NULL-deref

Thanks
David

David Herrmann (6):
  kdbus/selftests: fix CAP translation tests
  kdbus: drop redundant KDBUS_MSG_MAX_ITEMS
  kdbus: drop unused 'bloom_generation' field
  kdbus: drop support for required attach-flags on buses
  kdbus: pin namespaces on HELLO
  kdbus: fix NULL-deref in activator cleanup

 Documentation/kdbus/kdbus.bus.xml                | 15 -------
 Documentation/kdbus/kdbus.connection.xml         |  8 +---
 ipc/kdbus/bus.c                                  | 12 +-----
 ipc/kdbus/bus.h                                  |  2 -
 ipc/kdbus/connection.c                           | 18 +++-----
 ipc/kdbus/connection.h                           |  6 +++
 ipc/kdbus/limits.h                               |  3 --
 ipc/kdbus/message.c                              |  3 --
 ipc/kdbus/message.h                              |  2 -
 ipc/kdbus/metadata.c                             | 54 ++++++++++++------------
 ipc/kdbus/metadata.h                             |  1 +
 ipc/kdbus/names.c                                |  2 +-
 ipc/kdbus/queue.c                                |  1 +
 tools/testing/selftests/kdbus/kdbus-test.c       |  1 -
 tools/testing/selftests/kdbus/kdbus-util.c       | 20 +++------
 tools/testing/selftests/kdbus/kdbus-util.h       |  3 +-
 tools/testing/selftests/kdbus/test-connection.c  |  9 ----
 tools/testing/selftests/kdbus/test-metadata-ns.c |  9 ++--
 18 files changed, 57 insertions(+), 112 deletions(-)

-- 
2.4.5

[PATCH 1/6] kdbus/selftests: fix CAP translation tests

[David Herrmann]

We now support CAP translations. Make sure our tests reflect that. So far
they made sure we drop CAPS on namespace borders. This is wrong, though.
We really need to just make sure that no _or_ the correctly translated
caps are returned. Fix this.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 tools/testing/selftests/kdbus/test-metadata-ns.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kdbus/test-metadata-ns.c b/tools/testing/selftests/kdbus/test-metadata-ns.c
index ccdfae0..1f6edc0 100644
--- a/tools/testing/selftests/kdbus/test-metadata-ns.c
+++ b/tools/testing/selftests/kdbus/test-metadata-ns.c
@@ -168,9 +168,8 @@ static int __kdbus_clone_userns_test(const char *bus,
 		ASSERT_EXIT(ret == 0);
 		ASSERT_EXIT(msg->dst_id == userns_conn->id);
 
-		/* Different namespaces no CAPS */
 		item = kdbus_get_item(msg, KDBUS_ITEM_CAPS);
-		ASSERT_EXIT(item == NULL);
+		ASSERT_EXIT(item);
 
 		/* uid/gid not mapped, so we have unpriv cached creds */
 		ret = kdbus_match_kdbus_creds(msg, &unmapped_creds);
@@ -196,9 +195,8 @@ static int __kdbus_clone_userns_test(const char *bus,
 		ASSERT_EXIT(ret == 0);
 		ASSERT_EXIT(msg->dst_id == KDBUS_DST_ID_BROADCAST);
 
-		/* Different namespaces no CAPS */
 		item = kdbus_get_item(msg, KDBUS_ITEM_CAPS);
-		ASSERT_EXIT(item == NULL);
+		ASSERT_EXIT(item);
 
 		/* uid/gid not mapped, so we have unpriv cached creds */
 		ret = kdbus_match_kdbus_creds(msg, &unmapped_creds);
@@ -358,9 +356,8 @@ static int kdbus_clone_userns_test(const char *bus,
 
 	userns_conn_id = msg->src_id;
 
-	/* We do not share the userns, os no KDBUS_ITEM_CAPS */
 	item = kdbus_get_item(msg, KDBUS_ITEM_CAPS);
-	ASSERT_RETURN(item == NULL);
+	ASSERT_RETURN(item);
 
 	/*
 	 * Compare received items, creds must be translated into
-- 
2.4.5

[PATCH 2/6] kdbus: drop redundant KDBUS_MSG_MAX_ITEMS

[David Herrmann]

We already limit the size of the message object, there's no reason to add
an arbitrary additional limit on the number of items. We don't do this for
other item-arrays, so lets stop restricting the messages in this way.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 ipc/kdbus/limits.h  | 3 ---
 ipc/kdbus/message.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/ipc/kdbus/limits.h b/ipc/kdbus/limits.h
index 6450f58..c54925a 100644
--- a/ipc/kdbus/limits.h
+++ b/ipc/kdbus/limits.h
@@ -19,9 +19,6 @@
 /* maximum size of message header and items */
 #define KDBUS_MSG_MAX_SIZE		SZ_8K
 
-/* maximum number of message items */
-#define KDBUS_MSG_MAX_ITEMS		128
-
 /* maximum number of memfd items per message */
 #define KDBUS_MSG_MAX_MEMFD_ITEMS	16
 
diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
index 066e816..c499014 100644
--- a/ipc/kdbus/message.c
+++ b/ipc/kdbus/message.c
@@ -263,9 +263,6 @@ static int kdbus_msg_scan_items(struct kdbus_kmsg *kmsg,
 		size_t payload_size = KDBUS_ITEM_PAYLOAD_SIZE(item);
 		struct iovec *iov = kmsg->iov + kmsg->iov_count;
 
-		if (++n > KDBUS_MSG_MAX_ITEMS)
-			return -E2BIG;
-
 		switch (item->type) {
 		case KDBUS_ITEM_PAYLOAD_VEC: {
 			struct kdbus_msg_data *d = res->data + res->data_count;
-- 
2.4.5

Re: [PATCH 2/6] kdbus: drop redundant KDBUS_MSG_MAX_ITEMS

[Sergei Zviagintsev]

Hi,

On Thu, Jul 02, 2015 at 10:28:30AM +0200, David Herrmann wrote:
> We already limit the size of the message object, there's no reason to add
> an arbitrary additional limit on the number of items. We don't do this for
> other item-arrays, so lets stop restricting the messages in this way.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  ipc/kdbus/limits.h  | 3 ---
>  ipc/kdbus/message.c | 3 ---
>  2 files changed, 6 deletions(-)
> 
> diff --git a/ipc/kdbus/limits.h b/ipc/kdbus/limits.h
> index 6450f58..c54925a 100644
> --- a/ipc/kdbus/limits.h
> +++ b/ipc/kdbus/limits.h
> @@ -19,9 +19,6 @@
>  /* maximum size of message header and items */
>  #define KDBUS_MSG_MAX_SIZE		SZ_8K
>  
> -/* maximum number of message items */
> -#define KDBUS_MSG_MAX_ITEMS		128
> -
>  /* maximum number of memfd items per message */
>  #define KDBUS_MSG_MAX_MEMFD_ITEMS	16
>  
> diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
> index 066e816..c499014 100644
> --- a/ipc/kdbus/message.c
> +++ b/ipc/kdbus/message.c
> @@ -263,9 +263,6 @@ static int kdbus_msg_scan_items(struct kdbus_kmsg *kmsg,
>  		size_t payload_size = KDBUS_ITEM_PAYLOAD_SIZE(item);
>  		struct iovec *iov = kmsg->iov + kmsg->iov_count;
>  
> -		if (++n > KDBUS_MSG_MAX_ITEMS)
> -			return -E2BIG;
> -

This change makes useless assigning `n' to zero at line 260. Also `n'
could be renamed to smt like `n_res' or dropped.

>  		switch (item->type) {
>  		case KDBUS_ITEM_PAYLOAD_VEC: {
>  			struct kdbus_msg_data *d = res->data + res->data_count;
> -- 
> 2.4.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/6] kdbus: drop redundant KDBUS_MSG_MAX_ITEMS

[David Herrmann]

We already limit the size of the message object, there's no reason to add
an arbitrary additional limit on the number of items. We don't do this for
other item-arrays, so lets stop restricting the messages in this way.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
v2:
 - drop redundant "n = 0" assignment
 - rename 'n' to 'n_res'

 ipc/kdbus/limits.h  |  3 ---
 ipc/kdbus/message.c | 12 ++++--------
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/ipc/kdbus/limits.h b/ipc/kdbus/limits.h
index 6450f58..c54925a 100644
--- a/ipc/kdbus/limits.h
+++ b/ipc/kdbus/limits.h
@@ -19,9 +19,6 @@
 /* maximum size of message header and items */
 #define KDBUS_MSG_MAX_SIZE		SZ_8K
 
-/* maximum number of message items */
-#define KDBUS_MSG_MAX_ITEMS		128
-
 /* maximum number of memfd items per message */
 #define KDBUS_MSG_MAX_MEMFD_ITEMS	16
 
diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
index 066e816..e9da672 100644
--- a/ipc/kdbus/message.c
+++ b/ipc/kdbus/message.c
@@ -214,7 +214,7 @@ static int kdbus_msg_scan_items(struct kdbus_kmsg *kmsg,
 	struct kdbus_msg_resources *res = kmsg->res;
 	const struct kdbus_msg *msg = &kmsg->msg;
 	const struct kdbus_item *item;
-	size_t n, n_vecs, n_memfds;
+	size_t n_res, n_vecs, n_memfds;
 	bool has_bloom = false;
 	bool has_name = false;
 	bool has_fds = false;
@@ -243,9 +243,9 @@ static int kdbus_msg_scan_items(struct kdbus_kmsg *kmsg,
 		}
 	}
 
-	n = n_vecs + n_memfds;
-	if (n > 0) {
-		res->data = kcalloc(n, sizeof(*res->data), GFP_KERNEL);
+	n_res = n_vecs + n_memfds;
+	if (n_res > 0) {
+		res->data = kcalloc(n_res, sizeof(*res->data), GFP_KERNEL);
 		if (!res->data)
 			return -ENOMEM;
 	}
@@ -257,15 +257,11 @@ static int kdbus_msg_scan_items(struct kdbus_kmsg *kmsg,
 	}
 
 	/* import data payloads */
-	n = 0;
 	vec_size = 0;
 	KDBUS_ITEMS_FOREACH(item, msg->items, KDBUS_ITEMS_SIZE(msg, items)) {
 		size_t payload_size = KDBUS_ITEM_PAYLOAD_SIZE(item);
 		struct iovec *iov = kmsg->iov + kmsg->iov_count;
 
-		if (++n > KDBUS_MSG_MAX_ITEMS)
-			return -E2BIG;
-
 		switch (item->type) {
 		case KDBUS_ITEM_PAYLOAD_VEC: {
 			struct kdbus_msg_data *d = res->data + res->data_count;
-- 
2.4.5

[PATCH 3/6] kdbus: drop unused 'bloom_generation' field

[David Herrmann]

This field is never used, drop it.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 ipc/kdbus/message.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ipc/kdbus/message.h b/ipc/kdbus/message.h
index cdaa65c..3b73347 100644
--- a/ipc/kdbus/message.h
+++ b/ipc/kdbus/message.h
@@ -88,7 +88,6 @@ kdbus_msg_resources_unref(struct kdbus_msg_resources *r);
  * @notify_name:	Short-cut for faster lookup
  * @dst_name_id:	Short-cut to msg for faster lookup
  * @bloom_filter:	Bloom filter to match message properties
- * @bloom_generation:	Generation of bloom element set
  * @notify_entry:	List of kernel-generated notifications
  * @iov:		Array of iovec, describing the payload to copy
  * @iov_count:		Number of array members in @iov
@@ -107,7 +106,6 @@ struct kdbus_kmsg {
 
 	u64 dst_name_id;
 	const struct kdbus_bloom_filter *bloom_filter;
-	u64 bloom_generation;
 	struct list_head notify_entry;
 
 	struct iovec *iov;
-- 
2.4.5

[PATCH 4/6] kdbus: drop support for required attach-flags on buses

[David Herrmann]

This drops the KDBUS_ITEM_ATTACH_FLAGS_RECV item from KDBUS_CMD_BUS_MAKE.
This item was used to provide an attach-flags mask which defines metadata
items that all connections must have in their send-mask. Hence,
effectively forcing the transmission of such items in case the receiver
wants them.

This was never used by any code and is of questionable use. With our new
effort to make sure metadata items are only transmitted if the receiver
has actual access to the same data via /proc, this is no longer needed.
Drop support for this item now.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 Documentation/kdbus/kdbus.bus.xml               | 15 ---------------
 Documentation/kdbus/kdbus.connection.xml        |  8 +-------
 ipc/kdbus/bus.c                                 | 10 ----------
 ipc/kdbus/bus.h                                 |  2 --
 ipc/kdbus/connection.c                          | 10 ----------
 tools/testing/selftests/kdbus/kdbus-test.c      |  1 -
 tools/testing/selftests/kdbus/kdbus-util.c      | 20 +++++++-------------
 tools/testing/selftests/kdbus/kdbus-util.h      |  3 +--
 tools/testing/selftests/kdbus/test-connection.c |  9 ---------
 9 files changed, 9 insertions(+), 69 deletions(-)

diff --git a/Documentation/kdbus/kdbus.bus.xml b/Documentation/kdbus/kdbus.bus.xml
index 4b9a0ac..83f1198 100644
--- a/Documentation/kdbus/kdbus.bus.xml
+++ b/Documentation/kdbus/kdbus.bus.xml
@@ -198,21 +198,6 @@ struct kdbus_cmd {
             </varlistentry>
 
             <varlistentry>
-              <term><constant>KDBUS_ITEM_ATTACH_FLAGS_RECV</constant></term>
-              <listitem>
-                <para>
-                  An optional item that contains a set of required attach flags
-                  that connections must allow. This item is used as a
-                  negotiation measure during connection creation. If connections
-                  do not satisfy the bus requirements, they are not allowed on
-                  the bus. If not set, the bus does not require any metadata to
-                  be attached; in this case connections are free to set their
-                  own attach flags.
-                </para>
-              </listitem>
-            </varlistentry>
-
-            <varlistentry>
               <term><constant>KDBUS_ITEM_ATTACH_FLAGS_SEND</constant></term>
               <listitem>
                 <para>
diff --git a/Documentation/kdbus/kdbus.connection.xml b/Documentation/kdbus/kdbus.connection.xml
index cefb419..4bb5f30 100644
--- a/Documentation/kdbus/kdbus.connection.xml
+++ b/Documentation/kdbus/kdbus.connection.xml
@@ -355,13 +355,7 @@ struct kdbus_cmd_hello {
           Set the bits for metadata this connection permits to be sent to the
           receiving peer. Only metadata items that are both allowed to be sent
           by the sender and that are requested by the receiver will be attached
-          to the message. Note, however, that the bus may optionally require
-          some of those bits to be set. If the match fails, the ioctl will fail
-          with <varname>errno</varname> set to
-          <constant>ECONNREFUSED</constant>. In either case, when returning the
-          field will be set to the mask of metadata items that are enforced by
-          the bus with the <constant>KDBUS_FLAGS_KERNEL</constant> bit set as
-          well.
+          to the message.
         </para></listitem>
       </varlistentry>
 
diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
index bbdf0f2..7d2c336 100644
--- a/ipc/kdbus/bus.c
+++ b/ipc/kdbus/bus.c
@@ -66,23 +66,16 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
 				       const char *name,
 				       struct kdbus_bloom_parameter *bloom,
 				       const u64 *pattach_owner,
-				       const u64 *pattach_recv,
 				       u64 flags, kuid_t uid, kgid_t gid)
 {
 	struct kdbus_bus *b;
 	u64 attach_owner;
-	u64 attach_recv;
 	int ret;
 
 	if (bloom->size < 8 || bloom->size > KDBUS_BUS_BLOOM_MAX_SIZE ||
 	    !KDBUS_IS_ALIGNED8(bloom->size) || bloom->n_hash < 1)
 		return ERR_PTR(-EINVAL);
 
-	ret = kdbus_sanitize_attach_flags(pattach_recv ? *pattach_recv : 0,
-					  &attach_recv);
-	if (ret < 0)
-		return ERR_PTR(ret);
-
 	ret = kdbus_sanitize_attach_flags(pattach_owner ? *pattach_owner : 0,
 					  &attach_owner);
 	if (ret < 0)
@@ -111,7 +104,6 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
 
 	b->id = atomic64_inc_return(&domain->last_id);
 	b->bus_flags = flags;
-	b->attach_flags_req = attach_recv;
 	b->attach_flags_owner = attach_owner;
 	generate_random_uuid(b->id128);
 	b->bloom = *bloom;
@@ -380,7 +372,6 @@ struct kdbus_bus *kdbus_cmd_bus_make(struct kdbus_domain *domain,
 		{ .type = KDBUS_ITEM_MAKE_NAME, .mandatory = true },
 		{ .type = KDBUS_ITEM_BLOOM_PARAMETER, .mandatory = true },
 		{ .type = KDBUS_ITEM_ATTACH_FLAGS_SEND },
-		{ .type = KDBUS_ITEM_ATTACH_FLAGS_RECV },
 	};
 	struct kdbus_args args = {
 		.allowed_flags = KDBUS_FLAG_NEGOTIATE |
@@ -399,7 +390,6 @@ struct kdbus_bus *kdbus_cmd_bus_make(struct kdbus_domain *domain,
 	bus = kdbus_bus_new(domain,
 			    argv[1].item->str, &argv[2].item->bloom_parameter,
 			    argv[3].item ? argv[3].item->data64 : NULL,
-			    argv[4].item ? argv[4].item->data64 : NULL,
 			    cmd->flags, current_euid(), current_egid());
 	if (IS_ERR(bus)) {
 		ret = PTR_ERR(bus);
diff --git a/ipc/kdbus/bus.h b/ipc/kdbus/bus.h
index 5bea5ef..e019ef3 100644
--- a/ipc/kdbus/bus.h
+++ b/ipc/kdbus/bus.h
@@ -37,7 +37,6 @@ struct kdbus_user;
  * @node:		kdbus_node
  * @id:			ID of this bus in the domain
  * @bus_flags:		Simple pass-through flags from userspace to userspace
- * @attach_flags_req:	KDBUS_ATTACH_* flags required by connecting peers
  * @attach_flags_owner:	KDBUS_ATTACH_* flags of bus creator that other
  *			connections can see or query
  * @id128:		Unique random 128 bit ID of this bus
@@ -60,7 +59,6 @@ struct kdbus_bus {
 	/* static */
 	u64 id;
 	u64 bus_flags;
-	u64 attach_flags_req;
 	u64 attach_flags_owner;
 	u8 id128[16];
 	struct kdbus_bloom_parameter bloom;
diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 9993753..e5e9c1e 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -112,10 +112,6 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 	if (ret < 0)
 		return ERR_PTR(ret);
 
-	/* The attach flags must always satisfy the bus requirements. */
-	if (bus->attach_flags_req & ~attach_flags_send)
-		return ERR_PTR(-ECONNREFUSED);
-
 	conn = kzalloc(sizeof(*conn), GFP_KERNEL);
 	if (!conn)
 		return ERR_PTR(-ENOMEM);
@@ -1835,7 +1831,6 @@ exit:
  */
 int kdbus_cmd_update(struct kdbus_conn *conn, void __user *argp)
 {
-	struct kdbus_bus *bus = conn->ep->bus;
 	struct kdbus_item *item_policy;
 	u64 *item_attach_send = NULL;
 	u64 *item_attach_recv = NULL;
@@ -1876,11 +1871,6 @@ int kdbus_cmd_update(struct kdbus_conn *conn, void __user *argp)
 						  &attach_send);
 		if (ret < 0)
 			goto exit;
-
-		if (bus->attach_flags_req & ~attach_send) {
-			ret = -EINVAL;
-			goto exit;
-		}
 	}
 
 	if (item_attach_recv) {
diff --git a/tools/testing/selftests/kdbus/kdbus-test.c b/tools/testing/selftests/kdbus/kdbus-test.c
index 294e82a..db732e5 100644
--- a/tools/testing/selftests/kdbus/kdbus-test.c
+++ b/tools/testing/selftests/kdbus/kdbus-test.c
@@ -299,7 +299,6 @@ static int test_prepare_env(const struct kdbus_test *t,
 
 		ret = kdbus_create_bus(env->control_fd,
 				       args->busname ?: n,
-				       _KDBUS_ATTACH_ALL,
 				       _KDBUS_ATTACH_ALL, &s);
 		free(n);
 		ASSERT_RETURN(ret == 0);
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index 29a0cb1..a5e54ca 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -114,8 +114,7 @@ int kdbus_sysfs_set_parameter_mask(const char *path, uint64_t mask)
 }
 
 int kdbus_create_bus(int control_fd, const char *name,
-		     uint64_t req_meta, uint64_t owner_meta,
-		     char **path)
+		     uint64_t owner_meta, char **path)
 {
 	struct {
 		struct kdbus_cmd cmd;
@@ -127,12 +126,12 @@ int kdbus_create_bus(int control_fd, const char *name,
 			struct kdbus_bloom_parameter bloom;
 		} bp;
 
-		/* required and owner metadata items */
+		/* owner metadata items */
 		struct {
 			uint64_t size;
 			uint64_t type;
 			uint64_t flags;
-		} attach[2];
+		} attach;
 
 		/* name item */
 		struct {
@@ -152,13 +151,9 @@ int kdbus_create_bus(int control_fd, const char *name,
 	snprintf(bus_make.name.str, sizeof(bus_make.name.str),
 		 "%u-%s", getuid(), name);
 
-	bus_make.attach[0].type = KDBUS_ITEM_ATTACH_FLAGS_RECV;
-	bus_make.attach[0].size = sizeof(bus_make.attach[0]);
-	bus_make.attach[0].flags = req_meta;
-
-	bus_make.attach[1].type = KDBUS_ITEM_ATTACH_FLAGS_SEND;
-	bus_make.attach[1].size = sizeof(bus_make.attach[0]);
-	bus_make.attach[1].flags = owner_meta;
+	bus_make.attach.type = KDBUS_ITEM_ATTACH_FLAGS_SEND;
+	bus_make.attach.size = sizeof(bus_make.attach);
+	bus_make.attach.flags = owner_meta;
 
 	bus_make.name.type = KDBUS_ITEM_MAKE_NAME;
 	bus_make.name.size = KDBUS_ITEM_HEADER_SIZE +
@@ -167,8 +162,7 @@ int kdbus_create_bus(int control_fd, const char *name,
 	bus_make.cmd.flags = KDBUS_MAKE_ACCESS_WORLD;
 	bus_make.cmd.size = sizeof(bus_make.cmd) +
 			     bus_make.bp.size +
-			     bus_make.attach[0].size +
-			     bus_make.attach[1].size +
+			     bus_make.attach.size +
 			     bus_make.name.size;
 
 	kdbus_printf("Creating bus with name >%s< on control fd %d ...\n",
diff --git a/tools/testing/selftests/kdbus/kdbus-util.h b/tools/testing/selftests/kdbus/kdbus-util.h
index d1a0f1b..e1e18b9 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.h
+++ b/tools/testing/selftests/kdbus/kdbus-util.h
@@ -168,8 +168,7 @@ int kdbus_free(const struct kdbus_conn *conn, uint64_t offset);
 int kdbus_msg_dump(const struct kdbus_conn *conn,
 		   const struct kdbus_msg *msg);
 int kdbus_create_bus(int control_fd, const char *name,
-		     uint64_t req_meta, uint64_t owner_meta,
-		     char **path);
+		     uint64_t owner_meta, char **path);
 int kdbus_msg_send(const struct kdbus_conn *conn, const char *name,
 		   uint64_t cookie, uint64_t flags, uint64_t timeout,
 		   int64_t priority, uint64_t dst_id);
diff --git a/tools/testing/selftests/kdbus/test-connection.c b/tools/testing/selftests/kdbus/test-connection.c
index e7c4866..4688ce8 100644
--- a/tools/testing/selftests/kdbus/test-connection.c
+++ b/tools/testing/selftests/kdbus/test-connection.c
@@ -70,15 +70,6 @@ int kdbus_test_hello(struct kdbus_test_env *env)
 
 	hello.pool_size = POOL_SIZE;
 
-	/*
-	 * The connection created by the core requires ALL meta flags
-	 * to be sent. An attempt to send less than that should result in
-	 * -ECONNREFUSED.
-	 */
-	hello.attach_flags_send = _KDBUS_ATTACH_ALL & ~KDBUS_ATTACH_TIMESTAMP;
-	ret = kdbus_cmd_hello(fd, &hello);
-	ASSERT_RETURN(ret == -ECONNREFUSED);
-
 	hello.attach_flags_send = _KDBUS_ATTACH_ALL;
 	hello.offset = (__u64)-1;
 
-- 
2.4.5

[PATCH 5/6] kdbus: pin namespaces on HELLO

[David Herrmann]

Whenever we send messages to a target connection, all we know about the
target is the 'struct file' associated with the kdbus connection. Hence,
we cannot know which namespaces a receiving process will be in when it
calls KDBUS_CMD_RECV on the message. So far, we pinned all metadata we
wanna send and translate it on RECV-time, since we then know the exact
namespaces to translate into.

This has several drawbacks:
 - Depending on the process calling RECV, the behavior is different (as
   multiple processes might be in different namespaces but share the same
   fd). This is unwanted behavior, as described by Eric here:
       http://www.spinics.net/lists/netdev/msg329322.html
 - We need to pin metadata with a message instead of translating it right
   away.
 - We cannot prep a message at SEND time as we don't know the size of the
   translated metadata. Hence, we need to do all that at RECV time.

This patch changes the namespace behavior. Instead of using the namespaces
at RECV time, we now pin the namespaces at HELLO (i.e., open()). So
regardless who calls RECV on this file-descriptor, the same namespaces
will be used.
This gives us the advantage that we now always know the target namespaces
for a message. Hence, we can now properly prep a message at SEND time and
never have to carry any metadata pins around.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 ipc/kdbus/bus.c        |  2 +-
 ipc/kdbus/connection.c |  8 +++++++-
 ipc/kdbus/connection.h |  6 ++++++
 ipc/kdbus/metadata.c   | 54 ++++++++++++++++++++++++++------------------------
 ipc/kdbus/metadata.h   |  1 +
 ipc/kdbus/queue.c      |  1 +
 6 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
index 7d2c336..8fffc2f 100644
--- a/ipc/kdbus/bus.c
+++ b/ipc/kdbus/bus.c
@@ -507,7 +507,7 @@ int kdbus_cmd_bus_creator_info(struct kdbus_conn *conn, void __user *argp)
 		goto exit;
 	}
 
-	ret = kdbus_meta_export(bus->creator_meta, NULL, attach_flags,
+	ret = kdbus_meta_export(bus->creator_meta, NULL, conn, attach_flags,
 				slice, hdr_size, &meta_size);
 	if (ret < 0)
 		goto exit;
diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index e5e9c1e..bf9a8a6 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -130,6 +130,9 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 	atomic_set(&conn->lost_count, 0);
 	INIT_DELAYED_WORK(&conn->work, kdbus_reply_list_scan_work);
 	conn->cred = get_current_cred();
+	conn->user_ns = get_user_ns(current_user_ns());
+	conn->pid_ns = get_pid_ns(task_active_pid_ns(current));
+	get_fs_root(current->fs, &conn->root_path);
 	init_waitqueue_head(&conn->wait);
 	kdbus_queue_init(&conn->queue);
 	conn->privileged = privileged;
@@ -271,6 +274,9 @@ static void __kdbus_conn_free(struct kref *kref)
 	kdbus_match_db_free(conn->match_db);
 	kdbus_pool_free(conn->pool);
 	kdbus_ep_unref(conn->ep);
+	path_put(&conn->root_path);
+	put_pid_ns(conn->pid_ns);
+	put_user_ns(conn->user_ns);
 	put_cred(conn->cred);
 	kfree(conn->description);
 	kfree(conn->quota);
@@ -1792,7 +1798,7 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp)
 		goto exit;
 	}
 
-	ret = kdbus_meta_export(owner_conn->meta, conn_meta, attach_flags,
+	ret = kdbus_meta_export(owner_conn->meta, conn_meta, conn, attach_flags,
 				slice, sizeof(info), &meta_size);
 	if (ret < 0)
 		goto exit;
diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
index d1ffe90..226f3ff 100644
--- a/ipc/kdbus/connection.h
+++ b/ipc/kdbus/connection.h
@@ -59,6 +59,9 @@ struct kdbus_kmsg;
  * @pool:		The user's buffer to receive messages
  * @user:		Owner of the connection
  * @cred:		The credentials of the connection at creation time
+ * @user_ns:		User namespace at creation time
+ * @pid_ns:		Pid namespace at creation time
+ * @root_path:		Root path at creation time
  * @name_count:		Number of owned well-known names
  * @request_count:	Number of pending requests issued by this
  *			connection that are waiting for replies from
@@ -97,6 +100,9 @@ struct kdbus_conn {
 	struct kdbus_pool *pool;
 	struct kdbus_user *user;
 	const struct cred *cred;
+	struct user_namespace *user_ns;
+	struct pid_namespace *pid_ns;
+	struct path root_path;
 	atomic_t name_count;
 	atomic_t request_count;
 	atomic_t lost_count;
diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
index c36b9cc..79f0e8c 100644
--- a/ipc/kdbus/metadata.c
+++ b/ipc/kdbus/metadata.c
@@ -888,7 +888,8 @@ static int kdbus_meta_push_kvec(struct kvec *kvec,
 }
 
 static void kdbus_meta_export_caps(struct kdbus_meta_caps *out,
-				   struct kdbus_meta_proc *mp)
+				   struct kdbus_meta_proc *mp,
+				   struct user_namespace *user_ns)
 {
 	struct user_namespace *iter;
 	const struct cred *cred = mp->cred;
@@ -896,18 +897,18 @@ static void kdbus_meta_export_caps(struct kdbus_meta_caps *out,
 	int i;
 
 	/*
-	 * This translates the effective capabilities of 'cred' into the current
-	 * user-namespace. If the current user-namespace is a child-namespace of
+	 * This translates the effective capabilities of 'cred' into the given
+	 * user-namespace. If the given user-namespace is a child-namespace of
 	 * the user-namespace of 'cred', the mask can be copied verbatim. If
 	 * not, the mask is cleared.
 	 * There's one exception: If 'cred' is the owner of any user-namespace
-	 * in the path between the current user-namespace and the user-namespace
+	 * in the path between the given user-namespace and the user-namespace
 	 * of 'cred', then it has all effective capabilities set. This means,
 	 * the user who created a user-namespace always has all effective
 	 * capabilities in any child namespaces. Note that this is based on the
 	 * uid of the namespace creator, not the task hierarchy.
 	 */
-	for (iter = current_user_ns(); iter; iter = iter->parent) {
+	for (iter = user_ns; iter; iter = iter->parent) {
 		if (iter == cred->user_ns) {
 			parent = true;
 			break;
@@ -951,23 +952,22 @@ static void kdbus_meta_export_caps(struct kdbus_meta_caps *out,
 }
 
 /* This is equivalent to from_kuid_munged(), but maps INVALID_UID to itself */
-static uid_t kdbus_from_kuid_keep(kuid_t uid)
+static uid_t kdbus_from_kuid_keep(struct user_namespace *ns, kuid_t uid)
 {
-	return uid_valid(uid) ?
-		from_kuid_munged(current_user_ns(), uid) : ((uid_t)-1);
+	return uid_valid(uid) ? from_kuid_munged(ns, uid) : ((uid_t)-1);
 }
 
 /* This is equivalent to from_kgid_munged(), but maps INVALID_GID to itself */
-static gid_t kdbus_from_kgid_keep(kgid_t gid)
+static gid_t kdbus_from_kgid_keep(struct user_namespace *ns, kgid_t gid)
 {
-	return gid_valid(gid) ?
-		from_kgid_munged(current_user_ns(), gid) : ((gid_t)-1);
+	return gid_valid(gid) ? from_kgid_munged(ns, gid) : ((gid_t)-1);
 }
 
 /**
  * kdbus_meta_export() - export information from metadata into a slice
  * @mp:		Process metadata, or NULL
  * @mc:		Connection metadata, or NULL
+ * @conn:	Target connection to translate metadata into
  * @mask:	Mask of KDBUS_ATTACH_* flags to export
  * @slice:	The slice to export to
  * @offset:	The offset inside @slice to write to
@@ -983,18 +983,19 @@ static gid_t kdbus_from_kgid_keep(kgid_t gid)
  * kdbus_meta_export_prepare(); depending on the namespaces in question, it
  * might use up less than that.
  *
- * All information will be translated using the current namespaces.
+ * All information will be translated using the namespaces of @conn.
  *
  * Return: 0 on success, negative error number otherwise.
  */
 int kdbus_meta_export(struct kdbus_meta_proc *mp,
 		      struct kdbus_meta_conn *mc,
+		      struct kdbus_conn *conn,
 		      u64 mask,
 		      struct kdbus_pool_slice *slice,
 		      off_t offset,
 		      size_t *real_size)
 {
-	struct user_namespace *user_ns = current_user_ns();
+	struct user_namespace *user_ns = conn->user_ns;
 	struct kdbus_item_header item_hdr[13], *hdr;
 	char *exe_pathname = NULL;
 	struct kdbus_creds creds;
@@ -1016,23 +1017,23 @@ int kdbus_meta_export(struct kdbus_meta_proc *mp,
 	/* process metadata */
 
 	if (mp && (mask & KDBUS_ATTACH_CREDS)) {
-		creds.uid	= kdbus_from_kuid_keep(mp->uid);
-		creds.euid	= kdbus_from_kuid_keep(mp->euid);
-		creds.suid	= kdbus_from_kuid_keep(mp->suid);
-		creds.fsuid	= kdbus_from_kuid_keep(mp->fsuid);
-		creds.gid	= kdbus_from_kgid_keep(mp->gid);
-		creds.egid	= kdbus_from_kgid_keep(mp->egid);
-		creds.sgid	= kdbus_from_kgid_keep(mp->sgid);
-		creds.fsgid	= kdbus_from_kgid_keep(mp->fsgid);
+		creds.uid	= kdbus_from_kuid_keep(user_ns, mp->uid);
+		creds.euid	= kdbus_from_kuid_keep(user_ns, mp->euid);
+		creds.suid	= kdbus_from_kuid_keep(user_ns, mp->suid);
+		creds.fsuid	= kdbus_from_kuid_keep(user_ns, mp->fsuid);
+		creds.gid	= kdbus_from_kgid_keep(user_ns, mp->gid);
+		creds.egid	= kdbus_from_kgid_keep(user_ns, mp->egid);
+		creds.sgid	= kdbus_from_kgid_keep(user_ns, mp->sgid);
+		creds.fsgid	= kdbus_from_kgid_keep(user_ns, mp->fsgid);
 
 		cnt += kdbus_meta_push_kvec(kvec + cnt, hdr++, KDBUS_ITEM_CREDS,
 					    &creds, sizeof(creds), &size);
 	}
 
 	if (mp && (mask & KDBUS_ATTACH_PIDS)) {
-		pids.pid = pid_vnr(mp->tgid);
-		pids.tid = pid_vnr(mp->pid);
-		pids.ppid = pid_vnr(mp->ppid);
+		pids.pid = pid_nr_ns(mp->tgid, conn->pid_ns);
+		pids.tid = pid_nr_ns(mp->pid, conn->pid_ns);
+		pids.ppid = pid_nr_ns(mp->ppid, conn->pid_ns);
 
 		cnt += kdbus_meta_push_kvec(kvec + cnt, hdr++, KDBUS_ITEM_PIDS,
 					    &pids, sizeof(pids), &size);
@@ -1078,7 +1079,8 @@ int kdbus_meta_export(struct kdbus_meta_proc *mp,
 		 */
 
 		get_fs_root(current->fs, &p);
-		if (path_equal(&p, &mp->root_path)) {
+		if (path_equal(&p, &mp->root_path) &&
+		    path_equal(&p, &conn->root_path)) {
 			exe_page = (void *)__get_free_page(GFP_TEMPORARY);
 			if (!exe_page) {
 				path_put(&p);
@@ -1116,7 +1118,7 @@ int kdbus_meta_export(struct kdbus_meta_proc *mp,
 	if (mp && (mask & KDBUS_ATTACH_CAPS)) {
 		struct kdbus_meta_caps caps = {};
 
-		kdbus_meta_export_caps(&caps, mp);
+		kdbus_meta_export_caps(&caps, mp, user_ns);
 		cnt += kdbus_meta_push_kvec(kvec + cnt, hdr++,
 					    KDBUS_ITEM_CAPS, &caps,
 					    sizeof(caps), &size);
diff --git a/ipc/kdbus/metadata.h b/ipc/kdbus/metadata.h
index 79b6ac3..2dbbb3d 100644
--- a/ipc/kdbus/metadata.h
+++ b/ipc/kdbus/metadata.h
@@ -46,6 +46,7 @@ int kdbus_meta_export_prepare(struct kdbus_meta_proc *mp,
 			      u64 *mask, size_t *sz);
 int kdbus_meta_export(struct kdbus_meta_proc *mp,
 		      struct kdbus_meta_conn *mc,
+		      struct kdbus_conn *conn,
 		      u64 mask,
 		      struct kdbus_pool_slice *slice,
 		      off_t offset, size_t *real_size);
diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
index 25bb3ad..6650b78 100644
--- a/ipc/kdbus/queue.c
+++ b/ipc/kdbus/queue.c
@@ -479,6 +479,7 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
 
 		ret = kdbus_meta_export(entry->proc_meta,
 					entry->conn_meta,
+					conn_dst,
 					entry->attach_flags,
 					entry->slice,
 					entry->meta_offset,
-- 
2.4.5

Re: [PATCH 5/6] kdbus: pin namespaces on HELLO

[Sergei Zviagintsev]

Hi,

On Thu, Jul 02, 2015 at 10:28:33AM +0200, David Herrmann wrote:
> Whenever we send messages to a target connection, all we know about the
> target is the 'struct file' associated with the kdbus connection. Hence,
> we cannot know which namespaces a receiving process will be in when it
> calls KDBUS_CMD_RECV on the message. So far, we pinned all metadata we
> wanna send and translate it on RECV-time, since we then know the exact
> namespaces to translate into.
> 
> This has several drawbacks:
>  - Depending on the process calling RECV, the behavior is different (as
>    multiple processes might be in different namespaces but share the same
>    fd). This is unwanted behavior, as described by Eric here:
>        http://www.spinics.net/lists/netdev/msg329322.html
>  - We need to pin metadata with a message instead of translating it right
>    away.
>  - We cannot prep a message at SEND time as we don't know the size of the
>    translated metadata. Hence, we need to do all that at RECV time.
> 
> This patch changes the namespace behavior. Instead of using the namespaces
> at RECV time, we now pin the namespaces at HELLO (i.e., open()). So
> regardless who calls RECV on this file-descriptor, the same namespaces
> will be used.
> This gives us the advantage that we now always know the target namespaces
> for a message. Hence, we can now properly prep a message at SEND time and
> never have to carry any metadata pins around.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  ipc/kdbus/bus.c        |  2 +-
>  ipc/kdbus/connection.c |  8 +++++++-
>  ipc/kdbus/connection.h |  6 ++++++
>  ipc/kdbus/metadata.c   | 54 ++++++++++++++++++++++++++------------------------
>  ipc/kdbus/metadata.h   |  1 +
>  ipc/kdbus/queue.c      |  1 +
>  6 files changed, 44 insertions(+), 28 deletions(-)
> 
> diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
> index 7d2c336..8fffc2f 100644
> --- a/ipc/kdbus/bus.c
> +++ b/ipc/kdbus/bus.c
> @@ -507,7 +507,7 @@ int kdbus_cmd_bus_creator_info(struct kdbus_conn *conn, void __user *argp)
>  		goto exit;
>  	}
>  
> -	ret = kdbus_meta_export(bus->creator_meta, NULL, attach_flags,
> +	ret = kdbus_meta_export(bus->creator_meta, NULL, conn, attach_flags,
>  				slice, hdr_size, &meta_size);
>  	if (ret < 0)
>  		goto exit;
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index e5e9c1e..bf9a8a6 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -130,6 +130,9 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>  	atomic_set(&conn->lost_count, 0);
>  	INIT_DELAYED_WORK(&conn->work, kdbus_reply_list_scan_work);
>  	conn->cred = get_current_cred();
> +	conn->user_ns = get_user_ns(current_user_ns());
> +	conn->pid_ns = get_pid_ns(task_active_pid_ns(current));
> +	get_fs_root(current->fs, &conn->root_path);
>  	init_waitqueue_head(&conn->wait);
>  	kdbus_queue_init(&conn->queue);
>  	conn->privileged = privileged;
> @@ -271,6 +274,9 @@ static void __kdbus_conn_free(struct kref *kref)
>  	kdbus_match_db_free(conn->match_db);
>  	kdbus_pool_free(conn->pool);
>  	kdbus_ep_unref(conn->ep);
> +	path_put(&conn->root_path);
> +	put_pid_ns(conn->pid_ns);
> +	put_user_ns(conn->user_ns);
>  	put_cred(conn->cred);
>  	kfree(conn->description);
>  	kfree(conn->quota);
> @@ -1792,7 +1798,7 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp)
>  		goto exit;
>  	}
>  
> -	ret = kdbus_meta_export(owner_conn->meta, conn_meta, attach_flags,
> +	ret = kdbus_meta_export(owner_conn->meta, conn_meta, conn, attach_flags,
>  				slice, sizeof(info), &meta_size);
>  	if (ret < 0)
>  		goto exit;
> diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
> index d1ffe90..226f3ff 100644
> --- a/ipc/kdbus/connection.h
> +++ b/ipc/kdbus/connection.h
> @@ -59,6 +59,9 @@ struct kdbus_kmsg;
>   * @pool:		The user's buffer to receive messages
>   * @user:		Owner of the connection
>   * @cred:		The credentials of the connection at creation time
> + * @user_ns:		User namespace at creation time
> + * @pid_ns:		Pid namespace at creation time

Pid -> PID ?

> + * @root_path:		Root path at creation time
>   * @name_count:		Number of owned well-known names
>   * @request_count:	Number of pending requests issued by this
>   *			connection that are waiting for replies from
> @@ -97,6 +100,9 @@ struct kdbus_conn {
>  	struct kdbus_pool *pool;
>  	struct kdbus_user *user;
>  	const struct cred *cred;
> +	struct user_namespace *user_ns;
> +	struct pid_namespace *pid_ns;
> +	struct path root_path;
>  	atomic_t name_count;
>  	atomic_t request_count;
>  	atomic_t lost_count;
> diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
> index c36b9cc..79f0e8c 100644
> --- a/ipc/kdbus/metadata.c
> +++ b/ipc/kdbus/metadata.c
> @@ -888,7 +888,8 @@ static int kdbus_meta_push_kvec(struct kvec *kvec,
>  }
>  
>  static void kdbus_meta_export_caps(struct kdbus_meta_caps *out,
> -				   struct kdbus_meta_proc *mp)
> +				   struct kdbus_meta_proc *mp,
> +				   struct user_namespace *user_ns)
>  {
>  	struct user_namespace *iter;
>  	const struct cred *cred = mp->cred;
> @@ -896,18 +897,18 @@ static void kdbus_meta_export_caps(struct kdbus_meta_caps *out,
>  	int i;
>  
>  	/*
> -	 * This translates the effective capabilities of 'cred' into the current
> -	 * user-namespace. If the current user-namespace is a child-namespace of
> +	 * This translates the effective capabilities of 'cred' into the given
> +	 * user-namespace. If the given user-namespace is a child-namespace of
>  	 * the user-namespace of 'cred', the mask can be copied verbatim. If
>  	 * not, the mask is cleared.
>  	 * There's one exception: If 'cred' is the owner of any user-namespace
> -	 * in the path between the current user-namespace and the user-namespace
> +	 * in the path between the given user-namespace and the user-namespace
>  	 * of 'cred', then it has all effective capabilities set. This means,
>  	 * the user who created a user-namespace always has all effective
>  	 * capabilities in any child namespaces. Note that this is based on the
>  	 * uid of the namespace creator, not the task hierarchy.
>  	 */
> -	for (iter = current_user_ns(); iter; iter = iter->parent) {
> +	for (iter = user_ns; iter; iter = iter->parent) {

Is check `iter' for being not NULL is needed here? I mean, `iter' takes
range from `user_ns' (which is cred->user_ns) to &init_user_ns.

>  		if (iter == cred->user_ns) {
>  			parent = true;
>  			break;
> @@ -951,23 +952,22 @@ static void kdbus_meta_export_caps(struct kdbus_meta_caps *out,
>  }
>  
>  /* This is equivalent to from_kuid_munged(), but maps INVALID_UID to itself */
> -static uid_t kdbus_from_kuid_keep(kuid_t uid)
> +static uid_t kdbus_from_kuid_keep(struct user_namespace *ns, kuid_t uid)
>  {
> -	return uid_valid(uid) ?
> -		from_kuid_munged(current_user_ns(), uid) : ((uid_t)-1);
> +	return uid_valid(uid) ? from_kuid_munged(ns, uid) : ((uid_t)-1);
>  }
>  
>  /* This is equivalent to from_kgid_munged(), but maps INVALID_GID to itself */
> -static gid_t kdbus_from_kgid_keep(kgid_t gid)
> +static gid_t kdbus_from_kgid_keep(struct user_namespace *ns, kgid_t gid)
>  {
> -	return gid_valid(gid) ?
> -		from_kgid_munged(current_user_ns(), gid) : ((gid_t)-1);
> +	return gid_valid(gid) ? from_kgid_munged(ns, gid) : ((gid_t)-1);
>  }
>  
>  /**
>   * kdbus_meta_export() - export information from metadata into a slice
>   * @mp:		Process metadata, or NULL
>   * @mc:		Connection metadata, or NULL
> + * @conn:	Target connection to translate metadata into
>   * @mask:	Mask of KDBUS_ATTACH_* flags to export
>   * @slice:	The slice to export to
>   * @offset:	The offset inside @slice to write to
> @@ -983,18 +983,19 @@ static gid_t kdbus_from_kgid_keep(kgid_t gid)
>   * kdbus_meta_export_prepare(); depending on the namespaces in question, it
>   * might use up less than that.
>   *
> - * All information will be translated using the current namespaces.
> + * All information will be translated using the namespaces of @conn.
>   *
>   * Return: 0 on success, negative error number otherwise.
>   */
>  int kdbus_meta_export(struct kdbus_meta_proc *mp,
>  		      struct kdbus_meta_conn *mc,
> +		      struct kdbus_conn *conn,
>  		      u64 mask,
>  		      struct kdbus_pool_slice *slice,
>  		      off_t offset,
>  		      size_t *real_size)
>  {
> -	struct user_namespace *user_ns = current_user_ns();
> +	struct user_namespace *user_ns = conn->user_ns;
>  	struct kdbus_item_header item_hdr[13], *hdr;
>  	char *exe_pathname = NULL;
>  	struct kdbus_creds creds;
> @@ -1016,23 +1017,23 @@ int kdbus_meta_export(struct kdbus_meta_proc *mp,
>  	/* process metadata */
>  
>  	if (mp && (mask & KDBUS_ATTACH_CREDS)) {
> -		creds.uid	= kdbus_from_kuid_keep(mp->uid);
> -		creds.euid	= kdbus_from_kuid_keep(mp->euid);
> -		creds.suid	= kdbus_from_kuid_keep(mp->suid);
> -		creds.fsuid	= kdbus_from_kuid_keep(mp->fsuid);
> -		creds.gid	= kdbus_from_kgid_keep(mp->gid);
> -		creds.egid	= kdbus_from_kgid_keep(mp->egid);
> -		creds.sgid	= kdbus_from_kgid_keep(mp->sgid);
> -		creds.fsgid	= kdbus_from_kgid_keep(mp->fsgid);
> +		creds.uid	= kdbus_from_kuid_keep(user_ns, mp->uid);
> +		creds.euid	= kdbus_from_kuid_keep(user_ns, mp->euid);
> +		creds.suid	= kdbus_from_kuid_keep(user_ns, mp->suid);
> +		creds.fsuid	= kdbus_from_kuid_keep(user_ns, mp->fsuid);
> +		creds.gid	= kdbus_from_kgid_keep(user_ns, mp->gid);
> +		creds.egid	= kdbus_from_kgid_keep(user_ns, mp->egid);
> +		creds.sgid	= kdbus_from_kgid_keep(user_ns, mp->sgid);
> +		creds.fsgid	= kdbus_from_kgid_keep(user_ns, mp->fsgid);
>  
>  		cnt += kdbus_meta_push_kvec(kvec + cnt, hdr++, KDBUS_ITEM_CREDS,
>  					    &creds, sizeof(creds), &size);
>  	}
>  
>  	if (mp && (mask & KDBUS_ATTACH_PIDS)) {
> -		pids.pid = pid_vnr(mp->tgid);
> -		pids.tid = pid_vnr(mp->pid);
> -		pids.ppid = pid_vnr(mp->ppid);
> +		pids.pid = pid_nr_ns(mp->tgid, conn->pid_ns);
> +		pids.tid = pid_nr_ns(mp->pid, conn->pid_ns);
> +		pids.ppid = pid_nr_ns(mp->ppid, conn->pid_ns);
>  
>  		cnt += kdbus_meta_push_kvec(kvec + cnt, hdr++, KDBUS_ITEM_PIDS,
>  					    &pids, sizeof(pids), &size);
> @@ -1078,7 +1079,8 @@ int kdbus_meta_export(struct kdbus_meta_proc *mp,
>  		 */
>  
>  		get_fs_root(current->fs, &p);
> -		if (path_equal(&p, &mp->root_path)) {
> +		if (path_equal(&p, &mp->root_path) &&
> +		    path_equal(&p, &conn->root_path)) {
>  			exe_page = (void *)__get_free_page(GFP_TEMPORARY);
>  			if (!exe_page) {
>  				path_put(&p);
> @@ -1116,7 +1118,7 @@ int kdbus_meta_export(struct kdbus_meta_proc *mp,
>  	if (mp && (mask & KDBUS_ATTACH_CAPS)) {
>  		struct kdbus_meta_caps caps = {};
>  
> -		kdbus_meta_export_caps(&caps, mp);
> +		kdbus_meta_export_caps(&caps, mp, user_ns);
>  		cnt += kdbus_meta_push_kvec(kvec + cnt, hdr++,
>  					    KDBUS_ITEM_CAPS, &caps,
>  					    sizeof(caps), &size);
> diff --git a/ipc/kdbus/metadata.h b/ipc/kdbus/metadata.h
> index 79b6ac3..2dbbb3d 100644
> --- a/ipc/kdbus/metadata.h
> +++ b/ipc/kdbus/metadata.h
> @@ -46,6 +46,7 @@ int kdbus_meta_export_prepare(struct kdbus_meta_proc *mp,
>  			      u64 *mask, size_t *sz);
>  int kdbus_meta_export(struct kdbus_meta_proc *mp,
>  		      struct kdbus_meta_conn *mc,
> +		      struct kdbus_conn *conn,
>  		      u64 mask,
>  		      struct kdbus_pool_slice *slice,
>  		      off_t offset, size_t *real_size);
> diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
> index 25bb3ad..6650b78 100644
> --- a/ipc/kdbus/queue.c
> +++ b/ipc/kdbus/queue.c
> @@ -479,6 +479,7 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
>  
>  		ret = kdbus_meta_export(entry->proc_meta,
>  					entry->conn_meta,
> +					conn_dst,
>  					entry->attach_flags,
>  					entry->slice,
>  					entry->meta_offset,
> -- 
> 2.4.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] kdbus: pin namespaces on HELLO

[David Herrmann]

Hi

On Fri, Jul 3, 2015 at 11:44 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> On Thu, Jul 02, 2015 at 10:28:33AM +0200, David Herrmann wrote:
>>       /*
>> -      * This translates the effective capabilities of 'cred' into the current
>> -      * user-namespace. If the current user-namespace is a child-namespace of
>> +      * This translates the effective capabilities of 'cred' into the given
>> +      * user-namespace. If the given user-namespace is a child-namespace of
>>        * the user-namespace of 'cred', the mask can be copied verbatim. If
>>        * not, the mask is cleared.
>>        * There's one exception: If 'cred' is the owner of any user-namespace
>> -      * in the path between the current user-namespace and the user-namespace
>> +      * in the path between the given user-namespace and the user-namespace
>>        * of 'cred', then it has all effective capabilities set. This means,
>>        * the user who created a user-namespace always has all effective
>>        * capabilities in any child namespaces. Note that this is based on the
>>        * uid of the namespace creator, not the task hierarchy.
>>        */
>> -     for (iter = current_user_ns(); iter; iter = iter->parent) {
>> +     for (iter = user_ns; iter; iter = iter->parent) {
>
> Is check `iter' for being not NULL is needed here? I mean, `iter' takes
> range from `user_ns' (which is cred->user_ns) to &init_user_ns.

This patch does not change the logic of this lookup, but only the
source of the user-namespace. Hence, a change of this loop-head would
be inappropriate.
Nevertheless, I prefer being rather safe than sorry here, so I don't
mind the current code.

Thanks
David

[PATCH 6/6] kdbus: fix NULL-deref in activator cleanup

[David Herrmann]

Right now, we always assume an activator has a valid name and
conn->activator_of is set. However, this assumption is not true if the
setup of the activator fails. In those cases, the ->flags field indicates
an activator, but the name might not have been claimed, yet.

Fix the destructor of connections to not assume all activators have
claimed names.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 ipc/kdbus/names.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index d77ee08..057f806 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -444,7 +444,7 @@ void kdbus_name_release_all(struct kdbus_name_registry *reg,
 
 	down_write(&reg->rwlock);
 
-	if (kdbus_conn_is_activator(conn)) {
+	if (conn->activator_of) {
 		activator = conn->activator_of->activator;
 		conn->activator_of->activator = NULL;
 	}
-- 
2.4.5

Re: [PATCH 0/6] Miscellaneous kdbus fixes and code removal

[David Herrmann]

Hi

On Thu, Jul 2, 2015 at 10:28 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi Greg
>
> A set of 6 patches to drop unused code and fix some bugs:
>
>  #1: simple fix for test-cases
>  #2: drop redundant internal accounting
>  #3: drop unused structure member
>  #4: drop support for an HELLO-ioctl item that was never used. If this breaks
>      some user-space we're not aware of, we will have to revert it. But I
>      haven't seen anything using this so far. This does *not* break ABI, but
>      only cause the ioctl to fail if you pass that item.
>  #5: pin namespaces on kdbus-connections to simplify namespace handling, further
>      described in the commit-msg
>  #6: fix a NULL-deref

All are applied now.

David