On Sat, Apr 10, 2021 at 12:31:02AM +0000, Al Viro wrote: > On Fri, Apr 09, 2021 at 06:24:21PM +0200, Christian Brauner wrote: > > > Reading through the codebase of ecryptfs it currently takes path->mnt > > and then retrieves that path whenever it needs to perform operations in > > the underlying filesystem. Simply drop the old path->mnt once we've > > created a private mount and place the new private mnt into path->mnt. > > This should be all that is needed to make this work since ecryptfs uses > > the same lower path's vfsmount to construct the paths it uses to operate > > on the underlying filesystem. > > > + mnt = clone_private_mount(&path); > > Incidentally, why is that thing anything other than a trivial wrapper > for mnt_clone_internal() (if that - I'm not convinced that the check of > unbindable is the right thing to do here). Miklos? The unbindable check is irrelevant at least for both ecryptfs and for the corresponding cachefiles change I sent out since they don't care about it. In practice it doesn't matter to be honest. MS_UNBINDABLE is wildly esoteric in userspace (We had a glaring bug with that some time ago that went completely unnoticed for years.). Especially unlikely to be used for a users home directory (ecryptfs) or /var/cache/fscache (cachefiles). So even by leaving this check in it's very unlikely for any regressions to appear. I hadn't seen mnt_clone_internal() but it's different in so far as it sets MNT_INTERNAL whereas clone_private_mount() uses MNT_NS_INTERNAL. Which points me to another potential problem here: clone_private_mount() seems to want kern_unmount() to be called instead of just a simple mntput()? If that's relevant then I think the unbindable check should probably move out of clone_private_mount() and into overlayfs itself but the rest be kept.
