Re: Read-only `slaves` with shared subtrees?

[Ram Pai]

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 <linuxram@xxxxxxxxxx> 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