: Enable multiple mounts of devpts

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>
Subject: [RFC][PATCH 1/8]: /dev/tty tweak in init_dev()

When opening /dev/tty, __tty_open() finds the tty using get_current_tty(). When __tty_open() calls init_dev() it passes in this tty in '*ret_tty'. init_dev() ignores this and asks devpts again for the tty. Is that really necessary ?

This patch has the kernel internally create the [ptmx, c, 5:2] device when mounting devpts filesystem. The permissions for the device node can be specified by the '-o ptmx_mode=0666' option. The default mode is 0666.

Hopefully, presence of the 'ptmx' node in /dev/pts does not surprise user space.

Quoted 4 lines

This patch has the kernel internally create the [ptmx, c, 5:2] device when mounting devpts filesystem. The permissions for the device node can be specified by the '-o ptmx_mode=0666' option. The default mode is 0666.

NAK

Quoted 2 lines

Hopefully, presence of the 'ptmx' node in /dev/pts does not surprise user space.

If you are going to make major changes requiring user space changes to use them then you can change the user space rather than playing "gee where did that come from" with the existing system.

auto-created, than supporting mknod(2) inside the devpts filesystem. It's not a matter of "changing the user space"; it's a matter of what makes most sense inside the kernel.

Quoted 3 lines

auto-created, than supporting mknod(2) inside the devpts filesystem. It's not a matter of "changing the user space"; it's a matter of what makes most sense inside the kernel.

Having an extra node with different permissions suddenely appear without warning isn't I think good behaviour.

I'm open to being convinced and the
other problems with that code are more pressing.

Quoted 4 lines · Alan Cox

Quoted 3 lines

auto-created, than supporting mknod(2) inside the devpts filesystem. It's not a matter of "changing the user space"; it's a matter of what makes most sense inside the kernel.

Quoted 2 lines

Having an extra node with different permissions suddenely appear without warning isn't I think good behaviour.

