Linux-Fsdevel Archive mirror
 help / color / mirror / Atom feed
From: Christian Brauner <christian.brauner@ubuntu.com>
To: David Howells <dhowells@redhat.com>
Cc: Christian Brauner <brauner@kernel.org>,
	linux-cachefs@redhat.com, linux-fsdevel@vger.kernel.org,
	Amir Goldstein <amir73il@gmail.com>
Subject: Re: [PATCH] cachefiles: use private mounts in cache->mnt
Date: Tue, 6 Apr 2021 11:49:35 +0200	[thread overview]
Message-ID: <20210406094935.wg3wrctebqs466hz@wittgenstein> (raw)
In-Reply-To: <107463.1617700345@warthog.procyon.org.uk>

On Tue, Apr 06, 2021 at 10:12:25AM +0100, David Howells wrote:
> Christian Brauner <brauner@kernel.org> wrote:
> 
> > Besides that - and probably irrelevant from the perspective of a
> > cachefiles developer - it also makes things simpler for a variety of
> > other vfs features. One concrete example is fanotify.
> 
> What about cachefilesd?  That walks over the tree regularly, stats things and
> maybe deletes things.  Should that be in a private mount/namespace too?

You mean running the userspace cachefilesd daemon in a separate mount
namespace? I think that would make a lot of sense. Either the daemon
could manage a separate private mount namespace itself or if you support
systemd service files you could set:

PrivateMounts=yes

in the service file which:

"Takes a boolean parameter. If set, the processes of this unit will be
run in their own private file system (mount) namespace with all mount
propagation from the processes towards the host's main file system
namespace turned off. This means any file system mount points
established or removed by the unit's processes will be private to them
and not be visible to the host."

(Fwiw, Debian still ships /etc/init.d/cachefilesd which seems a bit
antique imho.)

> 
> > This seems a rather desirable property as the underlying path can't e.g.
> > suddenly go from read-write to read-only and in general it means that
> > cachefiles is always in full control of the underlying mount after the
> > user has allowed it to be used as a cache.
> 
> That's not entirely true, but I guess that emergency R/O conversion isn't a
> case that's worrisome - and, in any case, only affects the superblock.
> 
> >  	ret = -EINVAL;
> > -	if (mnt_user_ns(path.mnt) != &init_user_ns) {
> > +	if (mnt_user_ns(cache->mnt) != &init_user_ns) {
> >  		pr_warn("File cache on idmapped mounts not supported");
> >  		goto error_unsupported;
> >  	}
> 
> Is it worth doing this check before calling clone_private_mount()?

Yes, it's safe to do that. It's just my paranoia that made me write it
this way. In order to create an idmapped mount
real_mount(&path->mnt)->mnt_ns->seq must be 0, i.e. an anonymous mount
which can't be found through the fileystem. So in order for path->mnt to
be idmapped it must be already attached to the fileystem and we don't
allow mnt_userns to change (for good reasons).

> 
> > +	cache_path = path;
> > +	cache_path.mnt = cache->mnt;
> 
> Seems pointless to copy all of path into cache_path rather than just
> path.dentry.

Sure, will change to:

cache_path.dentry = path.dentry;
cache_path.mnt = cache->mnt;

Christian

      reply	other threads:[~2021-04-06  9:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-05 16:46 [PATCH] cachefiles: use private mounts in cache->mnt Christian Brauner
2021-04-06  9:12 ` David Howells
2021-04-06  9:49   ` Christian Brauner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210406094935.wg3wrctebqs466hz@wittgenstein \
    --to=christian.brauner@ubuntu.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=linux-cachefs@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).