From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751864AbdITTkD (ORCPT ); Wed, 20 Sep 2017 15:40:03 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:38060 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751692AbdITTkB (ORCPT ); Wed, 20 Sep 2017 15:40:01 -0400 Date: Wed, 20 Sep 2017 12:39:54 -0700 From: Ram Pai To: Dawid Ciezarkiewicz Cc: linux-kernel@vger.kernel.org Subject: Re: Read-only `slaves` with shared subtrees? Reply-To: Ram Pai References: <20170918204710.GI5698@ram.oc3035372033.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-12-10) X-TM-AS-GCONF: 00 x-cbid: 17092019-0040-0000-0000-000003A54BDF X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007768; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000230; SDB=6.00919887; UDB=6.00462182; IPR=6.00700114; BA=6.00005599; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017225; XFM=3.00000015; UTC=2017-09-20 19:39:59 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17092019-0041-0000-0000-0000079A506A Message-Id: <20170920193954.GK5698@ram.oc3035372033.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-09-20_05:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1709200262 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 19, 2017 at 04:18:02PM -0700, Dawid Ciezarkiewicz wrote: > On Mon, Sep 18, 2017 at 1:47 PM, Ram Pai wrote: > > It is possible to make a slave mount readonly, by remounting it with > > 'ro' flags. > > > > something like > > > > mount -o bind,remount,ro > > > > 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