[PATCH v3 1/2] nsfs: Add an ioctl() to return the namespace type

[PATCH v3 1/2] nsfs: Add an ioctl() to return the namespace type

[Michael Kerrisk (man-pages)]

Linux 4.9 added two ioctl() operations that can be used to discover:

* the parental relationships for hierarchical namespaces (user and PID)
  [NS_GET_PARENT]
* the user namespaces that owns a specified non-user-namespace
  [NS_GET_USERNS]

For no good reason that I can glean, NS_GET_USERNS was made synonymous
with NS_GET_PARENT for user namespaces. It might have been better if
NS_GET_USERNS had returned an error if the supplied file descriptor
referred to a user namespace, since it suggests that the caller may be
confused. More particularly, if it had generated an error, then I wouldn't
need the new ioctl() operation proposed here. (On the other hand, what
I propose here may be more generally useful.)

I would like to write code that discovers namespace relationships for
the purpose of understanding the namespace setup on a running system.
In particular, given a file descriptor (or pathname) for a namespace,
N, I'd like to obtain the corresponding user namespace.  Namespace N
might be a user namespace (in which case my code would just use N) or
a non-user namespace (in which case my code will use NS_GET_USERNS to
get the user namespace associated with N). The problem is that there
is no way to tell the difference by looking at the file descriptor
(and if I try to use NS_GET_USERNS on an N that is a user namespace, I
get the parent user namespace of N, which is not what I want).

This patch therefore adds a new ioctl(), NS_GET_NSTYPE, which, given
a file descriptor that refers to a user namespace, returns the
namespace type (one of the CLONE_NEW* constants).

Signed-off-by: Michael Kerrisk <mtk-manpages@gmail.com>
---
 fs/nsfs.c                 | 2 ++
 include/uapi/linux/nsfs.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index 8c9fb29..5d53476 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -172,6 +172,8 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
 		if (!ns->ops->get_parent)
 			return -EINVAL;
 		return open_related_ns(ns, ns->ops->get_parent);
+	case NS_GET_NSTYPE:
+		return ns->ops->type;
 	default:
 		return -ENOTTY;
 	}
diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h
index 3af6172..2b48df1 100644
--- a/include/uapi/linux/nsfs.h
+++ b/include/uapi/linux/nsfs.h
@@ -9,5 +9,8 @@
 #define NS_GET_USERNS	_IO(NSIO, 0x1)
 /* Returns a file descriptor that refers to a parent namespace */
 #define NS_GET_PARENT	_IO(NSIO, 0x2)
+/* Returns the type of namespace (CLONE_NEW* value) referred to by
+   file descriptor */
+#define NS_GET_NSTYPE	_IO(NSIO, 0x3)
 
 #endif /* __LINUX_NSFS_H */
-- 
2.5.5

[PATCH v3 2/2] nsfs: Add an ioctl() to return owner UID of a userns

[Michael Kerrisk (man-pages)]

I'd like to write code that discovers the user namespace hierarchy on
a running system, and also shows who owns the various user namespaces.
Currently, there is no way of getting the owner UID of a user
namespace. Therefore, this patch adds an NS_GET_CREATOR_UID ioctl()
that fetches the (munged) UID of the creator of the user namespace
referred to by the specified file descriptor.

If the supplied file descriptor does not refer to a user namespace,
the operation fails with the error EINVAL.

Acked-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Michael Kerrisk <mtk-manpages@gmail.com>

---
Open questions:

Should the type for the ioctl() argument be changed? I mean,
make the following changes to the patch below:

-       unsigned int __user *argp;
+       uid_t __user *argp;

And further below, change:

-             argp = (unsigned int __user *) arg;
+             argp = (uid_t __user *) arg;

?

V3 changes:
* Fixed data type of local variable 'uid'; thanks to Andrei Vagin

V2 changes:
* Renamed ioctl() from NS_CREATOR_UID to NS_OWNER_UID, at the
  suggestion of Eric Biederman.
* Make ioctl() return UID via buffer pointed to by argp. (Returning
  the UID via the result value could lead to problems since a large
  unsigned int UID might be misinterpreted as an error.) Thanks to
  Andrei Vagin for pointing this out.
---
 fs/nsfs.c                 | 11 +++++++++++
 include/uapi/linux/nsfs.h |  8 +++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index 5d53476..63a4ad4 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -7,6 +7,7 @@
 #include <linux/seq_file.h>
 #include <linux/user_namespace.h>
 #include <linux/nsfs.h>
+#include <linux/uaccess.h>
 
 static struct vfsmount *nsfs_mnt;
 
