sorry forgot to copy Eric.
On Wed, Sep 20, 2017 at 12:39:54PM -0700, Ram Pai wrote:
> On Tue, Sep 19, 2017 at 04:18:02PM -0700, Dawid Ciezarkiewicz wrote:
> > On Mon, Sep 18, 2017 at 1:47 PM, Ram Pai <linux...@us.ibm.com> wrote:
> > > It is possible to make a slave mount readonly, by remounting it with
> > > 'ro' flags.
> > >
> > > something like
> > >
> > > mount -o bind,remount,ro <slave-mount-dir>
> > >
> > > Any mount-propagation events reaching a read-only-slave does
> > > inherit the slave attribute. However it does not inherit the
> > > read-only attribute.
> >
> > I did try manually remounting, and it worked for me. If this could be
> > done atomically
> > (which I assume can't be, in the userspace) it could even be a workaround.
> >
> > > Should it inherit? or should it not? -- that has not been thought
> > > off AFAICT. it think we should let it inherit.
> >
> > It makes sense, and it would work in my use-case. I wonder
> > if that would break any existing expectations though.
>
> It could break existing expectations, for mounts created by propagation.
> This needs to be thought through. Also Should the same semantics
> apply to MNT_NOSUID, MNT_NOEXEC etc etc?
>
> Copying Eric. he should be able to tell if any of the container
> infrastructure assumes anything about mounts propagated to read-only
> mounts.
>
>
> >
> > I could at least test such a patch, it seems like a tiny change.
> > Should I give it a try and submit a patch? If you could PM me any pointers
> > it could help a lot since I'm not familiar with FS internals. So far I got
> > here:
>
> Here is a rough patch which will accomplish what you want; not
> compile-tested nor tested.
>
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index f8893dc..3239adc 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1061,6 +1061,9 @@ static struct mount *clone_mnt(struct mount *old,
> struct dentry *root,
> list_add_tail(&mnt->mnt_instance, &sb->s_mounts);
> unlock_mount_hash();
>
> + if (flag & CL_READONLY)
> + mnt->mnt.mnt_flags |= MNT_READONLY;
> +
> if ((flag & CL_SLAVE) ||
> ((flag & CL_SHARED_TO_SLAVE) && IS_MNT_SHARED(old))) {
> list_add(&mnt->mnt_slave, &old->mnt_slave_list);
> diff --git a/fs/pnode.c b/fs/pnode.c
> index 53d411a..aeb5b47 100644
> --- a/fs/pnode.c
> +++ b/fs/pnode.c
> @@ -262,6 +262,8 @@ static int propagate_one(struct mount *m)
> /* Notice when we are propagating across user namespaces */
> if (m->mnt_ns->user_ns != user_ns)
> type |= CL_UNPRIVILEGED;
> + if (m->mnt.mnt_flags & MNT_READONLY)
> + type |= CL_READONLY;
> child = copy_tree(last_source, last_source->mnt.mnt_root, type);
> if (IS_ERR(child))
> return PTR_ERR(child);
> diff --git a/fs/pnode.h b/fs/pnode.h
> index dc87e65..7c59469 100644
> --- a/fs/pnode.h
> +++ b/fs/pnode.h
> @@ -29,6 +29,7 @@
> #define CL_SHARED_TO_SLAVE 0x20
> #define CL_UNPRIVILEGED 0x40
> #define CL_COPY_MNT_NS_FILE 0x80
> +#define CL_READONLY 0x100
>
> #define CL_COPY_ALL (CL_COPY_UNBINDABLE | CL_COPY_MNT_NS_FILE)
>
> RP
--
Ram Pai
