diff mbox

fs: show locked and lock_ro options in mountinfo

Message ID 1427488774-5077-1-git-send-email-avagin@openvz.org (mailing list archive)
State New, archived
Headers show

Commit Message

Andrey Vagin March 27, 2015, 8:39 p.m. UTC
I don't see any reasons to hide them. This information can help to
understand errors.

And this information is required for correct checkpoint/restore of mount
namespaces.

Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
 fs/proc_namespace.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Richard Weinberger March 27, 2015, 9:42 p.m. UTC | #1
On Fri, Mar 27, 2015 at 9:39 PM, Andrey Vagin <avagin@openvz.org> wrote:
> I don't see any reasons to hide them. This information can help to
> understand errors.

Because these flags are set/read only internally by the VFS. In contrast
to the other flags shown by mountinfo MNT_LOCKED is not a mount option.

Why does it help to debug errors?
How would a user know that mount() with MS_BIND returns EINVAL because
the mount source is MNT_LOCKED? This information is useless for her.
If you argue like that you'd have to expose the whole VFS state to userland.

> And this information is required for correct checkpoint/restore of mount
> namespaces.

Why especially MNT_LOCKED and not all the other flags used by VFS?
Say MNT_DOOMED?
Andrei Vagin March 27, 2015, 10:35 p.m. UTC | #2
2015-03-28 0:42 GMT+03:00 Richard Weinberger <richard.weinberger@gmail.com>:
> On Fri, Mar 27, 2015 at 9:39 PM, Andrey Vagin <avagin@openvz.org> wrote:
>> I don't see any reasons to hide them. This information can help to
>> understand errors.
>
> Because these flags are set/read only internally by the VFS. In contrast
> to the other flags shown by mountinfo MNT_LOCKED is not a mount option.

But this flag is set as a result of the specified user action, when he
unshares userns and mntns. This flag affects visiable behaviour.

>
> Why does it help to debug errors?
> How would a user know that mount() with MS_BIND returns EINVAL because
> the mount source is MNT_LOCKED? This information is useless for her.

If I see lock_ro, I can be sure that mount -o remount,bind,rw /XXX will fail.
If I see locked, I  know that this mount can't be umounted or moved
and can be bind-mounted only recursively.

If a user see these flags, he can check that a mount namespace is
configured correctly without security issues.

Sorry but I don't understand why you think that this information is
useless for users.

> If you argue like that you'd have to expose the whole VFS state to userland.

I have not noticed other MNT_LOCK_* flags. I should think more about
what information are a really required for dumping mount namespaces.

>
>> And this information is required for correct checkpoint/restore of mount
>> namespaces.
>
> Why especially MNT_LOCKED and not all the other flags used by VFS?

My goal is to dump enough information about a mount namespace to be
able to restore it back later. I don't know how to do this without
knowledge about locked mounts. I will think.

> Say MNT_DOOMED?

Mounts with MNT_DOOMED are never shown in mountinfo, are they?

Thank you for looking at this patch.


>
> --
> Thanks,
> //richard
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Weinberger March 27, 2015, 10:47 p.m. UTC | #3
Hi!

Am 27.03.2015 um 23:35 schrieb Andrey Wagin:
> 2015-03-28 0:42 GMT+03:00 Richard Weinberger <richard.weinberger@gmail.com>:
>> On Fri, Mar 27, 2015 at 9:39 PM, Andrey Vagin <avagin@openvz.org> wrote:
>>> I don't see any reasons to hide them. This information can help to
>>> understand errors.
>>
>> Because these flags are set/read only internally by the VFS. In contrast
>> to the other flags shown by mountinfo MNT_LOCKED is not a mount option.
> 
> But this flag is set as a result of the specified user action, when he
> unshares userns and mntns. This flag affects visiable behaviour.

It is a implicit result. Used by the VFS internally.
If you expose it it becomes ABI and changing the behavior will be
tricky or impossible.

>>
>> Why does it help to debug errors?
>> How would a user know that mount() with MS_BIND returns EINVAL because
>> the mount source is MNT_LOCKED? This information is useless for her.
> 
> If I see lock_ro, I can be sure that mount -o remount,bind,rw /XXX will fail.
> If I see locked, I  know that this mount can't be umounted or moved
> and can be bind-mounted only recursively.
> 
> If a user see these flags, he can check that a mount namespace is
> configured correctly without security issues.
> 
> Sorry but I don't understand why you think that this information is
> useless for users.