Hm. Given that the single-instance mode is the backwards compatibility mode (and it's accessible from outside the filesystem), it probably makes sense to suppress creating this device node when *not* applying the "newns" option, or whatever we want to call it.

Quoted 2 lines

I'm open to being convinced and the
other problems with that code are more pressing.

Agreed.

-hpa

That makes sense. But if Suka does that, then is creating the device when the newns flag is specified ok with you, Alan?

Quoted 5 lines · Alan Cox

Quoted 3 lines

auto-created, than supporting mknod(2) inside the devpts filesystem. It's not a matter of "changing the user space"; it's a matter of what makes most sense inside the kernel.

Having an extra node with different permissions suddenely appear without warning isn't I think good behaviour.

Hm. Given that the single-instance mode is the backwards compatibility mode (and it's accessible from outside the filesystem), it probably makes sense to suppress creating this device node when *not* applying the "newns" option, or whatever we want to call it.

Quoted 3 lines

I'm open to being convinced and the
other problems with that code are more pressing.

Agreed.

-hpa

Quoted 11 lines · H. Peter Anvin [[email protected]] wrote:

Quoted 5 lines · Alan Cox

Quoted 3 lines

auto-created, than supporting mknod(2) inside the devpts filesystem. It's not a matter of "changing the user space"; it's a matter of what makes most sense inside the kernel.

Having an extra node with different permissions suddenely appear without warning isn't I think good behaviour.

Hm. Given that the single-instance mode is the backwards compatibility mode (and it's accessible from outside the filesystem), it probably makes sense to suppress creating this device node when *not* applying the "newns" option, or whatever we want to call it.

I had the new ptmx node only in 'multi-mount' mode initially. But if users want the multi-mount semantics, /dev/ptmx must be a symlink. If its a symlink, we break in the single-mount case (which does not have the ptmx node and we don't support mknod in pts).

Quoted 2 lines

Quoted 2 lines

I'm open to being convinced and the
other problems with that code are more pressing.

Yes, I will look at the latest in linux-next and the ->driver_data approach.

But just to confirm, we do want try and keep single-mount semantics.

Quoted 4 lines

I had the new ptmx node only in 'multi-mount' mode initially. But if users want the multi-mount semantics, /dev/ptmx must be a symlink. If its a symlink, we break in the single-mount case (which does not have the ptmx node and we don't support mknod in pts).

Then have user space make it a file bind mount instead of symlink. That should address all of the backwards compatibility concerns, and allow us to only create it when open.

Quoted 2 lines · Eric W. Biederman

Quoted 2 lines

I had the new ptmx node only in 'multi-mount' mode initially. But if users want the multi-mount semantics, /dev/ptmx must be a symlink. If its a

symlink,

Quoted 3 lines

Quoted 2 lines

we break in the single-mount case (which does not have the ptmx node and we don't support mknod in pts).

Quoted 3 lines

Then have user space make it a file bind mount instead of symlink. That should address all of the backwards compatibility concerns, and allow us to only create it when open.

The right thing is that, if you want to support back-and-forth flipping, to introduce a udev rule which looks for pts/ptmx, links to it if present, and otherwise creates the ptmx device node.

This is *only* required to support back-and-forth, and can be introduced at any time after this patch is in the kernel -- or even before.

The point of making it a bind is to address the concerns about backwards compatibility in user space. In particular security conscious applications and applications that perform sanity checks are known to ignore things if they are the wrong type in the filesystem.

Quoted 2 lines

This is *only* required to support back-and-forth, and can be introduced at any time after this patch is in the kernel -- or even before.

You can use a file bind mount just as easily as a symlink.

As for udev I haven't seen a version that is accessible to mere mortals yet and it doesn't seem like they plan on it being so. Eventually I will get around to making sense of it as we need to make it work in a container but so far it seems to be much more complex then it should be.

I had the new ptmx node only in 'multi-mount' mode initially. But if users want the multi-mount semantics, /dev/ptmx must be a symlink. If its a symlink, we break in the single-mount case (which does not have the ptmx node and we don't support mknod in pts).

Quoted 2 lines

Quoted 2 lines

I'm open to being convinced and the
other problems with that code are more pressing.

Yes, I will look at the latest in linux-next and the ->driver_data approach.

But just to confirm, we do want try and keep single-mount semantics.

Quoted 5 lines · sukadev-r/[email protected] wrote:

I had the new ptmx node only in 'multi-mount' mode initially. But if users want the multi-mount semantics, /dev/ptmx must be a symlink. If its a symlink,
we break in the single-mount case (which does not have the ptmx node and we don't support mknod in pts).

True, but changing that is still a configuration change (adding newns to the fstab); it's not that much more work to change whatever else needs to change.

I personally don't expect a whole lot of back-and-forth; I suspect people will switch from the legacy model to the newns model mostly as part of a distro upgrade.

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

I'm open to being convinced and the
other problems with that code are more pressing.

Yes, I will look at the latest in linux-next and the ->driver_data approach.
But just to confirm, we do want try and keep single-mount semantics.

Certainly for several years at least.

-hpa

Hmm, so, single and multi-mount don't coexist ? i.e some are multi-mounts while others are single-mounts.

The way I looked at is that even if a distro has not yet updated the startup script (fstab), we could use the multi-mount. Maybe a container startup script could change /dev/ptmx to symlink and both types of mounts can work simultaneously.

Would that be unnecessary ?

Quoted 10 lines · H. Peter Anvin [[email protected]] wrote:

Quoted 5 lines · sukadev-r/[email protected] wrote:

I had the new ptmx node only in 'multi-mount' mode initially. But if users want the multi-mount semantics, /dev/ptmx must be a symlink. If its a symlink,
we break in the single-mount case (which does not have the ptmx node and we don't support mknod in pts).

True, but changing that is still a configuration change (adding newns to the fstab); it's not that much more work to change whatever else needs to change.

Hmm, so, single and multi-mount don't coexist ? i.e some are multi-mounts while others are single-mounts.

The way I looked at is that even if a distro has not yet updated the startup script (fstab), we could use the multi-mount. Maybe a container startup script could change /dev/ptmx to symlink and both types of mounts can work simultaneously.

Would that be unnecessary ?

Quoted 5 lines

I personally don't expect a whole lot of back-and-forth; I suspect people will switch from the legacy model to the newns model mostly as part of a distro upgrade.

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

I'm open to being convinced and the
other problems with that code are more pressing.

Yes, I will look at the latest in linux-next and the ->driver_data approach.
But just to confirm, we do want try and keep single-mount semantics.

Certainly for several years at least.

Ok.

Quoted 2 lines

-hpa

TODO:
- Remove even initial kernel mount of devpts ? (If we do, how do we preserve single-mount semantics) ?

Quoted 3 lines · sukadev-r/[email protected] wrote:

TODO:
- Remove even initial kernel mount of devpts ? (If we do, how do we preserve single-mount semantics) ?

Doesn't make sense unless we decide to drop single-mount semantics in the (far) future. As long as we have an instance that services unconnected ptmx instances, it makes sense to have that instance available to the kernel at all times.

I don't like the name "newmnt" for the option; it is not just another mount, but a whole new instance of the pty space.

I observe you didn't incorporate my feedback with regards to get_node().

In this scheme, any and all uses of get_node() are bogus; as such, you're missing the huge opportunity for cleanup that comes along with this whole thing.

This means breaking compatibility in one very minor way, which is if people copy device nodes out of /dev/pts, but I am feeling pretty sure that that is much better than carrying the ugliness that goes along with the current code. Furthermore, if there are anyone who do something that silly, they need to fix it anyway.

The *entire* implementation of devpts_get_tty(), for example, should look like:

struct tty_struct *devpts_get_tty(struct inode *inode)

{
	struct super_block *sb = inode->i_sb;
	
	if (sb->s_magic == DEVPTS_SUPER_MAGIC)
		return (struct tty_struct *)inode->i_private;
	else
		return NULL;	/* Higher layer should return -ENXIO */
}

I really appreciate your tackling this implementation.

-hpa

Quoted 3 lines

I don't like the name "newmnt" for the option; it is not just another mount, but a whole new instance of the pty space.

I agree. Its mostly a place-holder for now. How about newns or newptsns ?

Quoted 1 line

I observe you didn't incorporate my feedback with regards to get_node().

Yes, I have not addressed it yet. Will look into it in the next pass, but will add to the todo list now.

Quoted 4 lines · sukadev-r/[email protected] wrote:

Quoted 2 lines

I don't like the name "newmnt" for the option; it is not just another mount, but a whole new instance of the pty space.

I agree. Its mostly a place-holder for now. How about newns or newptsns ?

I suggest "newinstance", but "newns" works, too.

Quoted 2 lines

Quoted 1 line

I suggest "newinstance", but "newns" works, too.

Could we also use this mount option to 'unshare' a new posix message queue namespace ?

Quoted 5 lines · Cedric Le Goater

Quoted 2 lines

Quoted 1 line

I suggest "newinstance", but "newns" works, too.

Could we also use this mount option to 'unshare' a new posix message queue namespace ?

Sorry, I fail to see the connection with devpts here? Are you suggesting using the same option for another filesystem (if so, which)?

Quoted 4 lines · H. Peter Anvin

Quoted 3 lines · Cedric Le Goater

Quoted 1 line

I suggest "newinstance", but "newns" works, too.

Could we also use this mount option to 'unshare' a new posix message queue namespace ?

Quoted 2 lines

Sorry, I fail to see the connection with devpts here? Are you suggesting using the same option for another filesystem (if so, which)?

yes. the posix message queues are also using a single superblock filesystem.

If we want isolate them (for container needs for example), we also need to create a new sb. The patchset I have uses a clone flag but using a mount 'newns' really sounds like a better idea.

Quoted 9 lines · H. Peter Anvin

Quoted 5 lines · Cedric Le Goater

Quoted 2 lines

Quoted 1 line

I suggest "newinstance", but "newns" works, too.

Could we also use this mount option to 'unshare' a new posix message queue namespace ?

Sorry, I fail to see the connection with devpts here? Are you suggesting using the same option for another filesystem (if so, which)?

yes. the posix message queues are also using a single superblock filesystem.

If we want isolate them (for container needs for example), we also need to create a new sb. The patchset I have uses a clone flag but using a mount 'newns' really sounds like a better idea.

C.

Quoted 7 lines · H. Peter Anvin

Quoted 5 lines · Cedric Le Goater

Quoted 2 lines

Quoted 1 line

I suggest "newinstance", but "newns" works, too.

Could we also use this mount option to 'unshare' a new posix message queue namespace ?

Quoted 2 lines

Sorry, I fail to see the connection with devpts here? Are you suggesting using the same option for another filesystem (if so, which)?

yes. the posix message queues are also using a single superblock filesystem.

If we want isolate them (for container needs for example), we also need to create a new sb. The patchset I have uses a clone flag but using a mount 'newns' really sounds like a better idea.

Quoted 7 lines · Cedric Le Goater

Quoted 6 lines · H. Peter Anvin

Quoted 3 lines · Cedric Le Goater

Quoted 1 line

I suggest "newinstance", but "newns" works, too.

Could we also use this mount option to 'unshare' a new posix message queue namespace ?

Sorry, I fail to see the connection with devpts here? Are you suggesting using the same option for another filesystem (if so, which)?

Quoted 5 lines

yes. the posix message queues are also using a single superblock filesystem.

If we want isolate them (for container needs for example), we also need to create a new sb. The patchset I have uses a clone flag but using a mount 'newns' really sounds like a better idea.

Let's call it newinstance if we are going to use the same option for devpts.

We can update "current->nsproxy->mqueuens" when the newinstance flag is passed and otherwise we can mount whatever is the current mqueue filesystem for the process.

That should be simple and just work.

Eric

Quoted 7 lines · Eric W. Biederman

Quoted 5 lines · Cedric Le Goater

Quoted 4 lines · H. Peter Anvin

Quoted 3 lines · Cedric Le Goater

Quoted 1 line

I suggest "newinstance", but "newns" works, too.

Could we also use this mount option to 'unshare' a new posix message queue namespace ?

Quoted 8 lines

Quoted 7 lines

Quoted 2 lines

Sorry, I fail to see the connection with devpts here? Are you suggesting using the same option for another filesystem (if so, which)?

yes. the posix message queues are also using a single superblock filesystem.

If we want isolate them (for container needs for example), we also need to create a new sb. The patchset I have uses a clone flag but using a mount 'newns' really sounds like a better idea.

Let's call it newinstance if we are going to use the same option for devpts.

ok.

Quoted 3 lines

We can update "current->nsproxy->mqueuens" when the newinstance flag is passed and otherwise we can mount whatever is the current mqueue filesystem for the process.

yes. I'll rebase my previous patchset on this idea.

Quoted 17 lines · Cedric Le Goater

Quoted 7 lines · Eric W. Biederman

Quoted 5 lines · Cedric Le Goater

Quoted 4 lines · H. Peter Anvin

Quoted 3 lines · Cedric Le Goater

Quoted 1 line

I suggest "newinstance", but "newns" works, too.

Could we also use this mount option to 'unshare' a new posix message queue namespace ?

Quoted 8 lines

Quoted 7 lines

Quoted 2 lines

Sorry, I fail to see the connection with devpts here? Are you suggesting using the same option for another filesystem (if so, which)?

yes. the posix message queues are also using a single superblock filesystem.

If we want isolate them (for container needs for example), we also need to create a new sb. The patchset I have uses a clone flag but using a mount 'newns' really sounds like a better idea.

Let's call it newinstance if we are going to use the same option for devpts.

Quoted 7 lines

ok.

Quoted 3 lines

We can update "current->nsproxy->mqueuens" when the newinstance flag is passed and otherwise we can mount whatever is the current mqueue filesystem for the process.

yes. I'll rebase my previous patchset on this idea.

Hello Eric,

I've spent some time on the code and I'm facing some issues with the nsproxy API if we are to keep the mqueue namespace in nsproxy:

	int copy_namespaces(unsigned long flags, struct task_struct *tsk);
	void exit_task_namespaces(struct task_struct *tsk);
	void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new);
	void free_nsproxy(struct nsproxy *ns);
	int unshare_nsproxy_namespaces(unsigned long, struct nsproxy **,
		struct fs_struct *);

nsproxy designed to work closely with the clone flags and it is not well suited to be called elsewhere than clone/unshare.

So I could either :

(1) make a special case for the mqueue namespace and duplicate some code

    to unshare it from ->get_sb() when the option 'newinstance' is used.

(2) to avoid duplicating code, use a clone_flags to unshare the mqueue namespace from ->get_sb() when the option 'newinstance' is used. that sounds silly because we might as well use sys_unshare() in that case.

(3) move mq_ns out of nsproxy. where shall I put it then ?

(3.1) task_struct ?
(3.2) mnt namespace maybe ?

Quoted 1 line

(3.2) mnt namespace maybe ?

I think the last one is the way to go.

mnt_namespace points to mq_ns.

At clone(CLONE_NEWMNT), the new mnt namespace receives a copy of the parent's mq_ns.

If a task does
mount -o newinstance -t mqueue none /dev/mqueue
then its current->nsproxy->mnt_namespace->mqns is switched to point to a new instance of the mq_ns.

mnt_ns->mq_ns has pointers to the sb (and hence root dentry) of the devpts fs.

When a task does mq_open(name, flag), then name is in the mqueuefs found in current->nsproxy->mnt_namespace->mqns.

But if a task does

	clone(CLONE_NEWMNT);
	mount --move /dev/mqueue /oldmqueue
	mount -o newinstance -t mqueue none /dev/mqueue

then that task can find files for the old mqueuefs under /oldmqueue, while mq_open() uses /dev/mqueue since that's what it finds through its mnt_namespace.

Quoted 26 lines · Serge E. Hallyn

Quoted 1 line

(3.2) mnt namespace maybe ?

I think the last one is the way to go.

mnt_namespace points to mq_ns.

At clone(CLONE_NEWMNT), the new mnt namespace receives a copy of the parent's mq_ns.

If a task does
mount -o newinstance -t mqueue none /dev/mqueue
then its current->nsproxy->mnt_namespace->mqns is switched to point to a new instance of the mq_ns.

mnt_ns->mq_ns has pointers to the sb (and hence root dentry) of the devpts fs.

When a task does mq_open(name, flag), then name is in the mqueuefs found in current->nsproxy->mnt_namespace->mqns.

But if a task does

	clone(CLONE_NEWMNT);
	mount --move /dev/mqueue /oldmqueue
	mount -o newinstance -t mqueue none /dev/mqueue

Quoted 3 lines

then that task can find files for the old mqueuefs under /oldmqueue, while mq_open() uses /dev/mqueue since that's what it finds through its mnt_namespace.

Serge if we can make the lookup a pure mount namespace operation i.e. a well known path. Than I don't have any problems with it. Otherwise it looks like abuse of the mount namespace.

In particular. The best approximation I have is to change the kernel to simply lookup "/dev/mqueue" and if not found fallback to the initial kernel instance.

I'm staring at the code as I really haven't looked at it enough but it sure looks like we can transform it into a proper filesystem with just a touch of backwards compatibility logic.
- put the current mq_namespace in the superblock.
- Have open/unlink lookup "/dev/mqueue" to find the filesystem if nothing is found fallback to the internal mount otherwise error.
- Possibly put the tunables in a subdirectory? and
bind mount that subdirectory on top of /proc/sys/fs/mqueue/

I'm too thrilled about the tunables but still.

Are there any security holes or other oddness we would encounter if we did that?

If we can turn the posix mqueue stuff into an honest to goodness filesystem then we completely avoid nsproxy,

and have something
that is much nicer to deal with long term.

Eric

Quoted 30 lines · Eric W. Biederman

Quoted 28 lines · Serge E. Hallyn

Quoted 1 line

(3.2) mnt namespace maybe ?

I think the last one is the way to go.

mnt_namespace points to mq_ns.

At clone(CLONE_NEWMNT), the new mnt namespace receives a copy of the parent's mq_ns.

If a task does
mount -o newinstance -t mqueue none /dev/mqueue
then its current->nsproxy->mnt_namespace->mqns is switched to point to a new instance of the mq_ns.

mnt_ns->mq_ns has pointers to the sb (and hence root dentry) of the devpts fs.

When a task does mq_open(name, flag), then name is in the mqueuefs found in current->nsproxy->mnt_namespace->mqns.

But if a task does

	clone(CLONE_NEWMNT);
	mount --move /dev/mqueue /oldmqueue
	mount -o newinstance -t mqueue none /dev/mqueue

then that task can find files for the old mqueuefs under /oldmqueue, while mq_open() uses /dev/mqueue since that's what it finds through its mnt_namespace.

Quoted 3 lines

Serge if we can make the lookup a pure mount namespace operation i.e. a well known path. Than I don't have any problems with it. Otherwise it looks like abuse of the mount namespace.

Why?

Actually it may work to just put mq_ns straight in the nsproxy.

So let's see:

mq_open(name, flag): opens name under the dentry pointed
to by current->nsproxy->mq_ns->mq_dentry
mount -t mqueue none /dev/mqueue: either returns -EBUSY
or just mounts current->nsproxy->mq_ns->mq_sb under /dev/mqueue
mount -o newinstance -t mqueue none /dev/mqueue: mounts
a new mq_ns instance under /dev/mqueue

While doing

	mount --make-rshared /vs1
	mount --bind /dev/mqueue /vs1/dev/mqueue
	create_a_new_container_chrooted_at(/vs1)
		mount -o newinstance -t mqueue none /dev/mqueue

would allow the host to see the child's /dev/mqueue under /vs1/dev/mqueue while having its own mqueuefs continue to be mounted under /dev/mqueue.

ok. complete isolation would require 2 steps. I guess this is acceptable because mq uses a fs

allowing the host to see the child's /dev/mqueue is also 'a nice to have' feature. unfortunately, we can't do that for all namespaces, for sysvipc for example. So I'm wondering if we should allow it at all ?

Quoted 31 lines · Serge E. Hallyn

Quoted 30 lines · Eric W. Biederman

Quoted 28 lines · Serge E. Hallyn

Quoted 1 line

(3.2) mnt namespace maybe ?

I think the last one is the way to go.

mnt_namespace points to mq_ns.

At clone(CLONE_NEWMNT), the new mnt namespace receives a copy of the parent's mq_ns.

If a task does
mount -o newinstance -t mqueue none /dev/mqueue
then its current->nsproxy->mnt_namespace->mqns is switched to point to a new instance of the mq_ns.

mnt_ns->mq_ns has pointers to the sb (and hence root dentry) of the devpts fs.

When a task does mq_open(name, flag), then name is in the mqueuefs found in current->nsproxy->mnt_namespace->mqns.

But if a task does

	clone(CLONE_NEWMNT);
	mount --move /dev/mqueue /oldmqueue
	mount -o newinstance -t mqueue none /dev/mqueue

then that task can find files for the old mqueuefs under /oldmqueue, while mq_open() uses /dev/mqueue since that's what it finds through its mnt_namespace.

Quoted 7 lines

Quoted 3 lines

Serge if we can make the lookup a pure mount namespace operation i.e. a well known path. Than I don't have any problems with it. Otherwise it looks like abuse of the mount namespace.

Why?

Actually it may work to just put mq_ns straight in the nsproxy.

ok. that's the path I was taking.

Quoted 10 lines

So let's see:

mq_open(name, flag): opens name under the dentry pointed
to by current->nsproxy->mq_ns->mq_dentry
mount -t mqueue none /dev/mqueue: either returns -EBUSY
or just mounts current->nsproxy->mq_ns->mq_sb under /dev/mqueue
mount -o newinstance -t mqueue none /dev/mqueue: mounts
a new mq_ns instance under /dev/mqueue

Quoted 8 lines

While doing

	mount --make-rshared /vs1
	mount --bind /dev/mqueue /vs1/dev/mqueue
	create_a_new_container_chrooted_at(/vs1)
		mount -o newinstance -t mqueue none /dev/mqueue

would allow the host to see the child's /dev/mqueue under /vs1/dev/mqueue while having its own mqueuefs continue to be mounted under /dev/mqueue.

ok. complete isolation would require 2 steps. I guess this is acceptable because mq uses a fs

allowing the host to see the child's /dev/mqueue is also 'a nice to have' feature. unfortunately, we can't do that for all namespaces, for sysvipc for example. So I'm wondering if we should allow it at all ?

Thanks,

C.

Quoted 21 lines · Serge E. Hallyn

Quoted 1 line

(3.2) mnt namespace maybe ?

I think the last one is the way to go.

mnt_namespace points to mq_ns.

At clone(CLONE_NEWMNT), the new mnt namespace receives a copy of the parent's mq_ns.

If a task does
mount -o newinstance -t mqueue none /dev/mqueue
then its current->nsproxy->mnt_namespace->mqns is switched to point to a new instance of the mq_ns.

mnt_ns->mq_ns has pointers to the sb (and hence root dentry) of the devpts fs.

When a task does mq_open(name, flag), then name is in the mqueuefs found in current->nsproxy->mnt_namespace->mqns.

But if a task does

Quoted 7 lines

	clone(CLONE_NEWMNT);
	mount --move /dev/mqueue /oldmqueue
	mount -o newinstance -t mqueue none /dev/mqueue

then that task can find files for the old mqueuefs under /oldmqueue, while mq_open() uses /dev/mqueue since that's what it finds through its mnt_namespace.

Serge if we can make the lookup a pure mount namespace operation i.e. a well known path. Than I don't have any problems with it. Otherwise it looks like abuse of the mount namespace.

In particular. The best approximation I have is to change the kernel to simply lookup "/dev/mqueue" and if not found fallback to the initial kernel instance.

I'm staring at the code as I really haven't looked at it enough but it sure looks like we can transform it into a proper filesystem with just a touch of backwards compatibility logic.
- put the current mq_namespace in the superblock.

- Have open/unlink lookup "/dev/mqueue" to find the filesystem if nothing is found fallback to the internal mount otherwise error.

- Possibly put the tunables in a subdirectory? and bind mount that subdirectory on top of /proc/sys/fs/mqueue/

I'm too thrilled about the tunables but still.

Are there any security holes or other oddness we would encounter if we did that?

If we can turn the posix mqueue stuff into an honest to goodness filesystem then we completely avoid nsproxy, and have something that is much nicer to deal with long term.

Eric

Quoted 35 lines · Eric W. Biederman

Quoted 21 lines · Serge E. Hallyn

Quoted 1 line

(3.2) mnt namespace maybe ?

I think the last one is the way to go.

mnt_namespace points to mq_ns.

At clone(CLONE_NEWMNT), the new mnt namespace receives a copy of the parent's mq_ns.

If a task does
mount -o newinstance -t mqueue none /dev/mqueue
then its current->nsproxy->mnt_namespace->mqns is switched to point to a new instance of the mq_ns.

mnt_ns->mq_ns has pointers to the sb (and hence root dentry) of the devpts fs.

When a task does mq_open(name, flag), then name is in the mqueuefs found in current->nsproxy->mnt_namespace->mqns.

But if a task does

Quoted 7 lines

	clone(CLONE_NEWMNT);
	mount --move /dev/mqueue /oldmqueue
	mount -o newinstance -t mqueue none /dev/mqueue

then that task can find files for the old mqueuefs under /oldmqueue, while mq_open() uses /dev/mqueue since that's what it finds through its mnt_namespace.

Serge if we can make the lookup a pure mount namespace operation i.e. a well known path. Than I don't have any problems with it. Otherwise it looks like abuse of the mount namespace.

Quoted 8 lines

In particular. The best approximation I have is to change the kernel to simply lookup "/dev/mqueue" and if not found fallback to the initial kernel instance.

I'm staring at the code as I really haven't looked at it enough but it sure looks like we can transform it into a proper filesystem with just a touch of backwards compatibility logic.
- put the current mq_namespace in the superblock.

ok that is done. using the s_fs_info.

Quoted 2 lines

- Have open/unlink lookup "/dev/mqueue" to find the filesystem if nothing is found fallback to the internal mount otherwise error.

what do you mean ? loop on the mnt_namespace of current to find a 'struct vfsmount' pointing to /dev/mqueue ?

Quoted 5 lines

(3) move mq_ns out of nsproxy. where shall I put it then ?

(3.1) task_struct ?
(3.2) mnt namespace maybe ?

I think the last one is the way to go.

mnt_namespace points to mq_ns.

At clone(CLONE_NEWMNT), the new mnt namespace receives a copy of the parent's mq_ns.

If a task does
mount -o newinstance -t mqueue none /dev/mqueue
then its current->nsproxy->mnt_namespace->mqns is switched to point to a new instance of the mq_ns.

mnt_ns->mq_ns has pointers to the sb (and hence root dentry) of the devpts fs.

When a task does mq_open(name, flag), then name is in the mqueuefs found in current->nsproxy->mnt_namespace->mqns.

But if a task does

	clone(CLONE_NEWMNT);
	mount --move /dev/mqueue /oldmqueue
	mount -o newinstance -t mqueue none /dev/mqueue

then that task can find files for the old mqueuefs under /oldmqueue, while mq_open() uses /dev/mqueue since that's what it finds through its mnt_namespace.

Quoted 6 lines

Quoted 5 lines

(3) move mq_ns out of nsproxy. where shall I put it then ?

(3.1) task_struct ?
(3.2) mnt namespace maybe ?

Quoted 6 lines

I think the last one is the way to go.

mnt_namespace points to mq_ns.

At clone(CLONE_NEWMNT), the new mnt namespace receives a copy of the parent's mq_ns.

hmm, hmm, hmm, I still thinking about this.

Quoted 5 lines

If a task does
mount -o newinstance -t mqueue none /dev/mqueue
then its current->nsproxy->mnt_namespace->mqns is switched to point to a new instance of the mq_ns.

Quoted 2 lines

mnt_ns->mq_ns has pointers to the sb (and hence root dentry) of the devpts fs.

[trying to understand what you have in mind ]

why not keep the 'struct vfsmount' in the mq_ns, as the code is doing today ? the vfsmount holds both the root dentry and the superblock.

Quoted 5 lines

When a task does mq_open(name, flag), then name is in the mqueuefs found in current->nsproxy->mnt_namespace->mqns.

But if a task does

Quoted 7 lines

	clone(CLONE_NEWMNT);
	mount --move /dev/mqueue /oldmqueue
	mount -o newinstance -t mqueue none /dev/mqueue

then that task can find files for the old mqueuefs under /oldmqueue, while mq_open() uses /dev/mqueue since that's what it finds through its mnt_namespace.

That I don't like.

Even though posix mqueue objects can outlive a process, I don't think a process should be able to peek and poke in a message queue namespace other than his. this is the basic principle of the namespaces :
isolation. Am I wrong ?

couldn't we just return EACCES ? (not posix)

Quoted 12 lines

Quoted 5 lines

When a task does mq_open(name, flag), then name is in the mqueuefs found in current->nsproxy->mnt_namespace->mqns.

But if a task does

Quoted 7 lines

	clone(CLONE_NEWMNT);
	mount --move /dev/mqueue /oldmqueue
	mount -o newinstance -t mqueue none /dev/mqueue

then that task can find files for the old mqueuefs under /oldmqueue, while mq_open() uses /dev/mqueue since that's what it finds through its mnt_namespace.

Quoted 6 lines

That I don't like.

Even though posix mqueue objects can outlive a process, I don't think a process should be able to peek and poke in a message queue namespace other than his. this is the basic principle of the namespaces :
isolation. Am I wrong ?

Yes you are wrong in this case. In particular consider mounts propagation, which allows you to to examine both a child namespace's mounts namespace, and, through the child's /sys (presumably mounted under /vs1/sys), his network namespace and eventually device namespace.

Quoted 1 line

couldn't we just return EACCES ? (not posix)

We could. And if we think there is really no value in viewing a child namespace's mqueuefs then we may as well. I just want to make it clear that the proposed semantics support it as an option.

Hello Eric,

I've spent some time on the code and I'm facing some issues with the nsproxy API if we are to keep the mqueue namespace in nsproxy:

	int copy_namespaces(unsigned long flags, struct task_struct *tsk);
	void exit_task_namespaces(struct task_struct *tsk);
	void switch_task_namespaces(struct task_struct *tsk, struct nsproxy
*new);
	void free_nsproxy(struct nsproxy *ns);
	int unshare_nsproxy_namespaces(unsigned long, struct nsproxy **,
		struct fs_struct *);

nsproxy designed to work closely with the clone flags and it is not well suited to be called elsewhere than clone/unshare.

So I could either :

(1) make a special case for the mqueue namespace and duplicate some code

    to unshare it from ->get_sb() when the option 'newinstance' is used.

(2) to avoid duplicating code, use a clone_flags to unshare the mqueue namespace from ->get_sb() when the option 'newinstance' is used. that sounds silly because we might as well use sys_unshare() in that case.

(3) move mq_ns out of nsproxy. where shall I put it then ?

    (3.1) task_struct ? 

(3.2) mnt namespace maybe ?

BTW, have you taken a look at what dave resent in July ?

	https://lists.linux-foundation.org/pipermail/containers/2008-July/011776.html

This is the mqueue namespace patchset using CLONE_NEWIPC to unshare.

I wonder if there are arguments against that approach. I might have forgotten some of the issues with unshare as I haven't looked at the code for some time.

Quoted 13 lines · Cedric Le Goater

Hello Eric,

I've spent some time on the code and I'm facing some issues with the nsproxy API if we are to keep the mqueue namespace in nsproxy:

	int copy_namespaces(unsigned long flags, struct task_struct *tsk);
	void exit_task_namespaces(struct task_struct *tsk);
	void switch_task_namespaces(struct task_struct *tsk, struct nsproxy
*new);
	void free_nsproxy(struct nsproxy *ns);
	int unshare_nsproxy_namespaces(unsigned long, struct nsproxy **,
		struct fs_struct *);

Quoted 2 lines

nsproxy designed to work closely with the clone flags and it is not well suited to be called elsewhere than clone/unshare.

Right.

Quoted 4 lines

So I could either :

(1) make a special case for the mqueue namespace and duplicate some code

    to unshare it from ->get_sb() when the option 'newinstance' is used.

This is probably the best option.

You should be able to just wrap create_new_namespaces(flags=0,...) to create a new one.

And then actually perform the unshare.

Which means in practice you will bump the ref count on mq_ns and then drop it and insert a new mq_ns pointer but overall that shouldn't be too bad.