Re: [RFC PATCH 02/27] containers: Implement containers as kernel objects

2 messages, 2 authors, 2019-02-19 · open the first message on its own page

Re: [RFC PATCH 02/27] containers: Implement containers as kernel objects

From: Eric W. Biederman <hidden>
Date: 2019-02-19 16:56:19

David Howells [off-list ref] writes:

The container id details are ludicrous and will break practically
every use case.  This completely unacceptable.

Nacked-by: "Eric W. Biederman" [off-list ref]
quoted hunk
diff --git a/include/linux/container.h b/include/linux/container.h
new file mode 100644
index 000000000000..0a8918435097
--- /dev/null
+++ b/include/linux/container.h
+/*
+ * The container object.
+ */
+struct container {
+	u64			id;		/* Container ID */
...

No.  This is absolutely unacceptable.
As this breaks breaks nested containers and process migration.
quoted hunk
+};
+
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d2f90fa92468..073a3a930514 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -36,6 +36,7 @@ struct backing_dev_info;
 struct bio_list;
 struct blk_plug;
 struct cfs_rq;
+struct container;
 struct fs_struct;
 struct futex_pi_state;
 struct io_context;
@@ -870,6 +871,8 @@ struct task_struct {
 
 	/* Namespaces: */
 	struct nsproxy			*nsproxy;
+	struct container		*container;
+	struct list_head		container_link;
Why?  nsproxy would be a much cheaper location to put this.
Less space and less foobar.
quoted hunk
 	/* Signal handlers: */
 	struct signal_struct		*signal;
diff --git a/kernel/container.c b/kernel/container.c
new file mode 100644
index 000000000000..ca4012632cfa
--- /dev/null
+++ b/kernel/container.c
@@ -0,0 +1,348 @@
[...]
+
+	c->id = atomic64_inc_return(&container_id_counter);
This id is not in a namespace, and it doesn't have enough bits
of entropy to be globally unique.   Not that 64bit is enough
to have a chance at being globablly unique.


Eric

Re: [RFC PATCH 02/27] containers: Implement containers as kernel objects

From: David Howells <dhowells@redhat.com>
Date: 2019-02-19 23:13:28

Eric W. Biederman [off-list ref] wrote:
quoted
+	c->id = atomic64_inc_return(&container_id_counter);
This id is not in a namespace, and it doesn't have enough bits
of entropy to be globally unique.   Not that 64bit is enough
to have a chance at being globablly unique.
It's in a container, so it doesn't need to be in a namespace.  The intended
purpose is for annotating audit messages.  Globally unique wasn't particularly
in mind.  It could be turned into, say, a uuid, so that isn't really a problem
at this point.

You are right, though, it really should be globally unique as best possible -
even the one in init_container should be.  Ideally, it would look the same
inside the root container as any subcontainer.

David
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help