You can only know if you know how the VFS works internally.
If know that EINVAL from mount(2) with MS_BIND can be caused by MNT_LOCKED
because I know the source. I bet you know the source too. But not Joey random
admin who looks into mountinfo to figure out why something does not work.

If you expose MNT_LOCKED to userspace you'll have to update also the mount(2)
manpage with all glory details of that flag.

>> If you argue like that you'd have to expose the whole VFS state to userland.
> 
> I have not noticed other MNT_LOCK_* flags. I should think more about
> what information are a really required for dumping mount namespaces.
> 
>>
>>> And this information is required for correct checkpoint/restore of mount
>>> namespaces.
>>
>> Why especially MNT_LOCKED and not all the other flags used by VFS?
> 
> My goal is to dump enough information about a mount namespace to be
> able to restore it back later. I don't know how to do this without
> knowledge about locked mounts. I will think.

How do you plan to restore a MNT_LOCKED mount?
IIRC we have currently no way to directly set MNT_LOCKED from userspace.

>> Say MNT_DOOMED?
> 
> Mounts with MNT_DOOMED are never shown in mountinfo, are they?

It was just an example. There are other flags too, did you double check
which ones you really need?

To make the story short, my concern is that exposing such flags to userspace
has to be well thought. :-)
As long they are just internal we can change them as we like, as soon userspace
depends somehow on it the pain begins.

Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrei Vagin March 31, 2015, 3:15 p.m. UTC | #4
2015-03-28 1:47 GMT+03:00 Richard Weinberger <richard@nod.at>:
> Hi!
>
> Am 27.03.2015 um 23:35 schrieb Andrey Wagin:
>> 2015-03-28 0:42 GMT+03:00 Richard Weinberger <richard.weinberger@gmail.com>:
>>> On Fri, Mar 27, 2015 at 9:39 PM, Andrey Vagin <avagin@openvz.org> wrote:
>>>> I don't see any reasons to hide them. This information can help to
>>>> understand errors.
>>>
>>> Because these flags are set/read only internally by the VFS. In contrast
>>> to the other flags shown by mountinfo MNT_LOCKED is not a mount option.
>>
>> But this flag is set as a result of the specified user action, when he
>> unshares userns and mntns. This flag affects visiable behaviour.
>
> It is a implicit result. Used by the VFS internally.
> If you expose it it becomes ABI and changing the behavior will be
> tricky or impossible.
>
>>>
>>> Why does it help to debug errors?
>>> How would a user know that mount() with MS_BIND returns EINVAL because
>>> the mount source is MNT_LOCKED? This information is useless for her.
>>
>> If I see lock_ro, I can be sure that mount -o remount,bind,rw /XXX will fail.
>> If I see locked, I  know that this mount can't be umounted or moved
>> and can be bind-mounted only recursively.
>>
>> If a user see these flags, he can check that a mount namespace is
>> configured correctly without security issues.
>>
>> Sorry but I don't understand why you think that this information is
>> useless for users.
>
> You can only know if you know how the VFS works internally.
> If know that EINVAL from mount(2) with MS_BIND can be caused by MNT_LOCKED
> because I know the source. I bet you know the source too. But not Joey random
> admin who looks into mountinfo to figure out why something does not work.
>
> If you expose MNT_LOCKED to userspace you'll have to update also the mount(2)
> manpage with all glory details of that flag.
>
>>> If you argue like that you'd have to expose the whole VFS state to userland.
>>
>> I have not noticed other MNT_LOCK_* flags. I should think more about
>> what information are a really required for dumping mount namespaces.
>>
>>>
>>>> And this information is required for correct checkpoint/restore of mount
>>>> namespaces.
>>>
>>> Why especially MNT_LOCKED and not all the other flags used by VFS?
>>
>> My goal is to dump enough information about a mount namespace to be
>> able to restore it back later. I don't know how to do this without
>> knowledge about locked mounts. I will think.
>
> How do you plan to restore a MNT_LOCKED mount?
> IIRC we have currently no way to directly set MNT_LOCKED from userspace.

It's the second question. The first question is how to check that we
will be able to restore what we are dumping.

If CRIU meets something what it doesn't know how to restore, it (must)
refuses to dump this configuration.

>
>>> Say MNT_DOOMED?
>>
>> Mounts with MNT_DOOMED are never shown in mountinfo, are they?
>
> It was just an example. There are other flags too, did you double check
> which ones you really need?
>
> To make the story short, my concern is that exposing such flags to userspace
> has to be well thought. :-)
> As long they are just internal we can change them as we like, as soon userspace
> depends somehow on it the pain begins.

