Patchwork [REVIEW,09/15] userns: Convert process event connector to handle kuids and kgids

login
register
mail settings
Submitter Eric W. Biederman
Date Aug. 26, 2012, 12:02 a.m.
Message ID <877gsmfrkc.fsf@xmission.com>
Download mbox | patch
Permalink /patch/180002/
State RFC
Delegated to: David Miller
Headers show

Comments

Eric W. Biederman - Aug. 26, 2012, 12:02 a.m.
- Only allow asking for events from the initial user and pid namespace,
  where we generate the events in.

- Convert kuids and kgids into the initial user namespace to report
  them via the process event connector.

Cc: Evgeniy Polyakov <zbr@ioremap.net>
Cc: David Miller <davem@davemloft.net>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 drivers/connector/cn_proc.c |   18 ++++++++++++++----
 init/Kconfig                |    1 -
 2 files changed, 14 insertions(+), 5 deletions(-)
Evgeniy Polyakov - Aug. 26, 2012, 12:33 p.m.
On Sat, Aug 25, 2012 at 05:02:59PM -0700, Eric W. Biederman (ebiederm@xmission.com) wrote:
> 
> - Only allow asking for events from the initial user and pid namespace,
>   where we generate the events in.
> 
> - Convert kuids and kgids into the initial user namespace to report
>   them via the process event connector.


Looks good, if IDs are really supposed to be sent only from root
namespace. And you dropped PROC_EVENTS from init/Kconfig, but since it
was no, it should be ok by default.

Feel free to add my acked-by: Evgeniy Polyakov <zbr@ioremap.net>
Although I thoughs it could be more interesting to generate events
including namespace id
Eric W. Biederman - Aug. 26, 2012, 1:43 p.m.
Evgeniy Polyakov <zbr@ioremap.net> writes:

> On Sat, Aug 25, 2012 at 05:02:59PM -0700, Eric W. Biederman (ebiederm@xmission.com) wrote:
>> 
>> - Only allow asking for events from the initial user and pid namespace,
>>   where we generate the events in.
>> 
>> - Convert kuids and kgids into the initial user namespace to report
>>   them via the process event connector.
>
>
> Looks good, if IDs are really supposed to be sent only from root
> namespace.

It is more about where events are recevied rather than where events are
sent from.

With this change events continue to be sent from all processes but can
only be received by processes in the initial user and initial pid
namespace.

To processes in other namespaces the uids and pids that the proc
connector code puts into it's messages are just non-sense, so I
cause the registration of connector clients in other namespaces to fail.

All of that I can do easily without a complete reworking of the current
connector code.  Which is enough for my current goal of building able
to build a kernel with all features enabled and only loosing features
if you are in somethin other than the initial namespaces.


> And you dropped PROC_EVENTS from init/Kconfig, but since it
> was no, it should be ok by default.

The removal of PROC_EVENTS from init/Kconfig was removing of the compile
guard that kept PROC_EVENTS and USER_NS from being selected at the same
time.  The curent user namespace code simply disables code that fails
to build when user namespace support is enabled.

> Feel free to add my acked-by: Evgeniy Polyakov <zbr@ioremap.net>
> Although I thoughs it could be more interesting to generate events
> including namespace id

There isn't a namespace id.  It is perfectly possible to generate events
that can be received by processes in multiple pid and user nameapces
just by calling pid_nr_ns() and from_kuid() with the right parameters.
The challenge is figuring out what namespaces have listening netlink
sockets we should send messages to.

Adding support for recepetion of connector proc events by listeners in
other namespaces looks like it would require near complete rewrite of
the connector code:
- Adding support for connector netlink sockets in other than the initial
  network namespace.
- Tracking which sockets were opened in which pid and which user
  namespaces.
- Filtering messages based on which namespaces the clients are listening
  in.
- Generating the different messages for the different sets of clients.

When someone cares enough I am certain the code will get written.
Having a clear unambigous that configuration does not work is
the first step in getting there.

I presume that whatever listens to connector based proc events listens
for the appropriate ack when registering and the daemon will fail to
start up.

Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 3e92b7d..fce2000 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -30,6 +30,7 @@ 
 #include <linux/gfp.h>
 #include <linux/ptrace.h>
 #include <linux/atomic.h>
+#include <linux/pid_namespace.h>
 
 #include <asm/unaligned.h>
 
@@ -127,11 +128,11 @@  void proc_id_connector(struct task_struct *task, int which_id)
 	rcu_read_lock();
 	cred = __task_cred(task);
 	if (which_id == PROC_EVENT_UID) {
-		ev->event_data.id.r.ruid = cred->uid;
-		ev->event_data.id.e.euid = cred->euid;
+		ev->event_data.id.r.ruid = from_kuid_munged(&init_user_ns, cred->uid);
+		ev->event_data.id.e.euid = from_kuid_munged(&init_user_ns, cred->euid);
 	} else if (which_id == PROC_EVENT_GID) {
-		ev->event_data.id.r.rgid = cred->gid;
-		ev->event_data.id.e.egid = cred->egid;
+		ev->event_data.id.r.rgid = from_kgid_munged(&init_user_ns, cred->gid);
+		ev->event_data.id.e.egid = from_kgid_munged(&init_user_ns, cred->egid);
 	} else {
 		rcu_read_unlock();
 		return;
@@ -303,6 +304,15 @@  static void cn_proc_mcast_ctl(struct cn_msg *msg,
 	if (msg->len != sizeof(*mc_op))
 		return;
 
+	/* 
+	 * Events are reported with respect to the initial pid
+	 * and user namespaces so ignore requestors from
+	 * other namespaces.
+	 */
+	if ((current_user_ns() != &init_user_ns) ||
+	    (task_active_pid_ns(current) != &init_pid_ns))
+		return;
+
 	mc_op = (enum proc_cn_mcast_op *)msg->data;
 	switch (*mc_op) {
 	case PROC_CN_MCAST_LISTEN:
diff --git a/init/Kconfig b/init/Kconfig
index 6c9d004..7327869 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -932,7 +932,6 @@  config UIDGID_CONVERTED
 	depends on QUOTA = n
 	depends on QUOTACTL = n
 	depends on DRM = n
-	depends on PROC_EVENTS = n
 
 	# Networking
 	depends on NET_9P = n