In commit bb666b7c2707 ("mm: add mmap_prepare() compatibility layer for nested file systems") we introduced the ability for 'nested' drivers and file systems to correctly invoke the f_op->mmap_prepare() handler from an f_op->mmap() handler via a compatibility layer implemented in compat_vma_mmap_prepare(). This invokes vma_to_desc() to populate vm_area_desc fields according to those found in the (not yet fully initialised) VMA passed to f_op->mmap(). However this function implicitly assumes that the struct file which we are operating upon is equal to vma->vm_file. This is not a safe assumption in all cases. This is not an issue currently, as so far we have only implemented f_op->mmap_prepare() handlers for some file systems and internal mm uses, and the only nested f_op->mmap() operations that can be performed upon these are those in backing_file_mmap() and coda_file_mmap(), both of which use vma->vm_file. However, moving forward, as we convert drivers to using f_op->mmap_prepare(), this will become a problem. Resolve this issue by explicitly setting desc->file to the provided file parameter and update callers accordingly. We also need to adjust set_vma_from_desc() to account for this fact, and only update the vma->vm_file field if the f_op->mmap_prepare() caller reassigns it. We may in future wish to add a new field to struct vm_area_desc to account for this 'nested mmap invocation' case, but for now it seems unnecessary. While we are here, also provide a variant of compat_vma_mmap_prepare() that operates against a pointer to any file_operations struct and does not assume that the file_operations struct we are interested in is file->f_op. This function is __compat_vma_mmap_prepare() and we invoke it from compat_vma_mmap_prepare() so that we share code between the two functions. This is important, because some drivers provide hooks in a separate struct, for instance struct drm_device provides an fops field for this purpose. Also update the VMA selftests accordingly. Signed-off-by: Lorenzo Stoakes --- 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); if (vma->vm_flags != desc->vm_flags) vm_flags_set(vma, desc->vm_flags); diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h index 6f95ec14974f..4ceb4284b6b9 100644 --- a/tools/testing/vma/vma_internal.h +++ b/tools/testing/vma/vma_internal.h @@ -1411,25 +1411,30 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma) /* Declared in vma.h. */ 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); 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); -static int compat_vma_mmap_prepare(struct file *file, - struct vm_area_struct *vma) +static inline 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 = file->f_op->mmap_prepare(vma_to_desc(vma, &desc)); + err = f_op->mmap_prepare(vma_to_desc(vma, file, &desc)); if (err) return err; - set_vma_from_desc(vma, &desc); + set_vma_from_desc(vma, file, &desc); return 0; } +static inline int compat_vma_mmap_prepare(struct file *file, + struct vm_area_struct *vma) +{ + return __compat_vma_mmap_prepare(file->f_op, file, vma); +} + /* Did the driver provide valid mmap hook configuration? */ static inline bool can_mmap_file(struct file *file) { -- 2.50.1