| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <68e2ab97-7855-4c13-8241-46fff7164700@lucifer.local>
Date: Wed, 3 Sep 2025 15:52:21 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: "Liam R. Howlett" <Liam.Howlett@...cle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
David Hildenbrand <david@...hat.com>, Vlastimil Babka <vbabka@...e.cz>,
Mike Rapoport <rppt@...nel.org>,
Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
Jann Horn <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org
Subject: Re: [PATCH] mm: do not assume file == vma->vm_file in
compat_vma_mmap_prepare()
On Tue, Sep 02, 2025 at 11:08:10AM -0400, Liam R. Howlett wrote:
>
> One nit below.
>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@...cle.com>
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
Powered by blists - more mailing lists