@@ -163,7 +164,10 @@ int open_related_ns(struct ns_common *ns,
 static long ns_ioctl(struct file *filp, unsigned int ioctl,
 			unsigned long arg)
 {
+	struct user_namespace *user_ns;
 	struct ns_common *ns = get_proc_ns(file_inode(filp));
+	unsigned int __user *argp;
+	uid_t uid;
 
 	switch (ioctl) {
 	case NS_GET_USERNS:
@@ -174,6 +178,13 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
 		return open_related_ns(ns, ns->ops->get_parent);
 	case NS_GET_NSTYPE:
 		return ns->ops->type;
+	case NS_GET_OWNER_UID:
+		if (ns->ops->type != CLONE_NEWUSER)
+			return -EINVAL;
+		user_ns = container_of(ns, struct user_namespace, ns);
+		argp = (unsigned int __user *) arg;
+		uid = from_kuid_munged(current_user_ns(), user_ns->owner);
+		return put_user(uid, argp);
 	default:
 		return -ENOTTY;
 	}
diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h
index 2b48df1..c4a925e 100644
--- a/include/uapi/linux/nsfs.h
+++ b/include/uapi/linux/nsfs.h
@@ -6,11 +6,13 @@
 #define NSIO	0xb7
 
 /* Returns a file descriptor that refers to an owning user namespace */
-#define NS_GET_USERNS	_IO(NSIO, 0x1)
+#define NS_GET_USERNS		_IO(NSIO, 0x1)
 /* Returns a file descriptor that refers to a parent namespace */
-#define NS_GET_PARENT	_IO(NSIO, 0x2)
+#define NS_GET_PARENT		_IO(NSIO, 0x2)
 /* Returns the type of namespace (CLONE_NEW* value) referred to by
    file descriptor */
-#define NS_GET_NSTYPE	_IO(NSIO, 0x3)
+#define NS_GET_NSTYPE		_IO(NSIO, 0x3)
+/* Get owner UID for a user namespace */
+#define NS_GET_OWNER_UID	_IO(NSIO, 0x4)
 
 #endif /* __LINUX_NSFS_H */
-- 
2.5.5

Re: [PATCH v3 2/2] nsfs: Add an ioctl() to return owner UID of a userns

[W. Trevor King]

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

On Tue, Jan 17, 2017 at 02:03:29PM +1300, Michael Kerrisk (man-pages) wrote:
> +	case NS_GET_OWNER_UID:
> +		if (ns->ops->type != CLONE_NEWUSER)
> +			return -EINVAL;
> +		user_ns = container_of(ns, struct user_namespace, ns);
> +		argp = (unsigned int __user *) arg;
> +		uid = from_kuid_munged(current_user_ns(), user_ns->owner);
> +		return put_user(uid, argp);
> …
> +/* Get owner UID for a user namespace */
> +#define NS_GET_OWNER_UID	_IO(NSIO, 0x4)

The comment here should probably be:

  Get owner UID (in the current user namespace) for a user namespace

or some such, to convey that current_user_ns is being passed to
from_kuid_munged.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 2/2] nsfs: Add an ioctl() to return owner UID of a userns

[Michael Kerrisk (man-pages)]

On 17 January 2017 at 14:19, W. Trevor King <wking@tremily.us> wrote:
> On Tue, Jan 17, 2017 at 02:03:29PM +1300, Michael Kerrisk (man-pages) wrote:
>> +     case NS_GET_OWNER_UID:
>> +             if (ns->ops->type != CLONE_NEWUSER)
>> +                     return -EINVAL;
>> +             user_ns = container_of(ns, struct user_namespace, ns);
>> +             argp = (unsigned int __user *) arg;
>> +             uid = from_kuid_munged(current_user_ns(), user_ns->owner);
>> +             return put_user(uid, argp);
>> …
>> +/* Get owner UID for a user namespace */
>> +#define NS_GET_OWNER_UID     _IO(NSIO, 0x4)
>
> The comment here should probably be:
>
>   Get owner UID (in the current user namespace) for a user namespace
>
> or some such, to convey that current_user_ns is being passed to
> from_kuid_munged.

Thanks, Trevor. I'll fix that. From a user-space perspective though
"in the current user namespace" should be "in the caller's user
namespace".

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v3 2/2] nsfs: Add an ioctl() to return owner UID of a userns

[Eric W. Biederman]

"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> writes:

