Re: [PATCH v3 3/3] fs: store real path instead of fake path in backing file f_path

From: Al Viro <viro@zeniv.linux.org.uk>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>,
	Christian Brauner <brauner@kernel.org>,
	Paul Moore <paul@paul-moore.com>,
	James Morris <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	Mimi Zohar <zohar@linux.ibm.com>,
	linux-security-module@vger.kernel.org,
	linux-integrity@vger.kernel.org, linux-unionfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v3 3/3] fs: store real path instead of fake path in backing file f_path
Date: Tue, 10 Oct 2023 17:55:04 +0100	[thread overview]
Message-ID: <20231010165504.GP800259@ZenIV> (raw)
In-Reply-To: <CAJfpeguEf71ZknP5rGU9YNtJTp1wBGBKyv6M0JZ=5ETuaipDxQ@mail.gmail.com>

On Tue, Oct 10, 2023 at 03:34:45PM +0200, Miklos Szeredi wrote:
> On Tue, 10 Oct 2023 at 15:17, Amir Goldstein <amir73il@gmail.com> wrote:
> 
> > Sorry, you asked about ovl mount.
> > To me it makes sense that if users observe ovl paths in writable mapped
> > memory, that ovl should not be remounted RO.
> > Anyway, I don't see a good reason to allow remount RO for ovl in that case.
> > Is there?
> 
> Agreed.
> 
> But is preventing remount RO important enough to warrant special
> casing of backing file in generic code?  I'm not convinced either
> way...

You definitely want to guarantee that remounting filesystem r/o
prevents the changes of visible contents; it's not just POSIX,
it's a fairly basic common assumption about any local filesystems.

Whether that should affect generic code...  You could do what CODA does,
I suppose; call ->mmap() of underlying file, then copy the resulting
->vm_ops into your private structure and override ->close() there
(keeping the original elsewhere in the same structure).  Then your
->close() would call the original and drop write access on the
ovl mount explicitly taken in your ->open().

*IF* we go that way, we probably ought to provide a ->get_path()
method for VMAs (NULL meaning "take ->vm_file->f_path") and use
that in procfs accesses.  That could reduce the impact on generic
code pretty much to zero - FMODE_BACKING included.

But it would cost you an allocation of vm_operations_struct per
mmap, most of them almost identical ;-/  And merging would not be
trivial - CODA stores a reference to original ->vm_file in
that structure, and uses container_of() to get to it in
their ->close().  AFAICS, there's no other safe place to stash
that information in anywhere in vm_area_struct.