I'm agree with you. I'm going to think more about this. Thank you for response.

>
> Thanks,
> //richard
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman March 31, 2015, 9:29 p.m. UTC | #5
Andrey Wagin <avagin@gmail.com> writes:

> 2015-03-28 1:47 GMT+03:00 Richard Weinberger <richard@nod.at>:
>> Hi!
>>
>> Am 27.03.2015 um 23:35 schrieb Andrey Wagin:
>>> 2015-03-28 0:42 GMT+03:00 Richard Weinberger <richard.weinberger@gmail.com>:
>>>> On Fri, Mar 27, 2015 at 9:39 PM, Andrey Vagin <avagin@openvz.org> wrote:
>>>>> I don't see any reasons to hide them. This information can help to
>>>>> understand errors.
>>>>
>>>> Because these flags are set/read only internally by the VFS. In contrast
>>>> to the other flags shown by mountinfo MNT_LOCKED is not a mount option.
>>>
>>> But this flag is set as a result of the specified user action, when he
>>> unshares userns and mntns. This flag affects visiable behaviour.
>>
>> It is a implicit result. Used by the VFS internally.
>> If you expose it it becomes ABI and changing the behavior will be
>> tricky or impossible.
>>
>>>>
>>>> Why does it help to debug errors?
>>>> How would a user know that mount() with MS_BIND returns EINVAL because
>>>> the mount source is MNT_LOCKED? This information is useless for her.
>>>
>>> If I see lock_ro, I can be sure that mount -o remount,bind,rw /XXX will fail.
>>> If I see locked, I  know that this mount can't be umounted or moved
>>> and can be bind-mounted only recursively.
>>>
>>> If a user see these flags, he can check that a mount namespace is
>>> configured correctly without security issues.
>>>
>>> Sorry but I don't understand why you think that this information is
>>> useless for users.
>>
>> You can only know if you know how the VFS works internally.
>> If know that EINVAL from mount(2) with MS_BIND can be caused by MNT_LOCKED
>> because I know the source. I bet you know the source too. But not Joey random
>> admin who looks into mountinfo to figure out why something does not work.
>>
>> If you expose MNT_LOCKED to userspace you'll have to update also the mount(2)
>> manpage with all glory details of that flag.
>>
>>>> If you argue like that you'd have to expose the whole VFS state to userland.
>>>
>>> I have not noticed other MNT_LOCK_* flags. I should think more about
>>> what information are a really required for dumping mount namespaces.
>>>
>>>>
>>>>> And this information is required for correct checkpoint/restore of mount
>>>>> namespaces.
>>>>
>>>> Why especially MNT_LOCKED and not all the other flags used by VFS?
>>>
>>> My goal is to dump enough information about a mount namespace to be
>>> able to restore it back later. I don't know how to do this without
>>> knowledge about locked mounts. I will think.
>>
>> How do you plan to restore a MNT_LOCKED mount?
>> IIRC we have currently no way to directly set MNT_LOCKED from userspace.
>
> It's the second question. The first question is how to check that we
> will be able to restore what we are dumping.
>
> If CRIU meets something what it doesn't know how to restore, it (must)
> refuses to dump this configuration.

As a practical matter if the underlying directory is empty, and will
remain empty MNT_LOCKED does not matter.


>>>> Say MNT_DOOMED?
>>>
>>> Mounts with MNT_DOOMED are never shown in mountinfo, are they?
>>
>> It was just an example. There are other flags too, did you double check
>> which ones you really need?
>>
>> To make the story short, my concern is that exposing such flags to userspace
>> has to be well thought. :-)
>> As long they are just internal we can change them as we like, as soon userspace
>> depends somehow on it the pain begins.
>
> I'm agree with you. I'm going to think more about this. Thank you for
> response.

A big question from me is do you have the ability to find the user
namespace of a mount namespace.  Without that these mount flags do not
matter.

I would think getting the user namespace of mount namespace and getting
the mount propgation tree correct would precede little things like
worrying if the mount propagation state is correct.

Eric

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

Patch

diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index 8db932d..ad86134 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -66,6 +66,8 @@  static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt)
 		{ MNT_NOATIME, ",noatime" },
 		{ MNT_NODIRATIME, ",nodiratime" },
 		{ MNT_RELATIME, ",relatime" },
+		{ MNT_LOCKED, ",locked" },
+		{ MNT_LOCK_READONLY, ",lock_ro"},
 		{ 0, NULL }
 	};
 	const struct proc_fs_info *fs_infop;