> I'd like to write code that discovers the user namespace hierarchy on
> a running system, and also shows who owns the various user namespaces.
> Currently, there is no way of getting the owner UID of a user
> namespace. Therefore, this patch adds an NS_GET_CREATOR_UID ioctl()
> that fetches the (munged) UID of the creator of the user namespace
> referred to by the specified file descriptor.
>
> If the supplied file descriptor does not refer to a user namespace,
> the operation fails with the error EINVAL.
>
> Acked-by: Andrey Vagin <avagin@openvz.org>
> Signed-off-by: Michael Kerrisk <mtk-manpages@gmail.com>
>
> ---
> Open questions:
>
> Should the type for the ioctl() argument be changed? I mean,
> make the following changes to the patch below:
>
> -       unsigned int __user *argp;
> +       uid_t __user *argp;
>
> And further below, change:
>
> -             argp = (unsigned int __user *) arg;
> +             argp = (uid_t __user *) arg;
>
> ?
>
> V3 changes:
> * Fixed data type of local variable 'uid'; thanks to Andrei Vagin
>
> V2 changes:
> * Renamed ioctl() from NS_CREATOR_UID to NS_OWNER_UID, at the
>   suggestion of Eric Biederman.
> * Make ioctl() return UID via buffer pointed to by argp. (Returning
>   the UID via the result value could lead to problems since a large
>   unsigned int UID might be misinterpreted as an error.) Thanks to
>   Andrei Vagin for pointing this out.
> ---
>  fs/nsfs.c                 | 11 +++++++++++
>  include/uapi/linux/nsfs.h |  8 +++++---
>  2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nsfs.c b/fs/nsfs.c
> index 5d53476..63a4ad4 100644
> --- a/fs/nsfs.c
> +++ b/fs/nsfs.c
> @@ -7,6 +7,7 @@
>  #include <linux/seq_file.h>
>  #include <linux/user_namespace.h>
>  #include <linux/nsfs.h>
> +#include <linux/uaccess.h>
>  
>  static struct vfsmount *nsfs_mnt;
>  
> @@ -163,7 +164,10 @@ int open_related_ns(struct ns_common *ns,
>  static long ns_ioctl(struct file *filp, unsigned int ioctl,
>  			unsigned long arg)
>  {
> +	struct user_namespace *user_ns;
>  	struct ns_common *ns = get_proc_ns(file_inode(filp));
> +	unsigned int __user *argp;
> +	uid_t uid;
>  
>  	switch (ioctl) {
>  	case NS_GET_USERNS:
> @@ -174,6 +178,13 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
>  		return open_related_ns(ns, ns->ops->get_parent);
>  	case NS_GET_NSTYPE:
>  		return ns->ops->type;
> +	case NS_GET_OWNER_UID:
> +		if (ns->ops->type != CLONE_NEWUSER)
> +			return -EINVAL;
> +		user_ns = container_of(ns, struct user_namespace, ns);
> +		argp = (unsigned int __user *) arg;
> +		uid = from_kuid_munged(current_user_ns(), user_ns->owner);
> +		return put_user(uid, argp);

There is a question of how do we want to handle a uid that does not map.
For most interfaces that return uids the uid is one small part of
it so it is better if return the unmapped uid value.

Does this applie to NS_GET_OWNER_UID or do we perhaps want to do:

		user_ns = container_of(ns, struct user_namespace, ns);
                argp = (unsigned int __user *) arg;
                uid = from_kuid(current_user_ns(), user_ns->owner);
                if (uid == (uid_t)-1)
                	return -EOVERFLOW;
                return put_user(uid, argp);

Which of these variants would make handling this error easiest in your
introspection code?

I suspect return -EOVERFLOW would be easiest to handle.

Eric

                
>  	default:
>  		return -ENOTTY;
>  	}
> diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h
> index 2b48df1..c4a925e 100644
> --- a/include/uapi/linux/nsfs.h
> +++ b/include/uapi/linux/nsfs.h
> @@ -6,11 +6,13 @@
>  #define NSIO	0xb7
>  
>  /* Returns a file descriptor that refers to an owning user namespace */
> -#define NS_GET_USERNS	_IO(NSIO, 0x1)
> +#define NS_GET_USERNS		_IO(NSIO, 0x1)
>  /* Returns a file descriptor that refers to a parent namespace */
> -#define NS_GET_PARENT	_IO(NSIO, 0x2)
> +#define NS_GET_PARENT		_IO(NSIO, 0x2)
>  /* Returns the type of namespace (CLONE_NEW* value) referred to by
>     file descriptor */
> -#define NS_GET_NSTYPE	_IO(NSIO, 0x3)
> +#define NS_GET_NSTYPE		_IO(NSIO, 0x3)
> +/* Get owner UID for a user namespace */
> +#define NS_GET_OWNER_UID	_IO(NSIO, 0x4)
>  
>  #endif /* __LINUX_NSFS_H */