| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <m1d3b0gg3r.fsf@fess.ebiederm.org>
Date: Tue, 03 Jan 2012 22:02:32 -0800
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Cyrill Gorcunov <gorcunov@...nvz.org>
Cc: linux-kernel@...r.kernel.org,
Pavel Emelyanov <xemul@...allels.com>,
Glauber Costa <glommer@...allels.com>,
Andi Kleen <andi@...stfloor.org>, Tejun Heo <tj@...nel.org>,
Matt Helsley <matthltc@...ibm.com>,
Pekka Enberg <penberg@...nel.org>,
Eric Dumazet <eric.dumazet@...il.com>,
Vasiliy Kulikov <segoon@...nwall.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Alexey Dobriyan <adobriyan@...il.com>
Subject: Re: [patch 2/4] proc: Show namespaces IDs in /proc/pid/ns/* files
Cyrill Gorcunov <gorcunov@...nvz.org> writes:
> This patch adds proc_ns_read method which provides
> IDs for /proc/pid/ns/* files.
Nacked-by: "Eric W. Biederman" <ebiederm@...ssion.com>
This is a poorly thought out user interface. If we are going to return
this kind of information and I believe we should we should, we should
return the id in the inode field with stat.
Comparing device+inode for equality is the traditional way to see if
two objects are the same in unix and there is no reason to make up
a new interface to get this functionality.
Furthermore we should always return the information, as it is valuable
even outside of the checkpoint/restart context.
I am also concerned that you appear to be building an interface
for use by checkpoint/restart that makes it impossible
checkpoint/restart the programs using that interface. The reason
is that you appear to be putting this nebulous id into a global
namespace and as such even if we wanted to I don't see how we could
build a version where we could restore the id during a restart. And
the thing is if you start building interfaces with identifiers you can
not possibly restore I expect you will find you have painted yourself
into a corner.
Using inode from stat avoids painting yourself into a corner because
you have the possibility of different mounts with different device
numbers having different inode numbers.
For the short term I don't see value in being able to restore the
object identifiers, but I do see a lot of value in allowing for a future
where nested checkpoint/restart is an option.
Eric
> Based-on-patch-from: Pavel Emelyanov <xemul@...allels.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov@...nvz.org>
> CC: Glauber Costa <glommer@...allels.com>
> CC: Andi Kleen <andi@...stfloor.org>
> CC: Tejun Heo <tj@...nel.org>
> CC: Matt Helsley <matthltc@...ibm.com>
> CC: Pekka Enberg <penberg@...nel.org>
> CC: Eric Dumazet <eric.dumazet@...il.com>
> CC: Vasiliy Kulikov <segoon@...nwall.com>
> CC: Andrew Morton <akpm@...ux-foundation.org>
> CC: Alexey Dobriyan <adobriyan@...il.com>
> ---
> Documentation/filesystems/proc.txt | 24 ++++++++++++++++++++++++
> fs/proc/namespaces.c | 22 ++++++++++++++++++++++
> include/linux/gen_obj_id.h | 1 +
> 3 files changed, 47 insertions(+)
>
> Index: linux-2.6.git/Documentation/filesystems/proc.txt
> ===================================================================
> --- linux-2.6.git.orig/Documentation/filesystems/proc.txt
> +++ linux-2.6.git/Documentation/filesystems/proc.txt
> @@ -40,6 +40,7 @@ Table of Contents
> 3.4 /proc/<pid>/coredump_filter - Core dump filtering settings
> 3.5 /proc/<pid>/mountinfo - Information about mounts
> 3.6 /proc/<pid>/comm & /proc/<pid>/task/<tid>/comm
> + 3.7 /proc/<pid>/ns - Information about namespaces
>
>
> ------------------------------------------------------------------------------
> @@ -1545,3 +1546,26 @@ a task to set its own or one of its thre
> is limited in size compared to the cmdline value, so writing anything longer
> then the kernel's TASK_COMM_LEN (currently 16 chars) will result in a truncated
> comm value.
> +
> +3.7 /proc/<pid>/ns - Information about namespaces
> +-----------------------------------------------------
> +
> +This directory consists of the following files "net", "uts", "ipc",
> +and depend if appropriate CONFIG_ entry is set, i.e. it's possible
> +to have only one, two or all three files here.
> +
> +Currently file contents provides that named "object id" number, which
> +is a number useful for the one purpose only -- to test if two differen
> +<pid> share the namespace.
> +
> +A typical format is
> +
> +id: 445332486300860161
> +
> +i.e. "id" followed by a number. One should never assume the number
> +means something, it is only useful for "sameness" test with another number
> +obtained from another <pid>.
> +
> +Moreover, a safe approach is to remember it as a string, since format may
> +change in future and id would be not a long integer value, but something
> +else, say SHA1/2 or even uuid encoded stream.
> Index: linux-2.6.git/fs/proc/namespaces.c
> ===================================================================
> --- linux-2.6.git.orig/fs/proc/namespaces.c
> +++ linux-2.6.git/fs/proc/namespaces.c
> @@ -12,6 +12,7 @@
> #include <linux/mnt_namespace.h>
> #include <linux/ipc_namespace.h>
> #include <linux/pid_namespace.h>
> +#include <linux/gen_obj_id.h>
> #include "internal.h"
>
>
> @@ -27,10 +28,31 @@ static const struct proc_ns_operations *
> #endif
> };
>
> +#ifdef CONFIG_GENERIC_OBJECT_ID
> +static ssize_t proc_ns_read(struct file *file, char __user *buf,
> + size_t len, loff_t *ppos)
> +{
> + struct proc_inode *ei = PROC_I(file->f_dentry->d_inode);
> + char tmp[32];
> +
> + snprintf(tmp, sizeof(tmp), "id:\t%lu\n",
> + gen_obj_id(ei->ns, GEN_OBJ_ID_NS));
> + return simple_read_from_buffer(buf, len, ppos, tmp, strlen(tmp));
> +}
> +
> +static const struct file_operations ns_file_operations = {
> + .llseek = no_llseek,
> + .read = proc_ns_read,
> +};
> +
> +#else
> +
> static const struct file_operations ns_file_operations = {
> .llseek = no_llseek,
> };
>
> +#endif /* CONFIG_GENERIC_OBJECT_ID */
> +
> static struct dentry *proc_ns_instantiate(struct inode *dir,
> struct dentry *dentry, struct task_struct *task, const void *ptr)
> {
> Index: linux-2.6.git/include/linux/gen_obj_id.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/gen_obj_id.h
> +++ linux-2.6.git/include/linux/gen_obj_id.h
> @@ -4,6 +4,7 @@
> #ifdef __KERNEL__
>
> enum {
> + GEN_OBJ_ID_NS,
> GEN_OBJ_ID_TYPES,
> };
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists