On Tue, Sep 02, 2025 at 11:08:10AM -0400, Liam R. Howlett wrote:
>
> One nit below.
>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
Thanks!
>
> > ---
> > include/linux/fs.h | 2 ++
> > mm/util.c | 33 +++++++++++++++++++++++---------
> > mm/vma.h | 14 ++++++++++----
> > tools/testing/vma/vma_internal.h | 19 +++++++++++-------
> > 4 files changed, 48 insertions(+), 20 deletions(-)
> >
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index d7ab4f96d705..3e7160415066 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2279,6 +2279,8 @@ static inline bool can_mmap_file(struct file *file)
> > return true;
> > }
> >
> > +int __compat_vma_mmap_prepare(const struct file_operations *f_op,
> > + struct file *file, struct vm_area_struct *vma);
> > int compat_vma_mmap_prepare(struct file *file, struct vm_area_struct *vma);
> >
> > static inline int vfs_mmap(struct file *file, struct vm_area_struct *vma)
> > diff --git a/mm/util.c b/mm/util.c
> > index bb4b47cd6709..83fe15e4483a 100644
> > --- a/mm/util.c
> > +++ b/mm/util.c
> > @@ -1133,6 +1133,29 @@ void flush_dcache_folio(struct folio *folio)
> > EXPORT_SYMBOL(flush_dcache_folio);
> > #endif
> >
> > +/**
> > + * __compat_vma_mmap_prepare() - See description for compat_vma_mmap_prepare()
> > + * for details. This is the same operation, only with a specific file operations
> > + * struct which may or may not be the same as vma->vm_file->f_op.
> > + * @f_op - The file operations whose .mmap_prepare() hook is specified.
> > + * @vma: The VMA to apply the .mmap_prepare() hook to.
> > + * Returns: 0 on success or error.
> > + */
> > +int __compat_vma_mmap_prepare(const struct file_operations *f_op,
> > + struct file *file, struct vm_area_struct *vma)
> > +{
> > + struct vm_area_desc desc;
> > + int err;
> > +
> > + err = f_op->mmap_prepare(vma_to_desc(vma, file, &desc));
> > + if (err)
> > + return err;
> > + set_vma_from_desc(vma, file, &desc);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(__compat_vma_mmap_prepare);
> > +
> > /**
> > * compat_vma_mmap_prepare() - Apply the file's .mmap_prepare() hook to an
> > * existing VMA
> > @@ -1161,15 +1184,7 @@ EXPORT_SYMBOL(flush_dcache_folio);
> > */
> > int compat_vma_mmap_prepare(struct file *file, struct vm_area_struct *vma)
> > {
> > - struct vm_area_desc desc;
> > - int err;
> > -
> > - err = file->f_op->mmap_prepare(vma_to_desc(vma, &desc));
> > - if (err)
> > - return err;
> > - set_vma_from_desc(vma, &desc);
> > -
> > - return 0;
> > + return __compat_vma_mmap_prepare(file->f_op, file, vma);
> > }
> > EXPORT_SYMBOL(compat_vma_mmap_prepare);
> >
> > diff --git a/mm/vma.h b/mm/vma.h
> > index bcdc261c5b15..9b21d47ba630 100644
> > --- a/mm/vma.h
> > +++ b/mm/vma.h
> > @@ -230,14 +230,14 @@ static inline int vma_iter_store_gfp(struct vma_iterator *vmi,
> > */
> >
> > static inline struct vm_area_desc *vma_to_desc(struct vm_area_struct *vma,
> > - struct vm_area_desc *desc)
> > + struct file *file, struct vm_area_desc *desc)
> > {
> > desc->mm = vma->vm_mm;
> > desc->start = vma->vm_start;
> > desc->end = vma->vm_end;
> >
> > desc->pgoff = vma->vm_pgoff;
> > - desc->file = vma->vm_file;
> > + desc->file = file;
> > desc->vm_flags = vma->vm_flags;
> > desc->page_prot = vma->vm_page_prot;
> >
> > @@ -248,7 +248,7 @@ static inline struct vm_area_desc *vma_to_desc(struct vm_area_struct *vma,
> > }
> >
> > static inline void set_vma_from_desc(struct vm_area_struct *vma,
> > - struct vm_area_desc *desc)
> > + struct file *orig_file, struct vm_area_desc *desc)
> > {
> > /*
> > * Since we're invoking .mmap_prepare() despite having a partially
> > @@ -258,7 +258,13 @@ static inline void set_vma_from_desc(struct vm_area_struct *vma,
> >
> > /* Mutable fields. Populated with initial state. */
> > vma->vm_pgoff = desc->pgoff;
> > - if (vma->vm_file != desc->file)
> > + /*
> > + * The desc->file may not be the same as vma->vm_file, but if the
> > + * f_op->mmap_prepare() handler is setting this parameter to something
> > + * different, it indicates that it wishes the VMA to have its file
> > + * assigned to this.
> > + */
> > + if (orig_file != desc->file && vma->vm_file != desc->file)
> > vma_set_file(vma, desc->file);
>
> So now we have to be sure both orig_file and vma->vm_file != desc->file
> to set it? This seems to make the function name less accurate.
I'll update the comment accordingly.
On this in general - In future an mmap_prepare() caller may wish to change
the file to desc->file from vma->vm_file which currently won't work for a
stacked file system.
It's pretty niche and unlikely anybody does it, but if they do, since I am
the one implementing all this I will adjust the descriptor to send a
separate file parameter and adjust this code accordingly.
Cheers, Lorenzo
