On Sat, Dec 23, 2023 at 3:04 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> On Sat, Dec 23, 2023 at 08:56:08AM +0200, Amir Goldstein wrote:
> > On Sat, Dec 23, 2023 at 8:54 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > >
> > > On Fri, Dec 22, 2023 at 2:54 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > > >
> > > > On Thu, Dec 21, 2023 at 11:54:10AM +0200, Amir Goldstein wrote:
> > > > > Assert that the file object is allocated in a backing_file container
> > > > > so that file_user_path() could be used to display the user path and
> > > > > not the backing file's path in /proc/<pid>/maps.
> > > > >
> > > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > > > ---
> > > > > fs/backing-file.c | 27 +++++++++++++++++++++++++++
> > > > > fs/overlayfs/file.c | 23 ++++++-----------------
> > > > > include/linux/backing-file.h | 2 ++
> > > > > 3 files changed, 35 insertions(+), 17 deletions(-)
> > > > >
> > > > > diff --git a/fs/backing-file.c b/fs/backing-file.c
> > > > > index 46488de821a2..1ad8c252ec8d 100644
> > > > > --- a/fs/backing-file.c
> > > > > +++ b/fs/backing-file.c
> > > > > @@ -11,6 +11,7 @@
> > > > > #include <linux/fs.h>
> > > > > #include <linux/backing-file.h>
> > > > > #include <linux/splice.h>
> > > > > +#include <linux/mm.h>
> > > > >
> > > > > #include "internal.h"
> > > > >
> > > > > @@ -284,6 +285,32 @@ ssize_t backing_file_splice_write(struct pipe_inode_info *pipe,
> > > > > }
> > > > > EXPORT_SYMBOL_GPL(backing_file_splice_write);
> > > > >
> > > > > +int backing_file_mmap(struct file *file, struct vm_area_struct *vma,
> > > > > + struct backing_file_ctx *ctx)
> > > > > +{
> > > > > + const struct cred *old_cred;
> > > > > + int ret;
> > > > > +
> > > > > + if (WARN_ON_ONCE(!(file->f_mode & FMODE_BACKING)) ||
> > > >
> > > > Couldn't that WARN_ON_ONCE() be in every one of these helpers in this
> > > > series? IOW, when would you ever want to use a backing_file_*() helper
> > > > on a non-backing file?
> > >
> > > AFAIK, the call chain below backing_file_splice*() and backing_file_*_iter()
> > > helpers never end up accessing file_user_path() or assuming that fd of file
> > > is installed in fd table, so there is no strong reason to enforce this with an
> > > assertion.
>
> Yeah, but you do use an override_cred() call and you do have that
> backing_file_ctx. It smells like a bug if anyone would pass in a
> non-backing file.
>
> > >
> > > We can do it for clarity of semantics, in case one of the call chains will
> > > start assuming a struct backing_file in the future. WDIT?
> >
> > Doh! WDYT?
>
> I'd add it as the whole series is predicated on this being used for
> backing files.
Sure. Will do.
Thanks,
Amir.
