mm: expand mmap_prepare functionality and usage
Subsystems (20)
Lorenzo Stoakes (Oracle)
maintainerv3 cover mm: expand mmap_prepare functionality and usage
This series expands the mmap_prepare functionality, which is intended to replace the deprecated f_op->mmap hook which has been the source of bugs and security issues for some time.
This series starts with some cleanup of existing mmap_prepare logic, then adds documentation for the mmap_prepare call to make it easier for filesystem and driver writers to understand how it works.
It then importantly adds a vm_ops->mapped hook, a key feature that was missing from mmap_prepare previously - this is invoked when a driver which specifies mmap_prepare has successfully been mapped but not merged with another VMA.
mmap_prepare is invoked prior to a merge being attempted, so you cannot manipulate state such as reference counts as if it were a new mapping.
The vm_ops->mapped hook allows a driver to perform tasks required at this stage, and provides symmetry against subsequent vm_ops->open,close calls.
The series uses this to correct the afs implementation which wrongly manipulated reference count at mmap_prepare time.
It then adds an mmap_prepare equivalent of vm_iomap_memory() - mmap_action_simple_ioremap(), then uses this to update a number of drivers.
It then splits out the mmap_prepare compatibility layer (which allows for invocation of mmap_prepare hooks in an mmap() hook) in such a way as to allow for more incremental implementation of mmap_prepare hooks.
It then uses this to extend mmap_prepare usage in drivers.
Finally it adds an mmap_prepare equivalent of vm_map_pages(), which lays the foundation for future work which will extend mmap_prepare to DMA coherent mappings.
v3:
* Propagated tags (thanks Suren, Richard!)
* Updated 12/16 to correctly clear the vm_area_desc data structure in set_desc_from_vma() as per Joshua Hahn (thanks! :)
* Fixed type in 12/16 as per Suren (cheers!)
* Fixed up 6/16 to use mmap_action_ioremap_full() in simple_ioremap_prepare() as suggested by Suren.
* Also fixed up 6/16 to call io_remap_pfn_range_prepare() direct rather than mmap_action_prepare() as per Suren.
* Also fixed up 6/16 to pass vm_len rather than vm_[start, end] to __simple_ioremap_prep() as per Suren (thanks for all the above! :)
* Fixed issue in rmap lock being held - we were referencing a vma->vm_file after the VMA was unmapped, so UAF. Avoid that. Also do_munmap() relies on rmap lock NOT being held or may deadlock, so extend functionality to ensure we drop it when it is held on error paths.
* Updated 'area' -> 'vma' variable in 3/16 in VMA test dup.h.
* Fixed up reference to __compat_vma_mmap() in 12/16 commit message.
* Updated 1/16 to no longer duplicatively apply io_remap_pfn_range_pfn().
* Updated 1/16 to delegate I/O remap complete to remap complete logic.
* Fixed various typos in 12/16.
* Fixed stale comment typos in 13/16.
* Fixed commit msg and comment typos in 14/16.
* Removed accidental sneak peak to future functionality in 15/16 commit message :).
* Fixed up field names to be identical in VMA tests + mm_types.h in 6/16, 15/16.
v2:
* Rebased on
https://lore.kernel.org/all/[email protected]/ to make Andrew's life easier :)
* Folded all interim fixes into series (thanks Randy for many doc fixes!))
* As per Suren, removed a comment about allocations too small to fail.
* As per Randy, fixed up typo in documentation for vm_area_desc.
* Fixed mmap_action_prepare() not returning if invalid action->type specified, as updated from Andrew's interim fix (thanks!) and also reported by kernel test bot.
* Updated mmap_action_prepare() and specific prepare functions to only pass vm_area_desc parameter as per Suren.
* Fixed up whitespace as per Suren.
* Updated vm_op->open comment in vm_operations_struct to reference forking as per Suren.
* Added a commit to check that input range is within VMA on remap as per Suren (this also covers I/O remap and all other cases already asserted).
* Updated AFS to not incorrectly reference count on mmap prepare as per Usama.
* Also updated various static AFS functions to be consistent with each other.
* Updated AFS commit message to reflect mmap_prepare being before any VMA merging as per Suren.
* Updated __compat_vma_mapped() to check for NULL vm_ops as per Usama.
* Updated __compat_vma_mapped() to not reference an unmapped VMA's fields as per Usama.
* Updated __vma_check_mmap_hook() to check for NULL vm_ops as per Usama.
* Dropped comment about preferring mmap_prepare as seems overly confusing, as per Suren.
* Updated the mmap lock assert in unmap_vma_locked() to a write lock assert as per Suren.
* Copied vm_ops->open comment over to VMA tests in appropriate patch as per Suren.
* Updated mmap_prepare documentation to reflect the fact that no resources should be allocated upon mmap_prepare.
* Updated mmap_prepare documentation to reference the vm_ops->mapped callback.
* Fixed stray markdown '## How to use' in documentation.
* Fixed bug reported by kernel test bot re: overlooked vma_desc_test_flags() -> vma_desc_test() in MTD driver for nommu.
https://lore.kernel.org/linux-mm/[email protected]/
v1:
https://lore.kernel.org/linux-mm/[email protected]/
Lorenzo Stoakes (Oracle) (16):
mm: various small mmap_prepare cleanups
mm: add documentation for the mmap_prepare file operation callback
mm: document vm_operations_struct->open the same as close()
mm: add vm_ops->mapped hook
fs: afs: correctly drop reference count on mapping failure
mm: add mmap_action_simple_ioremap()
misc: open-dice: replace deprecated mmap hook with mmap_prepare
hpet: replace deprecated mmap hook with mmap_prepare
mtdchar: replace deprecated mmap hook with mmap_prepare, clean up
stm: replace deprecated mmap hook with mmap_prepare
staging: vme_user: replace deprecated mmap hook with mmap_prepare
mm: allow handling of stacked mmap_prepare hooks in more drivers
drivers: hv: vmbus: replace deprecated mmap hook with mmap_prepare
uio: replace deprecated mmap hook with mmap_prepare in uio_info
mm: add mmap_action_map_kernel_pages[_full]()
mm: on remap assert that input range within the proposed VMA
Documentation/driver-api/vme.rst | 2 +- Documentation/filesystems/index.rst | 1 + Documentation/filesystems/mmap_prepare.rst | 168 ++++++++++++++ drivers/char/hpet.c | 12 +- drivers/hv/hyperv_vmbus.h | 4 +- drivers/hv/vmbus_drv.c | 31 ++- drivers/hwtracing/stm/core.c | 31 ++- drivers/misc/open-dice.c | 19 +- drivers/mtd/mtdchar.c | 21 +- drivers/staging/vme_user/vme.c | 20 +- drivers/staging/vme_user/vme.h | 2 +- drivers/staging/vme_user/vme_user.c | 51 +++-- drivers/target/target_core_user.c | 26 ++- drivers/uio/uio.c | 10 +- drivers/uio/uio_hv_generic.c | 11 +- fs/afs/file.c | 36 ++- include/linux/fs.h | 14 +- include/linux/hyperv.h | 4 +- include/linux/mm.h | 159 ++++++++++++- include/linux/mm_types.h | 17 +- include/linux/uio_driver.h | 4 +- mm/internal.h | 41 ++-- mm/memory.c | 175 ++++++++++---- mm/util.c | 251 ++++++++++++++------- mm/vma.c | 53 +++-- mm/vma.h | 2 +- tools/testing/vma/include/dup.h | 152 ++++++++++--- tools/testing/vma/include/stubs.h | 9 +- 28 files changed, 990 insertions(+), 336 deletions(-) create mode 100644 Documentation/filesystems/mmap_prepare.rst
--
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 1/16 mm: various small mmap_prepare cleanups
Rather than passing arbitrary fields, pass a vm_area_desc pointer to mmap prepare functions to mmap prepare, and an action and vma pointer to mmap complete in order to put all the action-specific logic in the function actually doing the work.
Additionally, allow mmap prepare functions to return an error so we can error out as soon as possible if there is something logically incorrect in the input.
Update remap_pfn_range_prepare() to properly check the input range for the CoW case.
Also remove io_remap_pfn_range_complete(), as we can simply set up the fields correctly in io_remap_pfn_range_prepare() and use remap_pfn_range_complete() for this.
While we're here, make remap_pfn_range_prepare_vma() a little neater, and pass mmap_action directly to call_action_complete().
Then, update compat_vma_mmap() to perform its logic directly, as __compat_vma_map() is not used by anything so we don't need to export it.
Also update compat_vma_mmap() to use vfs_mmap_prepare() rather than calling the mmap_prepare op directly.
Finally, update the VMA userland tests to reflect the changes.
--- include/linux/fs.h | 2 - include/linux/mm.h | 7 +- mm/internal.h | 32 ++++---- mm/memory.c | 45 +++++++---- mm/util.c | 121 +++++++++++++----------------- mm/vma.c | 24 +++--- tools/testing/vma/include/dup.h | 7 +- tools/testing/vma/include/stubs.h | 8 +- 8 files changed, 126 insertions(+), 120 deletions(-)
include/linux/fs.h +0 -2
@@ -2058,8 +2058,6 @@ static inline bool can_mmap_file(struct file *file)return true; } -int __compat_vma_mmap (const struct file_operations *f_op, -struct file *file,struct vm_area_struct *vma);int compat_vma_mmap (struct file *file,struct vm_area_struct *vma);static inline int vfs_mmap (struct file *file,struct vm_area_struct *vma)
include/linux/mm.h +3 -4
@@ -4320,10 +4320,9 @@ static inline void mmap_action_ioremap_full(struct vm_area_desc *desc,mmap_action_ioremap (desc, desc->start, start_pfn,vma_desc_size (desc)); } -void mmap_action_prepare (struct mmap_action *action, -struct vm_area_desc *desc); -int mmap_action_complete (struct mmap_action *action, -struct vm_area_struct *vma); +int mmap_action_prepare (struct vm_area_desc *desc); +int mmap_action_complete (struct vm_area_struct *vma, +struct mmap_action *action);/* Look up the first VMA which exactly match the interval vm_start ... vm_end */ static inline struct vm_area_struct *find_exact_vma(struct mm_struct *mm,
mm/internal.h +17 -15
@@ -1793,26 +1793,28 @@ int walk_page_range_debug(struct mm_struct *mm, unsigned long start,void dup_mm_exe_file (struct mm_struct *mm,struct mm_struct *oldmm);int dup_mmap (struct mm_struct *mm,struct mm_struct *oldmm); -void remap_pfn_range_prepare (struct vm_area_desc *desc,unsigned long pfn); -int remap_pfn_range_complete (struct vm_area_struct *vma,unsigned long addr, -unsigned long pfn,unsigned long size,pgprot_t pgprot); +int remap_pfn_range_prepare (struct vm_area_desc *desc); +int remap_pfn_range_complete (struct vm_area_struct *vma, +struct mmap_action *action); -static inline void io_remap_pfn_range_prepare (struct vm_area_desc *desc, -unsigned long orig_pfn,unsigned long size) +static inline int io_remap_pfn_range_prepare (struct vm_area_desc *desc) { +struct mmap_action *action = &desc->action; +const unsigned long orig_pfn = action->remap.start_pfn; +const pgprot_t orig_pgprot = action->remap.pgprot; +const unsigned long size = action->remap.size;const unsigned long pfn =io_remap_pfn_range_pfn (orig_pfn, size); +int err; -return remap_pfn_range_prepare (desc, pfn); -} + action->remap.start_pfn = pfn; + action->remap.pgprot =pgprot_decrypted (orig_pgprot); + err =remap_pfn_range_prepare (desc); +if (err) +return err; -static inline int io_remap_pfn_range_complete (struct vm_area_struct *vma, -unsigned long addr,unsigned long orig_pfn,unsigned long size, -pgprot_t orig_prot) -{ -const unsigned long pfn =io_remap_pfn_range_pfn (orig_pfn, size); -const pgprot_t prot =pgprot_decrypted (orig_prot); - -return remap_pfn_range_complete (vma, addr, pfn, size, prot); +/* Remap does the actual work. */ + action->type =MMAP_REMAP_PFN ; +return 0 ; }#ifdef CONFIG_MMU_NOTIFIER
mm/memory.c +29 -16
65 lines not on this page · raw message
mm/util.c +54 -67
192 lines not on this page · raw message
mm/vma.c +14 -10
@@ -2640,15 +2640,18 @@ static void __mmap_complete(struct mmap_state *map, struct vm_area_struct *vma)vma_set_page_prot (vma); } -static void call_action_prepare (struct mmap_state *map, -struct vm_area_desc *desc) +static int call_action_prepare (struct mmap_state *map, +struct vm_area_desc *desc) { -struct mmap_action *action = &desc->action; +int err; -mmap_action_prepare (action, desc); + err =mmap_action_prepare (desc); +if (err) +return err; -if (action->hide_from_rmap_until_complete) +if (desc->action.hide_from_rmap_until_complete) map->hold_file_rmap_lock = true; +return 0 ; }/*
@@ -2672,7 +2675,9 @@ static int call_mmap_prepare(struct mmap_state *map,if (err)return err; -call_action_prepare (map, desc); + err =call_action_prepare (map, desc); +if (err) +return err;/* Update fields permitted to be changed. */ map->pgoff = desc->pgoff;
@@ -2727,13 +2732,12 @@ static bool can_set_ksm_flags_early(struct mmap_state *map) }static int call_action_complete (struct mmap_state *map, -struct vm_area_desc *desc, +struct mmap_action *action,struct vm_area_struct *vma) { -struct mmap_action *action = &desc->action;int ret; - ret =mmap_action_complete (action, vma); + ret =mmap_action_complete (vma, action);/* If we held the file rmap we need to release it. */ if (map->hold_file_rmap_lock) {
@@ -2795,7 +2799,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,__mmap_complete (&map, vma);if (have_mmap_prepare && allocated_new) { - error =call_action_complete (&map, &desc, vma); + error =call_action_complete (&map, &desc.action, vma);if (error)return error;
tools/testing/vma/include/dup.h +5 -2
@@ -1271,9 +1271,12 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,if (err)return err; -mmap_action_prepare (&desc.action, &desc); + err =mmap_action_prepare (&desc); +if (err) +return err; +set_vma_from_desc (vma, &desc); -return mmap_action_complete (&desc.action, vma); +return mmap_action_complete (vma, &desc.action); } static inline int compat_vma_mmap(struct file *file,
tools/testing/vma/include/stubs.h +4 -4
@@ -81,13 +81,13 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma) { } -static inline void mmap_action_prepare (struct mmap_action *action, -struct vm_area_desc *desc) +static inline int mmap_action_prepare (struct vm_area_desc *desc) { +return 0 ; } -static inline int mmap_action_complete (struct mmap_action *action, -struct vm_area_struct *vma) +static inline int mmap_action_complete (struct vm_area_struct *vma, +struct mmap_action *action) {return 0 ; }
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 2/16 mm: add documentation for the mmap_prepare file operation callback
In reply to Lorenzo Stoakes (Oracle)This documentation makes it easier for a driver/file system implementer to correctly use this callback.
It covers the fundamentals, whilst intentionally leaving the less lovely possible actions one might take undocumented (for instance - the success_hook, error_hook fields in mmap_action).
The document also covers the new VMA flags implementation which is the only one which will work correctly with mmap_prepare.
--- Documentation/filesystems/index.rst | 1 + Documentation/filesystems/mmap_prepare.rst | 142 +++++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 Documentation/filesystems/mmap_prepare.rst
Documentation/filesystems/index.rst +1 -0
@@ -29,6 +29,7 @@ algorithms work. fiemap files locks + mmap_prepare multigrain-ts mount_api quota
Documentation/filesystems/mmap_prepare.rst new file +142 -0
142 lines not on this page · raw message
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 3/16 mm: document vm_operations_struct->open the same as close()
In reply to Lorenzo Stoakes (Oracle)Describe when the operation is invoked and the context in which it is invoked, matching the description already added for vm_op->close().
While we're here, update all outdated references to an 'area' field for VMAs to the more consistent 'vma'.
--- include/linux/mm.h | 15 ++++++++++----- tools/testing/vma/include/dup.h | 15 ++++++++++----- 2 files changed, 20 insertions(+), 10 deletions(-)
include/linux/mm.h +10 -5
@@ -766,15 +766,20 @@ struct vm_uffd_ops;* to the functions called when a no-page or a wp-page exception occurs. */ struct vm_operations_struct { -void (*open)(struct vm_area_struct * area); +/** +* @open: Called when a VMA is remapped, split or forked. Not called +* upon first mapping a VMA. +* Context: User context. May sleep. Caller holds mmap_lock. +*/ +void (*open)(struct vm_area_struct *vma);/** * @close: Called when the VMA is being removed from the MM. * Context: User context. May sleep. Caller holds mmap_lock. */ -void (*close)(struct vm_area_struct * area); +void (*close)(struct vm_area_struct *vma);/* Called any time before splitting to check if it's allowed */ -int (*may_split)(struct vm_area_struct *area,unsigned long addr); -int (*mremap)(struct vm_area_struct *area); +int (*may_split)(struct vm_area_struct *vma,unsigned long addr); +int (*mremap)(struct vm_area_struct *vma);/* * Called by mprotect() to make driver-specific permission * checks before mprotect() is finalised. The VMA must not
@@ -786,7 +791,7 @@ struct vm_operations_struct {vm_fault_t (*huge_fault)(struct vm_fault *vmf,unsigned int order);vm_fault_t (*map_pages)(struct vm_fault *vmf,pgoff_t start_pgoff,pgoff_t end_pgoff); -unsigned long (*pagesize)(struct vm_area_struct * area); +unsigned long (*pagesize)(struct vm_area_struct *vma);/* notification that a previously read-only page is about to become * writable, if an error is returned it will cause a SIGBUS */
tools/testing/vma/include/dup.h +10 -5
@@ -632,15 +632,20 @@ struct vm_area_struct { } __randomize_layout;struct vm_operations_struct { -void (*open)(struct vm_area_struct * area); +/** +* @open: Called when a VMA is remapped, split or forked. Not called +* upon first mapping a VMA. +* Context: User context. May sleep. Caller holds mmap_lock. +*/ +void (*open)(struct vm_area_struct *vma);/** * @close: Called when the VMA is being removed from the MM. * Context: User context. May sleep. Caller holds mmap_lock. */ -void (*close)(struct vm_area_struct * area); +void (*close)(struct vm_area_struct *vma);/* Called any time before splitting to check if it's allowed */ -int (*may_split)(struct vm_area_struct *area,unsigned long addr); -int (*mremap)(struct vm_area_struct *area); +int (*may_split)(struct vm_area_struct *vma,unsigned long addr); +int (*mremap)(struct vm_area_struct *vma);/* * Called by mprotect() to make driver-specific permission * checks before mprotect() is finalised. The VMA must not
@@ -652,7 +657,7 @@ struct vm_operations_struct {vm_fault_t (*huge_fault)(struct vm_fault *vmf,unsigned int order);vm_fault_t (*map_pages)(struct vm_fault *vmf,pgoff_t start_pgoff,pgoff_t end_pgoff); -unsigned long (*pagesize)(struct vm_area_struct * area); +unsigned long (*pagesize)(struct vm_area_struct *vma);/* notification that a previously read-only page is about to become * writable, if an error is returned it will cause a SIGBUS */
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 4/16 mm: add vm_ops->mapped hook
In reply to Lorenzo Stoakes (Oracle)Previously, when a driver needed to do something like establish a reference count, it could do so in the mmap hook in the knowledge that the mapping would succeed.
With the introduction of f_op->mmap_prepare this is no longer the case, as it is invoked prior to actually establishing the mapping.
mmap_prepare is not appropriate for this kind of thing as it is called before any merge might take place, and after which an error might occur meaning resources could be leaked.
To take this into account, introduce a new vm_ops->mapped callback which is invoked when the VMA is first mapped (though notably - not when it is merged - which is correct and mirrors existing mmap/open/close behaviour).
We do better that vm_ops->open() here, as this callback can return an error, at which point the VMA will be unmapped.
Note that vm_ops->mapped() is invoked after any mmap action is complete (such as I/O remapping).
We intentionally do not expose the VMA at this point, exposing only the fields that could be used, and an output parameter in case the operation needs to update the vma->vm_private_data field.
In order to deal with stacked filesystems which invoke inner filesystem's mmap() invocations, add __compat_vma_mapped() and invoke it on vfs_mmap() (via compat_vma_mmap()) to ensure that the mapped callback is handled when an mmap() caller invokes a nested filesystem's mmap_prepare() callback.
We can now also remove call_action_complete() and invoke mmap_action_complete() directly, as we separate out the rmap lock logic.
The rmap lock logic, which was added in order to keep hugetlb working (!) to allow for the rmap lock to be held longer, needs to be propagated to the error paths on mmap complete and mapped hook error paths.
This is because do_munmap() might otherwise deadlock with the rmap being held, so instead we unlock at the point of unmap.
This is fine as any reliance on the rmap being held is irrelevant on error.
While we're here, refactor mmap_action_finish() to avoid a big if (err) branch.
We also abstract unmapping of a VMA on mmap action completion into its own helper function, unmap_vma_locked().
Update the mmap_prepare documentation to describe the mapped hook and make it clear what its intended use is.
Additionally, update VMA userland test headers to reflect the change.
--- Documentation/filesystems/mmap_prepare.rst | 15 +++ include/linux/fs.h | 9 +- include/linux/mm.h | 20 +++- mm/internal.h | 8 ++ mm/util.c | 129 ++++++++++++++------- mm/vma.c | 35 +++--- tools/testing/vma/include/dup.h | 27 ++++- tools/testing/vma/include/stubs.h | 3 +- 8 files changed, 186 insertions(+), 60 deletions(-)
Documentation/filesystems/mmap_prepare.rst +15 -0
@@ -25,6 +25,21 @@ That is - no resources should be allocated nor state updated to reflect that a mapping has been established, as the mapping may either be merged, or fail to be mapped after the callback is complete. +Mapped callback +--------------- + +If resources need to be allocated per-mapping, or state such as a reference +count needs to be manipulated, this should be done using the ``vm_ops->mapped`` +hook, which itself should be set by the >mmap_prepare hook. + +This callback is only invoked if a new mapping has been established and was not +merged with any other, and is invoked at a point where no error may occur before +the mapping is established. + +You may return an error to the callback itself, which will cause the mapping to +become unmapped and an error returned to the mmap() caller. This is useful if +resources need to be allocated, and that allocation might fail. + How To Use ==========
include/linux/fs.h +8 -1
@@ -2059,13 +2059,20 @@ static inline bool can_mmap_file(struct file *file) }int compat_vma_mmap (struct file *file,struct vm_area_struct *vma); +int __vma_check_mmap_hook (struct vm_area_struct *vma);static inline int vfs_mmap (struct file *file,struct vm_area_struct *vma) { +int err; +if (file->f_op->mmap_prepare)return compat_vma_mmap (file, vma); -return file->f_op->mmap (file, vma); + err = file->f_op->mmap (file, vma); +if (err) +return err; + +return __vma_check_mmap_hook (vma); }static inline int vfs_mmap_prepare (struct file *file,struct vm_area_desc *desc)
include/linux/mm.h +19 -1
@@ -777,6 +777,23 @@ struct vm_operations_struct {* Context: User context. May sleep. Caller holds mmap_lock. */ void (*close)(struct vm_area_struct *vma); +/** +* @mapped: Called when the VMA is first mapped in the MM. Not called if +* the new VMA is merged with an adjacent VMA. +* +* The @vm_private_data field is an output field allowing the user to +* modify vma->vm_private_data as necessary. +* +* ONLY valid if set from f_op->mmap_prepare. Will result in an error if +* set from f_op->mmap. +* +* Returns %0 on success, or an error otherwise. On error, the VMA will +* be unmapped. +* +* Context: User context. May sleep. Caller holds mmap_lock. +*/ +int (*mapped)(unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data);/* Called any time before splitting to check if it's allowed */ int (*may_split)(struct vm_area_struct *vma,unsigned long addr);int (*mremap)(struct vm_area_struct *vma);
@@ -4327,7 +4344,8 @@ static inline void mmap_action_ioremap_full(struct vm_area_desc *desc,int mmap_action_prepare (struct vm_area_desc *desc);int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action); +struct mmap_action *action, +bool rmap_lock_held);/* Look up the first VMA which exactly match the interval vm_start ... vm_end */ static inline struct vm_area_struct *find_exact_vma(struct mm_struct *mm,
mm/internal.h +8 -0
@@ -202,6 +202,14 @@ static inline void vma_close(struct vm_area_struct *vma)/* unmap_vmas is in mm/memory.c */ void unmap_vmas (struct mmu_gather *tlb,struct unmap_desc *unmap); +static inline void unmap_vma_locked (struct vm_area_struct *vma) +{ +const size_t len =vma_pages (vma) <<PAGE_SHIFT ; + +mmap_assert_write_locked (vma->vm_mm); +do_munmap (vma->vm_mm, vma->vm_start, len,NULL ); +} +#ifdef CONFIG_MMU static inline void get_anon_vma (struct anon_vma *anon_vma)
mm/util.c +86 -43
192 lines not on this page · raw message
mm/vma.c +22 -13
@@ -2731,21 +2731,28 @@ static bool can_set_ksm_flags_early(struct mmap_state *map)return false; } -static int call_action_complete (struct mmap_state *map, -struct mmap_action *action, -struct vm_area_struct *vma) +static int call_mapped_hook (struct mmap_state *map, +struct vm_area_struct *vma) { -int ret; - - ret =mmap_action_complete (vma, action); +const struct vm_operations_struct *vm_ops = vma->vm_ops; +void *vm_private_data = vma->vm_private_data; +int err; -/* If we held the file rmap we need to release it. */ -if (map->hold_file_rmap_lock) { -struct file *file = vma->vm_file; +if (!vm_ops || !vm_ops->mapped) +return 0 ; + err = vm_ops->mapped (vma->vm_start, vma->vm_end, vma->vm_pgoff, + vma->vm_file, &vm_private_data); +if (err) { +if (map->hold_file_rmap_lock) +i_mmap_unlock_write (vma->vm_file->f_mapping); -i_mmap_unlock_write (file->f_mapping); +unmap_vma_locked (vma); +return err; } -return ret; +/* Update private data if changed. */ +if (vm_private_data != vma->vm_private_data) + vma->vm_private_data = vm_private_data; +return 0 ; } static unsigned long __mmap_region(struct file *file, unsigned long addr,
@@ -2799,8 +2806,10 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,__mmap_complete (&map, vma);if (have_mmap_prepare && allocated_new) { - error =call_action_complete (&map, &desc.action, vma); - + error =mmap_action_complete (vma, &desc.action, + map.hold_file_rmap_lock); +if (!error) + error =call_mapped_hook (&map, vma);if (error)return error; }
tools/testing/vma/include/dup.h +26 -1
@@ -643,6 +643,23 @@ struct vm_operations_struct {* Context: User context. May sleep. Caller holds mmap_lock. */ void (*close)(struct vm_area_struct *vma); +/** +* @mapped: Called when the VMA is first mapped in the MM. Not called if +* the new VMA is merged with an adjacent VMA. +* +* The @vm_private_data field is an output field allowing the user to +* modify vma->vm_private_data as necessary. +* +* ONLY valid if set from f_op->mmap_prepare. Will result in an error if +* set from f_op->mmap. +* +* Returns %0 on success, or an error otherwise. On error, the VMA will +* be unmapped. +* +* Context: User context. May sleep. Caller holds mmap_lock. +*/ +int (*mapped)(unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data);/* Called any time before splitting to check if it's allowed */ int (*may_split)(struct vm_area_struct *vma,unsigned long addr);int (*mremap)(struct vm_area_struct *vma);
@@ -1281,7 +1298,7 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,return err;set_vma_from_desc (vma, &desc); -return mmap_action_complete (vma, &desc.action); +return mmap_action_complete (vma, &desc.action,/*rmap_lock_held=*/ false); } static inline int compat_vma_mmap(struct file *file,
@@ -1500,3 +1517,11 @@ static inline pgprot_t vma_get_page_prot(vma_flags_t vma_flags)return vm_get_page_prot (vm_flags); } + +static inline void unmap_vma_locked (struct vm_area_struct *vma) +{ +const size_t len =vma_pages (vma) <<PAGE_SHIFT ; + +mmap_assert_write_locked (vma->vm_mm); +do_munmap (vma->vm_mm, vma->vm_start, len,NULL ); +}
tools/testing/vma/include/stubs.h +2 -1
@@ -87,7 +87,8 @@ static inline int mmap_action_prepare(struct vm_area_desc *desc) }static inline int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action) +struct mmap_action *action, +bool rmap_lock_held) {return 0 ; }
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 5/16 fs: afs: correctly drop reference count on mapping failure
In reply to Lorenzo Stoakes (Oracle)Commit 9d5403b1036c ("fs: convert most other generic_file_*mmap() users to .mmap_prepare()") updated AFS to use the mmap_prepare callback in favour of the deprecated mmap callback.
However, it did not account for the fact that mmap_prepare is called pre-merge, and may then be merged, nor that mmap_prepare can fail to map due to an out of memory error.
Both of those are cases in which we should not be incrementing a reference count.
With the newly added vm_ops->mapped callback available, we can simply defer this operation to that callback which is only invoked once the mapping is successfully in place (but not yet visible to userspace as the mmap and VMA write locks are held).
Therefore add afs_mapped() to implement this callback for AFS, and remove the code doing so in afs_mmap_prepare().
Also update afs_vm_open(), afs_vm_close() and afs_vm_map_pages() to be consistent in how the vnode is accessed.
--- fs/afs/file.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-)
fs/afs/file.c +26 -10
@@ -28,6 +28,8 @@ static ssize_t afs_file_splice_read(struct file *in, loff_t *ppos,static void afs_vm_open (struct vm_area_struct *area);static void afs_vm_close (struct vm_area_struct *area);static vm_fault_t afs_vm_map_pages (struct vm_fault *vmf,pgoff_t start_pgoff,pgoff_t end_pgoff); +static int afs_mapped (unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data);const struct file_operations afs_file_operations = { .open = afs_open,
@@ -61,6 +63,7 @@ const struct address_space_operations afs_file_aops = { };static const struct vm_operations_struct afs_vm_ops = { + .mapped = afs_mapped, .open = afs_vm_open, .close = afs_vm_close, .fault = filemap_fault,
@@ -494,32 +497,45 @@ static void afs_drop_open_mmap(struct afs_vnode *vnode)*/ static int afs_file_mmap_prepare (struct vm_area_desc *desc) { -struct afs_vnode *vnode =AFS_FS_I (file_inode (desc->file));int ret; -afs_add_open_mmap (vnode); - ret =generic_file_mmap_prepare (desc); -if (ret ==0 ) - desc->vm_ops = &afs_vm_ops; -else -afs_drop_open_mmap (vnode); +if (ret) +return ret; + + desc->vm_ops = &afs_vm_ops;return ret; } +static int afs_mapped (unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data) +{ +struct afs_vnode *vnode =AFS_FS_I (file_inode (file)); + +afs_add_open_mmap (vnode); +return 0 ; +} +static void afs_vm_open (struct vm_area_struct *vma) { -afs_add_open_mmap (AFS_FS_I (file_inode (vma->vm_file))); +struct file *file = vma->vm_file; +struct afs_vnode *vnode =AFS_FS_I (file_inode (file)); + +afs_add_open_mmap (vnode); }static void afs_vm_close (struct vm_area_struct *vma) { -afs_drop_open_mmap (AFS_FS_I (file_inode (vma->vm_file))); +struct file *file = vma->vm_file; +struct afs_vnode *vnode =AFS_FS_I (file_inode (file)); + +afs_drop_open_mmap (vnode); }static vm_fault_t afs_vm_map_pages (struct vm_fault *vmf,pgoff_t start_pgoff,pgoff_t end_pgoff) { -struct afs_vnode *vnode =AFS_FS_I (file_inode (vmf->vma->vm_file)); +struct file *file = vmf->vma->vm_file; +struct afs_vnode *vnode =AFS_FS_I (file_inode (file));if (afs_check_validity (vnode))return filemap_map_pages (vmf, start_pgoff, end_pgoff);
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 6/16 mm: add mmap_action_simple_ioremap()
In reply to Lorenzo Stoakes (Oracle)Currently drivers use vm_iomap_memory() as a simple helper function for I/O remapping memory over a range starting at a specified physical address over a specified length.
In order to utilise this from mmap_prepare, separate out the core logic into __simple_ioremap_prep(), update vm_iomap_memory() to use it, and add simple_ioremap_prepare() to do the same with a VMA descriptor object.
We also add MMAP_SIMPLE_IO_REMAP and relevant fields to the struct mmap_action type to permit this operation also.
We use mmap_action_ioremap() to set up the actual I/O remap operation once we have checked and figured out the parameters, which makes simple_ioremap_prepare() easy to implement.
We then add mmap_action_simple_ioremap() to allow drivers to make use of this mode.
We update the mmap_prepare documentation to describe this mode. Finally, we update the VMA tests to reflect this change.
--- Documentation/filesystems/mmap_prepare.rst | 3 + include/linux/mm.h | 24 +++++- include/linux/mm_types.h | 6 +- mm/internal.h | 1 + mm/memory.c | 85 +++++++++++++++------- mm/util.c | 5 ++ tools/testing/vma/include/dup.h | 6 +- 7 files changed, 102 insertions(+), 28 deletions(-)
Documentation/filesystems/mmap_prepare.rst +3 -0
@@ -153,5 +153,8 @@ pointer. These are: * mmap_action_ioremap_full() - Same as mmap_action_ioremap(), only remaps the entire mapping from ``start_pfn`` onward. +* mmap_action_simple_ioremap() - Sets up an I/O remap from a specified + physical address and over a specified length. + **NOTE:** The ``action`` field should never normally be manipulated directly, rather you ought to use one of these helpers.
include/linux/mm.h +23 -1
@@ -4337,11 +4337,33 @@ static inline void mmap_action_ioremap(struct vm_area_desc *desc,* @start_pfn: The first PFN in the range to remap. */ static inline void mmap_action_ioremap_full (struct vm_area_desc *desc, -unsigned long start_pfn) +unsigned long start_pfn) {mmap_action_ioremap (desc, desc->start, start_pfn,vma_desc_size (desc)); } +/** +* mmap_action_simple_ioremap - helper for mmap_prepare hook to specify that the +* physical range in [start_phys_addr, start_phys_addr + size) should be I/O +* remapped. +* @desc: The VMA descriptor for the VMA requiring remap. +* @start_phys_addr: Start of the physical memory to be mapped. +* @size: Size of the area to map. +* +* NOTE: Some drivers might want to tweak desc->page_prot for purposes of +* write-combine or similar. +*/ +static inline void mmap_action_simple_ioremap (struct vm_area_desc *desc, +phys_addr_t start_phys_addr, +unsigned long size) +{ +struct mmap_action *action = &desc->action; + + action->simple_ioremap.start_phys_addr = start_phys_addr; + action->simple_ioremap.size = size; + action->type =MMAP_SIMPLE_IO_REMAP ; +} +int mmap_action_prepare (struct vm_area_desc *desc); int mmap_action_complete(struct vm_area_struct *vma, struct mmap_action *action,
include/linux/mm_types.h +5 -1
@@ -814,6 +814,7 @@ enum mmap_action_type {MMAP_NOTHING ,/* Mapping is complete, no further action. */ MMAP_REMAP_PFN ,/* Remap PFN range. */ MMAP_IO_REMAP_PFN ,/* I/O remap PFN range. */ + MMAP_SIMPLE_IO_REMAP, /* I/O remap with guardrails. */ };/*
@@ -822,13 +823,16 @@ enum mmap_action_type {*/ struct mmap_action {union { -/* Remap range. */ struct {unsigned long start;unsigned long start_pfn;unsigned long size;pgprot_t pgprot; } remap; +struct { +phys_addr_t start_phys_addr; +unsigned long size; + } simple_ioremap; };enum mmap_action_type type;
mm/internal.h +1 -0
@@ -1804,6 +1804,7 @@ int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm);int remap_pfn_range_prepare (struct vm_area_desc *desc);int remap_pfn_range_complete (struct vm_area_struct *vma,struct mmap_action *action); +int simple_ioremap_prepare (struct vm_area_desc *desc);static inline int io_remap_pfn_range_prepare (struct vm_area_desc *desc) {
mm/memory.c +60 -25
98 lines not on this page · raw message
mm/util.c +5 -0
@@ -1392,6 +1392,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)return remap_pfn_range_prepare (desc);case MMAP_IO_REMAP_PFN :return io_remap_pfn_range_prepare (desc); +case MMAP_SIMPLE_IO_REMAP : +return simple_ioremap_prepare (desc); }WARN_ON_ONCE (1 );
@@ -1423,6 +1425,7 @@ int mmap_action_complete(struct vm_area_struct *vma, err =remap_pfn_range_complete (vma, action);break ;case MMAP_IO_REMAP_PFN : +case MMAP_SIMPLE_IO_REMAP :/* Should have been delegated. */ WARN_ON_ONCE (1 ); err = -EINVAL ;
@@ -1441,6 +1444,7 @@ int mmap_action_prepare(struct vm_area_desc *desc)break ;case MMAP_REMAP_PFN :case MMAP_IO_REMAP_PFN : +case MMAP_SIMPLE_IO_REMAP :WARN_ON_ONCE (1 );/* nommu cannot handle these. */ break ; }
@@ -1460,6 +1464,7 @@ int mmap_action_complete(struct vm_area_struct *vma,break ;case MMAP_REMAP_PFN :case MMAP_IO_REMAP_PFN : +case MMAP_SIMPLE_IO_REMAP :WARN_ON_ONCE (1 );/* nommu cannot handle this. */ err = -EINVAL ;
tools/testing/vma/include/dup.h +5 -1
@@ -453,6 +453,7 @@ enum mmap_action_type {MMAP_NOTHING ,/* Mapping is complete, no further action. */ MMAP_REMAP_PFN ,/* Remap PFN range. */ MMAP_IO_REMAP_PFN ,/* I/O remap PFN range. */ + MMAP_SIMPLE_IO_REMAP, /* I/O remap with guardrails. */ };/*
@@ -461,13 +462,16 @@ enum mmap_action_type {*/ struct mmap_action {union { -/* Remap range. */ struct {unsigned long start;unsigned long start_pfn;unsigned long size;pgprot_t pgprot; } remap; +struct { +phys_addr_t start_phys_addr; +unsigned long size; + } simple_ioremap; };enum mmap_action_type type;
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 7/16 misc: open-dice: replace deprecated mmap hook with mmap_prepare
In reply to Lorenzo Stoakes (Oracle)The f_op->mmap interface is deprecated, so update driver to use its successor, mmap_prepare.
The driver previously used vm_iomap_memory(), so this change replaces it with its mmap_prepare equivalent, mmap_action_simple_ioremap().
--- drivers/misc/open-dice.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
drivers/misc/open-dice.c +11 -8
@@ -86,29 +86,32 @@ static ssize_t open_dice_write(struct file *filp, const char __user *ptr,/* * Creates a mapping of the reserved memory region in user address space. */ -static int open_dice_mmap (struct file *filp,struct vm_area_struct *vma) +static int open_dice_mmap_prepare (struct vm_area_desc *desc) { +struct file *filp = desc->file;struct open_dice_drvdata *drvdata =to_open_dice_drvdata (filp); -if (vma->vm_flags &VM_MAYSHARE ) { +if (vma_desc_test (desc,VMA_MAYSHARE_BIT )) {/* Do not allow userspace to modify the underlying data. */ -if (vma->vm_flags &VM_WRITE ) +if (vma_desc_test (desc,VMA_WRITE_BIT ))return -EPERM ;/* Ensure userspace cannot acquire VM_WRITE later. */ -vm_flags_clear (vma,VM_MAYWRITE ); +vma_desc_clear_flags (desc,VMA_MAYWRITE_BIT ); }/* Create write-combine mapping so all clients observe a wipe. */ - vma->vm_page_prot =pgprot_writecombine (vma->vm_page_prot); -vm_flags_set (vma,VM_DONTCOPY |VM_DONTDUMP ); -return vm_iomap_memory (vma, drvdata->rmem->base, drvdata->rmem->size); + desc->page_prot =pgprot_writecombine (desc->page_prot); +vma_desc_set_flags (desc,VMA_DONTCOPY_BIT ,VMA_DONTDUMP_BIT ); +mmap_action_simple_ioremap (desc, drvdata->rmem->base, + drvdata->rmem->size); +return 0 ; }static const struct file_operations open_dice_fops = { .owner =THIS_MODULE , .read = open_dice_read, .write = open_dice_write, - .mmap = open_dice_mmap, + .mmap_prepare = open_dice_mmap_prepare, };static int __init open_dice_probe (struct platform_device *pdev)
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 8/16 hpet: replace deprecated mmap hook with mmap_prepare
In reply to Lorenzo Stoakes (Oracle)The f_op->mmap interface is deprecated, so update driver to use its successor, mmap_prepare.
The driver previously used vm_iomap_memory(), so this change replaces it with its mmap_prepare equivalent, mmap_action_simple_ioremap().
--- drivers/char/hpet.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/char/hpet.c +7 -5
@@ -354,8 +354,9 @@ static __init int hpet_mmap_enable(char *str) }__setup ("hpet_mmap=" , hpet_mmap_enable); -static int hpet_mmap (struct file *file,struct vm_area_struct *vma) +static int hpet_mmap_prepare (struct vm_area_desc *desc) { +struct file *file = desc->file;struct hpet_dev *devp;unsigned long addr;
@@ -368,11 +369,12 @@ static int hpet_mmap(struct file *file, struct vm_area_struct *vma)if (addr & (PAGE_SIZE -1 ))return -ENOSYS ; - vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); - return vm_iomap_memory(vma, addr, PAGE_SIZE); + desc->page_prot =pgprot_noncached (desc->page_prot); +mmap_action_simple_ioremap (desc, addr,PAGE_SIZE ); + return 0; }#else -static int hpet_mmap (struct file *file,struct vm_area_struct *vma) +static int hpet_mmap_prepare (struct vm_area_desc *desc) {return -ENOSYS ; }
@@ -710,7 +712,7 @@ static const struct file_operations hpet_fops = { .open = hpet_open, .release = hpet_release, .fasync = hpet_fasync, - .mmap = hpet_mmap, + .mmap_prepare = hpet_mmap_prepare, };static int hpet_is_known (struct hpet_data *hdp)
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 9/16 mtdchar: replace deprecated mmap hook with mmap_prepare, clean up
In reply to Lorenzo Stoakes (Oracle)Replace the deprecated mmap callback with mmap_prepare.
Commit f5cf8f07423b ("mtd: Disable mtdchar mmap on MMU systems") commented out the CONFIG_MMU part of this function back in 2012, so after ~14 years it's probably reasonable to remove this altogether rather than updating dead code.
--- drivers/mtd/mtdchar.c | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-)
drivers/mtd/mtdchar.c +3 -18
@@ -1376,27 +1376,12 @@ static unsigned mtdchar_mmap_capabilities(struct file *file)/* * set up a mapping for shared memory segments */ -static int mtdchar_mmap (struct file *file,struct vm_area_struct *vma) +static int mtdchar_mmap_prepare (struct vm_area_desc *desc) {#ifdef CONFIG_MMU -struct mtd_file_info *mfi = file->private_data; -struct mtd_info *mtd = mfi->mtd; -struct map_info *map = mtd->priv; - -/* This is broken because it assumes the MTD device is map-based -and that mtd->priv is a valid struct map_info. It should be -replaced with something that uses the mtd_get_unmapped_area() -operation properly. */ -if (0 /*mtd->type == MTD_RAM || mtd->type == MTD_ROM*/ ) { -#ifdef pgprot_noncached -if (file->f_flags &O_DSYNC || map->phys >=__pa (high_memory)) - vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); -#endif -return vm_iomap_memory (vma, map->phys, map->size); - }return -ENODEV ;#else -return vma->vm_flags &VM_SHARED ?0 : -EACCES ; +return vma_desc_test (desc,VMA_SHARED_BIT ) ?0 : -EACCES ;#endif }
@@ -1411,7 +1396,7 @@ static const struct file_operations mtd_fops = {#endif .open = mtdchar_open, .release = mtdchar_close, - .mmap = mtdchar_mmap, + .mmap_prepare = mtdchar_mmap_prepare, #ifndef CONFIG_MMU .get_unmapped_area = mtdchar_get_unmapped_area, .mmap_capabilities = mtdchar_mmap_capabilities,
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 10/16 stm: replace deprecated mmap hook with mmap_prepare
In reply to Lorenzo Stoakes (Oracle)The f_op->mmap interface is deprecated, so update driver to use its successor, mmap_prepare.
The driver previously used vm_iomap_memory(), so this change replaces it with its mmap_prepare equivalent, mmap_action_simple_ioremap().
Also, in order to correctly maintain reference counting, add a vm_ops->mapped callback to increment the reference count when successfully mapped.
--- drivers/hwtracing/stm/core.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-)
drivers/hwtracing/stm/core.c +21 -10
@@ -666,6 +666,16 @@ static ssize_t stm_char_write(struct file *file, const char __user *buf,return count; } +static int stm_mmap_mapped (unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data) +{ +struct stm_file *stmf = file->private_data; +struct stm_device *stm = stmf->stm; + +pm_runtime_get_sync (&stm->dev); +return 0 ; +} +static void stm_mmap_open (struct vm_area_struct *vma) {struct stm_file *stmf = vma->vm_file->private_data;
@@ -684,12 +694,14 @@ static void stm_mmap_close(struct vm_area_struct *vma) }static const struct vm_operations_struct stm_mmap_vmops = { + .mapped = stm_mmap_mapped, .open = stm_mmap_open, .close = stm_mmap_close, }; -static int stm_char_mmap (struct file *file,struct vm_area_struct *vma) +static int stm_char_mmap_prepare (struct vm_area_desc *desc) { +struct file *file = desc->file;struct stm_file *stmf = file->private_data;struct stm_device *stm = stmf->stm;unsigned long size, phys;
@@ -697,10 +709,10 @@ static int stm_char_mmap(struct file *file, struct vm_area_struct *vma)if (!stm->data->mmio_addr)return -EOPNOTSUPP ; -if (vma->vm_pgoff) +if (desc->pgoff)return -EINVAL ; - size = vma->vm_end - vma->vm_start; + size =vma_desc_size (desc);if (stmf->output.nr_chans * stm->data->sw_mmiosz != size)return -EINVAL ;
@@ -712,13 +724,12 @@ static int stm_char_mmap(struct file *file, struct vm_area_struct *vma)if (!phys)return -EINVAL ; -pm_runtime_get_sync (&stm->dev); - - vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); -vm_flags_set (vma,VM_IO |VM_DONTEXPAND |VM_DONTDUMP ); - vma->vm_ops = &stm_mmap_vmops; -vm_iomap_memory (vma, phys, size); + desc->page_prot =pgprot_noncached (desc->page_prot); +vma_desc_set_flags (desc,VMA_IO_BIT ,VMA_DONTEXPAND_BIT , +VMA_DONTDUMP_BIT ); + desc->vm_ops = &stm_mmap_vmops; +mmap_action_simple_ioremap (desc, phys, size);return 0 ; }
@@ -836,7 +847,7 @@ static const struct file_operations stm_fops = { .open = stm_char_open, .release = stm_char_release, .write = stm_char_write, - .mmap = stm_char_mmap, + .mmap_prepare = stm_char_mmap_prepare, .unlocked_ioctl = stm_char_ioctl, .compat_ioctl = compat_ptr_ioctl, };
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 11/16 staging: vme_user: replace deprecated mmap hook with mmap_prepare
In reply to Lorenzo Stoakes (Oracle)The f_op->mmap interface is deprecated, so update driver to use its successor, mmap_prepare.
The driver previously used vm_iomap_memory(), so this change replaces it with its mmap_prepare equivalent, mmap_action_simple_ioremap().
Functions that wrap mmap() are also converted to wrap mmap_prepare() instead.
Also update the documentation accordingly.
--- Documentation/driver-api/vme.rst | 2 +- drivers/staging/vme_user/vme.c | 20 +++++------ drivers/staging/vme_user/vme.h | 2 +- drivers/staging/vme_user/vme_user.c | 51 +++++++++++++++++------------ 4 files changed, 42 insertions(+), 33 deletions(-)
Documentation/driver-api/vme.rst +1 -1
@@ -107,7 +107,7 @@ The function :c:func:`vme_master_read` can be used to read from and In addition to simple reads and writes, :c:func:`vme_master_rmw` is provided to do a read-modify-write transaction. Parts of a VME window can also be mapped -into user space memory using :c:func:`vme_master_mmap`. +into user space memory using :c:func:`vme_master_mmap_prepare`. Slave windows
drivers/staging/vme_user/vme.c +10 -10
@@ -735,9 +735,9 @@ unsigned int vme_master_rmw(struct vme_resource *resource, unsigned int mask,EXPORT_SYMBOL (vme_master_rmw);/** -* vme_master_mmap - Mmap region of VME master window. +* vme_master_mmap_prepare - Mmap region of VME master window. * @resource: Pointer to VME master resource. -* @vma: Pointer to definition of user mapping. +* @desc: Pointer to descriptor of user mapping. * * Memory map a region of the VME master window into user space. *
@@ -745,12 +745,13 @@ EXPORT_SYMBOL(vme_master_rmw);* resource or -EFAULT if map exceeds window size. Other generic mmap * errors may also be returned. */ -int vme_master_mmap (struct vme_resource *resource,struct vm_area_struct *vma) +int vme_master_mmap_prepare (struct vme_resource *resource, +struct vm_area_desc *desc) { +const unsigned long vma_size =vma_desc_size (desc);struct vme_bridge *bridge =find_bridge (resource);struct vme_master_resource *image;phys_addr_t phys_addr; -unsigned long vma_size;if (resource->type !=VME_MASTER ) {dev_err (bridge->parent,"Not a master resource\n" );
@@ -758,19 +759,18 @@ int vme_master_mmap(struct vme_resource *resource, struct vm_area_struct *vma) } image = list_entry(resource->entry, struct vme_master_resource, list); - phys_addr = image->bus_resource.start + (vma->vm_pgoff <<PAGE_SHIFT ); - vma_size = vma->vm_end - vma->vm_start; + phys_addr = image->bus_resource.start + (desc->pgoff <<PAGE_SHIFT );if (phys_addr + vma_size > image->bus_resource.end +1 ) {dev_err (bridge->parent,"Map size cannot exceed the window size\n" );return -EFAULT ; } - vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); - -return vm_iomap_memory (vma, phys_addr, vma->vm_end - vma->vm_start); + desc->page_prot =pgprot_noncached (desc->page_prot); +mmap_action_simple_ioremap (desc, phys_addr, vma_size); +return 0 ; } -EXPORT_SYMBOL (vme_master_mmap); +EXPORT_SYMBOL (vme_master_mmap_prepare);/** * vme_master_free - Free VME master window
drivers/staging/vme_user/vme.h +1 -1
@@ -151,7 +151,7 @@ ssize_t vme_master_read(struct vme_resource *resource, void *buf, size_t count,ssize_t vme_master_write (struct vme_resource *resource,void *buf,size_t count,loff_t offset);unsigned int vme_master_rmw (struct vme_resource *resource,unsigned int mask,unsigned int compare,unsigned int swap,loff_t offset); -int vme_master_mmap (struct vme_resource *resource,struct vm_area_struct *vma); +int vme_master_mmap_prepare (struct vme_resource *resource,struct vm_area_desc *desc);void vme_master_free (struct vme_resource *resource);struct vme_resource *vme_dma_request (struct vme_dev *vdev,u32 route);
drivers/staging/vme_user/vme_user.c +30 -21
83 lines not on this page · raw message
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 12/16 mm: allow handling of stacked mmap_prepare hooks in more drivers
In reply to Lorenzo Stoakes (Oracle)While the conversion of mmap hooks to mmap_prepare is underway, we will encounter situations where mmap hooks need to invoke nested mmap_prepare hooks.
The nesting of mmap hooks is termed 'stacking'. In order to flexibly facilitate the conversion of custom mmap hooks in drivers which stack, we must split up the existing __compat_vma_mmap() function into two separate functions:
* compat_set_desc_from_vma() - This allows the setting of a vm_area_desc object's fields to the relevant fields of a VMA.
* __compat_vma_mmap() - Once an mmap_prepare hook has been executed upon a vm_area_desc object, this function performs any mmap actions specified by the mmap_prepare hook and then invokes its vm_ops->mapped() hook if any were specified.
In ordinary cases, where a file's f_op->mmap_prepare() hook simply needs to be invoked in a stacked mmap() hook, compat_vma_mmap() can be used.
However some drivers define their own nested hooks, which are invoked in turn by another hook.
A concrete example is vmbus_channel->mmap_ring_buffer(), which is invoked in turn by bin_attribute->mmap():
vmbus_channel->mmap_ring_buffer() has a signature of:
int (*mmap_ring_buffer)(struct vmbus_channel *channel,
struct vm_area_struct *vma);
And bin_attribute->mmap() has a signature of:
int (*mmap)(struct file *, struct kobject *,
const struct bin_attribute *attr, struct vm_area_struct *vma);
And so compat_vma_mmap() cannot be used here for incremental conversion of hooks from mmap() to mmap_prepare().
There are many such instances like this, where conversion to mmap_prepare would otherwise cascade to a huge change set due to nesting of this kind.
The changes in this patch mean we could now instead convert vmbus_channel->mmap_ring_buffer() to vmbus_channel->mmap_prepare_ring_buffer(), and implement something like:
struct vm_area_desc desc; int err; compat_set_desc_from_vma(&desc, file, vma); err = channel->mmap_prepare_ring_buffer(channel, &desc); if (err) return err; return __compat_vma_mmap(&desc, vma);
Allowing us to incrementally update this logic, and other logic like it.
Unfortunately, as part of this change, we need to be able to flexibly assign to the VMA descriptor, so have to remove some of the const declarations within the structure.
Also update the VMA tests to reflect the changes.
--- include/linux/fs.h | 3 + include/linux/mm_types.h | 4 +- mm/util.c | 112 ++++++++++++++++++++++--------- mm/vma.h | 2 +- tools/testing/vma/include/dup.h | 114 +++++++++++++++++++++----------- 5 files changed, 162 insertions(+), 73 deletions(-)
include/linux/fs.h +3 -0
@@ -2058,6 +2058,9 @@ static inline bool can_mmap_file(struct file *file)return true; } +void compat_set_desc_from_vma (struct vm_area_desc *desc,const struct file *file, +const struct vm_area_struct *vma); +int __compat_vma_mmap (struct vm_area_desc *desc,struct vm_area_struct *vma);int compat_vma_mmap (struct file *file,struct vm_area_struct *vma);int __vma_check_mmap_hook (struct vm_area_struct *vma);
include/linux/mm_types.h +2 -2
@@ -891,8 +891,8 @@ static __always_inline bool vma_flags_empty(vma_flags_t *flags)*/ struct vm_area_desc {/* Immutable state. */ -const struct mm_struct *const mm; -struct file *const file;/* May vary from vm_file in stacked callers. */ +struct mm_struct *mm; +struct file *file;/* May vary from vm_file in stacked callers. */ unsigned long start;unsigned long end;
mm/util.c +82 -30
152 lines not on this page · raw message
mm/vma.h +1 -1
@@ -300,7 +300,7 @@ static inline int vma_iter_store_gfp(struct vma_iterator *vmi,* f_op->mmap() but which might have an underlying file system which implements * f_op->mmap_prepare(). */ -static inline void set_vma_from_desc (struct vm_area_struct *vma, +static inline void compat_set_vma_from_desc (struct vm_area_struct *vma,struct vm_area_desc *desc) {/*
tools/testing/vma/include/dup.h +74 -40
158 lines not on this page · raw message
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 13/16 drivers: hv: vmbus: replace deprecated mmap hook with mmap_prepare
In reply to Lorenzo Stoakes (Oracle)The f_op->mmap interface is deprecated, so update the vmbus driver to use its successor, mmap_prepare.
This updates all callbacks which referenced the function pointer hv_mmap_ring_buffer to instead reference hv_mmap_prepare_ring_buffer, utilising the newly introduced compat_set_desc_from_vma() and __compat_vma_mmap() to be able to implement this change.
The UIO HV generic driver is the only user of hv_create_ring_sysfs(), which is the only function which references vmbus_channel->mmap_prepare_ring_buffer which, in turn, is the only external interface to hv_mmap_prepare_ring_buffer.
This patch therefore updates this caller to use mmap_prepare instead, which also previously used vm_iomap_memory(), so this change replaces it with its mmap_prepare equivalent, mmap_action_simple_ioremap().
--- drivers/hv/hyperv_vmbus.h | 4 ++-- drivers/hv/vmbus_drv.c | 31 +++++++++++++++++++------------ drivers/uio/uio_hv_generic.c | 11 ++++++----- include/linux/hyperv.h | 4 ++-- 4 files changed, 29 insertions(+), 21 deletions(-)
drivers/hv/hyperv_vmbus.h +2 -2
@@ -545,8 +545,8 @@ static inline int hv_debug_add_dev_dir(struct hv_device *dev)/* Create and remove sysfs entry for memory mapped ring buffers for a channel */ int hv_create_ring_sysfs (struct vmbus_channel *channel, -int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel, -struct vm_area_struct *vma)); +int (*hv_mmap_prepare_ring_buffer)(struct vmbus_channel *channel, +struct vm_area_desc *desc));int hv_remove_ring_sysfs (struct vmbus_channel *channel);#endif /* _HYPERV_VMBUS_H */
drivers/hv/vmbus_drv.c +19 -12
@@ -1951,12 +1951,19 @@ static int hv_mmap_ring_buffer_wrapper(struct file *filp, struct kobject *kobj, struct vm_area_struct *vma) { struct vmbus_channel *channel = container_of(kobj, struct vmbus_channel, kobj); +struct vm_area_desc desc; +int err;/* -* hv_(create|remove)_ring_sysfs implementation ensures that mmap_ring_buffer -* is not NULL. +* hv_(create|remove)_ring_sysfs implementation ensures that +* mmap_prepare_ring_buffer is not NULL. */ -return channel->mmap_ring_buffer (channel, vma); +compat_set_desc_from_vma (&desc, filp, vma); + err = channel->mmap_prepare_ring_buffer (channel, &desc); +if (err) +return err; + +return __compat_vma_mmap (&desc, vma); }static struct bin_attribute chan_attr_ring_buffer = {
@@ -2048,13 +2055,13 @@ static const struct kobj_type vmbus_chan_ktype = {/** * hv_create_ring_sysfs() - create "ring" sysfs entry corresponding to ring buffers for a channel. * @channel: Pointer to vmbus_channel structure -* @hv_mmap_ring_buffer: function pointer for initializing the function to be called on mmap of +* @hv_mmap_prepare_ring_buffer: function pointer for initializing the function to be called on mmap * channel's "ring" sysfs node, which is for the ring buffer of that channel. * Function pointer is of below type: -* int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel, -* struct vm_area_struct *vma)) -* This has a pointer to the channel and a pointer to vm_area_struct, -* used for mmap, as arguments. +* int (*hv_mmap_prepare_ring_buffer)(struct vmbus_channel *channel, +* struct vm_area_desc *desc)) +* This has a pointer to the channel and a pointer to vm_area_desc, +* used for mmap_prepare, as arguments. * * Sysfs node for ring buffer of a channel is created along with other fields, however its * visibility is disabled by default. Sysfs creation needs to be controlled when the use-case
@@ -2071,12 +2078,12 @@ static const struct kobj_type vmbus_chan_ktype = {* Returns 0 on success or error code on failure. */ int hv_create_ring_sysfs (struct vmbus_channel *channel, -int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel, -struct vm_area_struct *vma)) +int (*hv_mmap_prepare_ring_buffer)(struct vmbus_channel *channel, +struct vm_area_desc *desc)) {struct kobject *kobj = &channel->kobj; - channel->mmap_ring_buffer = hv_mmap_ring_buffer; + channel->mmap_prepare_ring_buffer = hv_mmap_prepare_ring_buffer; channel->ring_sysfs_visible = true;return sysfs_update_group (kobj, &vmbus_chan_group);
@@ -2098,7 +2105,7 @@ int hv_remove_ring_sysfs(struct vmbus_channel *channel) channel->ring_sysfs_visible = false; ret =sysfs_update_group (kobj, &vmbus_chan_group); - channel->mmap_ring_buffer =NULL ; + channel->mmap_prepare_ring_buffer =NULL ;return ret; }EXPORT_SYMBOL_GPL (hv_remove_ring_sysfs);
drivers/uio/uio_hv_generic.c +6 -5
@@ -154,15 +154,16 @@ static void hv_uio_rescind(struct vmbus_channel *channel)* The ring buffer is allocated as contiguous memory by vmbus_open */ static int -hv_uio_ring_mmap (struct vmbus_channel *channel,struct vm_area_struct *vma) +hv_uio_ring_mmap_prepare (struct vmbus_channel *channel,struct vm_area_desc *desc) {void *ring_buffer =page_address (channel->ringbuffer_page);if (channel->state !=CHANNEL_OPENED_STATE )return -ENODEV ; -return vm_iomap_memory (vma,virt_to_phys (ring_buffer), - channel->ringbuffer_pagecount <<PAGE_SHIFT ); +mmap_action_simple_ioremap (desc,virt_to_phys (ring_buffer), + channel->ringbuffer_pagecount <<PAGE_SHIFT ); +return 0 ; }/* Callback from VMBUS subsystem when new channel created. */
@@ -183,7 +184,7 @@ hv_uio_new_channel(struct vmbus_channel *new_sc) }set_channel_read_mode (new_sc,HV_CALL_ISR ); - ret =hv_create_ring_sysfs (new_sc, hv_uio_ring_mmap); + ret =hv_create_ring_sysfs (new_sc, hv_uio_ring_mmap_prepare);if (ret) {dev_err (device,"sysfs create ring bin file failed; %d\n" , ret);vmbus_close (new_sc);
@@ -366,7 +367,7 @@ hv_uio_probe(struct hv_device *dev,* or decoupled from uio_hv_generic probe. Userspace programs can make use of inotify * APIs to make sure that ring is created. */ -hv_create_ring_sysfs (channel, hv_uio_ring_mmap); +hv_create_ring_sysfs (channel, hv_uio_ring_mmap_prepare);hv_set_drvdata (dev, pdata);
include/linux/hyperv.h +2 -2
@@ -1015,8 +1015,8 @@ struct vmbus_channel {/* The max size of a packet on this channel */ u32 max_pkt_size; -/* function to mmap ring buffer memory to the channel's sysfs ring attribute */ -int (*mmap_ring_buffer)(struct vmbus_channel *channel,struct vm_area_struct *vma); +/* function to mmap_prepare ring buffer memory to the channel's sysfs ring attribute */ +int (*mmap_prepare_ring_buffer)(struct vmbus_channel *channel,struct vm_area_desc *desc);/* boolean to control visibility of sysfs for ring buffer */ bool ring_sysfs_visible;
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 14/16 uio: replace deprecated mmap hook with mmap_prepare in uio_info
In reply to Lorenzo Stoakes (Oracle)The f_op->mmap interface is deprecated, so update uio_info to use its successor, mmap_prepare.
Therefore, replace the uio_info->mmap hook with a new uio_info->mmap_prepare hook, and update its one user, target_core_user, to both specify this new mmap_prepare hook and also to use the new vm_ops->mapped() hook to continue to maintain a correct udev->kref refcount.
Then update uio_mmap() to utilise the mmap_prepare compatibility layer to invoke this callback from the uio mmap invocation.
--- drivers/target/target_core_user.c | 26 ++++++++++++++++++-------- drivers/uio/uio.c | 10 ++++++++-- include/linux/uio_driver.h | 4 ++-- 3 files changed, 28 insertions(+), 12 deletions(-)
drivers/target/target_core_user.c +18 -8
@@ -1860,6 +1860,17 @@ static struct page *tcmu_try_get_data_page(struct tcmu_dev *udev, uint32_t dpi)return NULL ; } +static int tcmu_vma_mapped (unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data) +{ +struct tcmu_dev *udev = *vm_private_data; + +pr_debug ("vma_mapped\n" ); + +kref_get (&udev->kref); +return 0 ; +} +static void tcmu_vma_open (struct vm_area_struct *vma) {struct tcmu_dev *udev = vma->vm_private_data;
@@ -1919,26 +1930,25 @@ static vm_fault_t tcmu_vma_fault(struct vm_fault *vmf) }static const struct vm_operations_struct tcmu_vm_ops = { + .mapped = tcmu_vma_mapped, .open = tcmu_vma_open, .close = tcmu_vma_close, .fault = tcmu_vma_fault, }; -static int tcmu_mmap (struct uio_info *info,struct vm_area_struct *vma) +static int tcmu_mmap_prepare (struct uio_info *info,struct vm_area_desc *desc) { struct tcmu_dev *udev = container_of(info, struct tcmu_dev, uio_info); -vm_flags_set (vma,VM_DONTEXPAND |VM_DONTDUMP ); - vma->vm_ops = &tcmu_vm_ops; +vma_desc_set_flags (desc,VMA_DONTEXPAND_BIT ,VMA_DONTDUMP_BIT ); + desc->vm_ops = &tcmu_vm_ops; - vma->vm_private_data = udev; + desc->private_data = udev;/* Ensure the mmap is exactly the right size */ -if (vma_pages (vma) != udev->mmap_pages) +if (vma_desc_pages (desc) != udev->mmap_pages)return -EINVAL ; -tcmu_vma_open (vma); -return 0 ; }
@@ -2253,7 +2263,7 @@ static int tcmu_configure_device(struct se_device *dev) info->irqcontrol = tcmu_irqcontrol; info->irq =UIO_IRQ_CUSTOM ; - info->mmap = tcmu_mmap; + info->mmap_prepare = tcmu_mmap_prepare; info->open = tcmu_open; info->release = tcmu_release;
drivers/uio/uio.c +8 -2
@@ -850,8 +850,14 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma) goto out; } -if (idev->info->mmap) { - ret = idev->info->mmap (idev->info, vma); +if (idev->info->mmap_prepare) { +struct vm_area_desc desc; + +compat_set_desc_from_vma (&desc, filep, vma); + ret = idev->info->mmap_prepare (idev->info, &desc); +if (ret) + goto out; + ret =__compat_vma_mmap (&desc, vma); goto out; }
include/linux/uio_driver.h +2 -2
@@ -97,7 +97,7 @@ struct uio_device {* @irq_flags: flags for request_irq() * @priv: optional private data * @handler: the device's irq handler -* @mmap: mmap operation for this uio device +* @mmap_prepare: mmap_prepare operation for this uio device * @open: open operation for this uio device * @release: release operation for this uio device * @irqcontrol: disable/enable irqs when 0/1 is written to /dev/uioX
@@ -112,7 +112,7 @@ struct uio_info {unsigned long irq_flags;void *priv;irqreturn_t (*handler)(int irq,struct uio_info *dev_info); -int (*mmap)(struct uio_info *info,struct vm_area_struct *vma); +int (*mmap_prepare)(struct uio_info *info,struct vm_area_desc *desc);int (*open)(struct uio_info *info,struct inode *inode);int (*release)(struct uio_info *info,struct inode *inode);int (*irqcontrol)(struct uio_info *info,s32 irq_on);
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 15/16 mm: add mmap_action_map_kernel_pages[_full]()
In reply to Lorenzo Stoakes (Oracle)A user can invoke mmap_action_map_kernel_pages() to specify that the mapping should map kernel pages starting from desc->start of a specified number of pages specified in an array.
In order to implement this, adjust mmap_action_prepare() to be able to return an error code, as it makes sense to assert that the specified parameters are valid as quickly as possible as well as updating the VMA flags to include VMA_MIXEDMAP_BIT as necessary.
This provides an mmap_prepare equivalent of vm_insert_pages(). We additionally update the existing vm_insert_pages() code to use range_in_vma() and add a new range_in_vma_desc() helper function for the mmap_prepare case, sharing the code between the two in range_is_subset().
We add both mmap_action_map_kernel_pages() and mmap_action_map_kernel_pages_full() to allow for both partial and full VMA mappings.
We update the documentation to reflect the new features.
Finally, we update the VMA tests accordingly to reflect the changes.
--- Documentation/filesystems/mmap_prepare.rst | 8 ++ include/linux/mm.h | 95 +++++++++++++++++++++- include/linux/mm_types.h | 7 ++ mm/memory.c | 42 +++++++++- mm/util.c | 6 ++ tools/testing/vma/include/dup.h | 7 ++ 6 files changed, 159 insertions(+), 6 deletions(-)
Documentation/filesystems/mmap_prepare.rst +8 -0
@@ -156,5 +156,13 @@ pointer. These are: * mmap_action_simple_ioremap() - Sets up an I/O remap from a specified physical address and over a specified length. +* mmap_action_map_kernel_pages() - Maps a specified array of `struct page` + pointers in the VMA from a specific offset. + +* mmap_action_map_kernel_pages_full() - Maps a specified array of `struct + page` pointers over the entire VMA. The caller must ensure there are + sufficient entries in the page array to cover the entire range of the + described VMA. + **NOTE:** The ``action`` field should never normally be manipulated directly, rather you ought to use one of these helpers.
include/linux/mm.h +93 -2
122 lines not on this page · raw message
include/linux/mm_types.h +7 -0
@@ -815,6 +815,7 @@ enum mmap_action_type {MMAP_REMAP_PFN ,/* Remap PFN range. */ MMAP_IO_REMAP_PFN ,/* I/O remap PFN range. */ MMAP_SIMPLE_IO_REMAP ,/* I/O remap with guardrails. */ + MMAP_MAP_KERNEL_PAGES, /* Map kernel page range from array. */ };/*
@@ -833,6 +834,12 @@ struct mmap_action {phys_addr_t start_phys_addr;unsigned long size; } simple_ioremap; +struct { +unsigned long start; +struct page **pages; +unsigned long nr_pages; +pgoff_t pgoff; + } map_kernel; };enum mmap_action_type type;
mm/memory.c +38 -4
57 lines not on this page · raw message
mm/util.c +6 -0
@@ -1446,6 +1446,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)return io_remap_pfn_range_prepare (desc);case MMAP_SIMPLE_IO_REMAP :return simple_ioremap_prepare (desc); +case MMAP_MAP_KERNEL_PAGES : +return map_kernel_pages_prepare (desc); }WARN_ON_ONCE (1 );
@@ -1476,6 +1478,9 @@ int mmap_action_complete(struct vm_area_struct *vma,case MMAP_REMAP_PFN : err =remap_pfn_range_complete (vma, action);break ; +case MMAP_MAP_KERNEL_PAGES : + err =map_kernel_pages_complete (vma, action); +break ;case MMAP_IO_REMAP_PFN :case MMAP_SIMPLE_IO_REMAP :/* Should have been delegated. */
@@ -1497,6 +1502,7 @@ int mmap_action_prepare(struct vm_area_desc *desc)case MMAP_REMAP_PFN :case MMAP_IO_REMAP_PFN :case MMAP_SIMPLE_IO_REMAP : +case MMAP_MAP_KERNEL_PAGES :WARN_ON_ONCE (1 );/* nommu cannot handle these. */ break ; }
tools/testing/vma/include/dup.h +7 -0
@@ -454,6 +454,7 @@ enum mmap_action_type {MMAP_REMAP_PFN ,/* Remap PFN range. */ MMAP_IO_REMAP_PFN ,/* I/O remap PFN range. */ MMAP_SIMPLE_IO_REMAP ,/* I/O remap with guardrails. */ + MMAP_MAP_KERNEL_PAGES, /* Map kernel page range from an array. */ };/*
@@ -472,6 +473,12 @@ struct mmap_action {phys_addr_t start_phys_addr;unsigned long size; } simple_ioremap; +struct { +unsigned long start; +struct page **pages; +unsigned long nr_pages; +pgoff_t pgoff; + } map_kernel; };enum mmap_action_type type;
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH v3 16/16 mm: on remap assert that input range within the proposed VMA
In reply to Lorenzo Stoakes (Oracle)Now we have range_in_vma_desc(), update remap_pfn_range_prepare() to check whether the input range in contained within the specified VMA, so we can fail at prepare time if an invalid range is specified.
This covers the I/O remap mmap actions also which ultimately call into this function, and other mmap action types either already span the full VMA or check this already.
--- mm/memory.c | 3 +++ 1 file changed, 3 insertions(+)
mm/memory.c +3 -0
@@ -3142,6 +3142,9 @@ int remap_pfn_range_prepare(struct vm_area_desc *desc)const bool is_cow =vma_desc_is_cow_mapping (desc);int err; +if (!range_in_vma_desc (desc, start, end)) +return -EFAULT ; + err =get_remap_pgoff (is_cow, start, end, desc->start, desc->end, pfn, &desc->pgoff);if (err)
Signature · 1 line
2.53.0
Andrew Morton
maintainerQuoted 4 lines · On Thu, 19 Mar 2026 18:23:24 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
This series expands the
mmap_preparefunctionality, which is intended to replace the deprecatedf_op->mmaphook which has been the source of bugs and security issues for some time.
Thanks, I updated mm.git's mm-new branch to this version.
Quoted 19 lines
v3:
* Propagated tags (thanks Suren, Richard!)
* Updated 12/16 to correctly clear thevm_area_descdata structure inset_desc_from_vma()as per Joshua Hahn (thanks! :)
* Fixed type in 12/16 as per Suren (cheers!)
* Fixed up 6/16 to usemmap_action_ioremap_full()insimple_ioremap_prepare()as suggested by Suren.
* Also fixed up 6/16 to callio_remap_pfn_range_prepare()direct rather thanmmap_action_prepare()as per Suren.
* Also fixed up 6/16 to passvm_lenrather than vm_[start, end] to__simple_ioremap_prep()as per Suren (thanks for all the above! :)
* Fixed issue in rmap lock being held - we were referencing avma->vm_fileafter the VMA was unmapped, so UAF. Avoid that. Alsodo_munmap()relies on rmap lock NOT being held or may deadlock, so extend functionality to ensure we drop it when it is held on error paths.
* Updated 'area' -> 'vma' variable in 3/16 in VMA testdup.h.
* Fixed up reference to__compat_vma_mmap()in 12/16 commit message.
* Updated 1/16 to no longer duplicatively applyio_remap_pfn_range_pfn().
Quoted 8 lines
* Updated 1/16 to delegate I/O remap complete to remap complete logic.
* Fixed various typos in 12/16.
* Fixed stale comment typos in 13/16.
* Fixed commit msg and comment typos in 14/16.
* Removed accidental sneak peak to future functionality in 15/16 commit message :).
* Fixed up field names to be identical in VMA tests +mm_types.hin 6/16, 15/16.
Here's how v3 altered mm.git:
drivers/hv/vmbus_drv.c | 6 +- drivers/target/target_core_user.c | 2 include/linux/mm.h | 3 - include/linux/uio_driver.h | 2 mm/internal.h | 22 ++++----- mm/memory.c | 16 +++---- mm/util.c | 62 ++++++++++++++-------------- mm/vma.c | 24 +++------- tools/testing/vma/include/dup.h | 19 ++++---- tools/testing/vma/include/stubs.h | 3 - 10 files changed, 77 insertions(+), 82 deletions(-)
drivers/hv/vmbus_drv.c renamed from drivers/hv/vmbus_drv.c~b +3 -3
@@ -1955,8 +1955,8 @@ static int hv_mmap_ring_buffer_wrapper(sint err;/* -* hv_(create|remove)_ring_sysfs implementation ensures that mmap_ring_buffer -* is not NULL. +* hv_(create|remove)_ring_sysfs implementation ensures that +* mmap_prepare_ring_buffer is not NULL. */ compat_set_desc_from_vma (&desc, filp, vma); err = channel->mmap_prepare_ring_buffer (channel, &desc);
@@ -2055,7 +2055,7 @@ static const struct kobj_type vmbus_chan/** * hv_create_ring_sysfs() - create "ring" sysfs entry corresponding to ring buffers for a channel. * @channel: Pointer to vmbus_channel structure -* @hv_mmap_ring_buffer: function pointer for initializing the function to be called on mmap +* @hv_mmap_prepare_ring_buffer: function pointer for initializing the function to be called on mmap * channel's "ring" sysfs node, which is for the ring buffer of that channel. * Function pointer is of below type: * int (*hv_mmap_prepare_ring_buffer)(struct vmbus_channel *channel,
drivers/target/target_core_user.c renamed from drivers/target/target_core_user.c~b +1 -1
@@ -1865,7 +1865,7 @@ static int tcmu_vma_mapped(unsigned long {struct tcmu_dev *udev = *vm_private_data; -pr_debug ("vma_open\n" ); +pr_debug ("vma_mapped\n" );kref_get (&udev->kref);return 0 ;
include/linux/mm.h renamed from include/linux/mm.h~b +2 -1
@@ -4405,7 +4405,8 @@ static inline void mmap_action_map_kerneint mmap_action_prepare (struct vm_area_desc *desc);int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action); +struct mmap_action *action, +bool rmap_lock_held);/* Look up the first VMA which exactly match the interval vm_start ... vm_end */ static inline struct vm_area_struct *find_exact_vma(struct mm_struct *mm,
include/linux/uio_driver.h renamed from include/linux/uio_driver.h~b +1 -1
@@ -97,7 +97,7 @@ struct uio_device {* @irq_flags: flags for request_irq() * @priv: optional private data * @handler: the device's irq handler -* @mmap_prepare: mmap_pepare operation for this uio device +* @mmap_prepare: mmap_prepare operation for this uio device * @open: open operation for this uio device * @release: release operation for this uio device * @irqcontrol: disable/enable irqs when 0/1 is written to /dev/uioX
mm/internal.h renamed from mm/internal.h~b +9 -13
@@ -1805,29 +1805,25 @@ int remap_pfn_range_prepare(struct vm_arint remap_pfn_range_complete (struct vm_area_struct *vma,struct mmap_action *action);int simple_ioremap_prepare (struct vm_area_desc *desc); -/* No simple_ioremap_complete, is ultimately handled by remap complete. */ static inline int io_remap_pfn_range_prepare (struct vm_area_desc *desc) {struct mmap_action *action = &desc->action;const unsigned long orig_pfn = action->remap.start_pfn; +const pgprot_t orig_pgprot = action->remap.pgprot;const unsigned long size = action->remap.size;const unsigned long pfn =io_remap_pfn_range_pfn (orig_pfn, size); +int err; action->remap.start_pfn = pfn; -return remap_pfn_range_prepare (desc); -} - -static inline int io_remap_pfn_range_complete (struct vm_area_struct *vma, -struct mmap_action *action) -{ -const unsigned long size = action->remap.size; -const unsigned long orig_pfn = action->remap.start_pfn; -const pgprot_t orig_prot = vma->vm_page_prot; + action->remap.pgprot =pgprot_decrypted (orig_pgprot); + err =remap_pfn_range_prepare (desc); +if (err) +return err; - action->remap.pgprot =pgprot_decrypted (orig_prot); - action->remap.start_pfn =io_remap_pfn_range_pfn (orig_pfn, size); -return remap_pfn_range_complete (vma, action); +/* Remap does the actual work. */ + action->type =MMAP_REMAP_PFN ; +return 0 ; }#ifdef CONFIG_MMU_NOTIFIER
mm/memory.c renamed from mm/memory.c~b +7 -9
@@ -3207,11 +3207,10 @@ int remap_pfn_range_complete(struct vm_areturn do_remap_pfn_range (vma, start, pfn, size, prot); } -static int __simple_ioremap_prep (unsigned long vm_start,unsigned long vm_end, -pgoff_t vm_pgoff,phys_addr_t start_phys, -unsigned long size,unsigned long *pfnp) +static int __simple_ioremap_prep (unsigned long vm_len,pgoff_t vm_pgoff, +phys_addr_t start_phys,unsigned long size, +unsigned long *pfnp) { -const unsigned long vm_len = vm_end - vm_start;unsigned long pfn, pages;/* Check that the physical memory area passed in looks valid */
@@ -3250,14 +3249,14 @@ int simple_ioremap_prepare(struct vm_areunsigned long pfn;int err; - err =__simple_ioremap_prep (desc->start, desc->end, desc->pgoff, + err =__simple_ioremap_prep (vma_desc_size (desc), desc->pgoff, start, size, &pfn);if (err)return err;/* The I/O remap logic does the heavy lifting. */ -mmap_action_ioremap (desc, desc->start, pfn,vma_desc_size (desc)); -return mmap_action_prepare (desc); +mmap_action_ioremap_full (desc, pfn); +return io_remap_pfn_range_prepare (desc); }/**
@@ -3283,8 +3282,7 @@ int vm_iomap_memory(struct vm_area_strucunsigned long pfn;int err; - err =__simple_ioremap_prep (vm_start, vm_end, vma->vm_pgoff, start, - len, &pfn); + err =__simple_ioremap_prep (vm_len, vma->vm_pgoff, start, len, &pfn);if (err)return err;
mm/util.c renamed from mm/util.c~b +33 -29
132 lines not on this page · raw message
mm/vma.c renamed from mm/vma.c~b +8 -16
@@ -2732,7 +2732,8 @@ static bool can_set_ksm_flags_early(strureturn false; } -static int call_mapped_hook (struct vm_area_struct *vma) +static int call_mapped_hook (struct mmap_state *map, +struct vm_area_struct *vma) {const struct vm_operations_struct *vm_ops = vma->vm_ops;void *vm_private_data = vma->vm_private_data;
@@ -2743,6 +2744,9 @@ static int call_mapped_hook(struct vm_ar err = vm_ops->mapped (vma->vm_start, vma->vm_end, vma->vm_pgoff, vma->vm_file, &vm_private_data);if (err) { +if (map->hold_file_rmap_lock) +i_mmap_unlock_write (vma->vm_file->f_mapping); +unmap_vma_locked (vma);return err; }
@@ -2752,17 +2756,6 @@ static int call_mapped_hook(struct vm_arreturn 0 ; } -static void maybe_drop_file_rmap_lock (struct mmap_state *map, -struct vm_area_struct *vma) -{ -struct file *file; - -if (!map->hold_file_rmap_lock) -return ; - file = vma->vm_file; -i_mmap_unlock_write (file->f_mapping); -} -static unsigned long __mmap_region (struct file *file,unsigned long addr,unsigned long len,vma_flags_t vma_flags,unsigned long pgoff,struct list_head *uf)
@@ -2814,11 +2807,10 @@ static unsigned long __mmap_region(struc__mmap_complete (&map, vma);if (have_mmap_prepare && allocated_new) { - error =mmap_action_complete (vma, &desc.action); + error =mmap_action_complete (vma, &desc.action, + map.hold_file_rmap_lock);if (!error) - error =call_mapped_hook (vma); - -maybe_drop_file_rmap_lock (&map, vma); + error =call_mapped_hook (&map, vma);if (error)return error; }
tools/testing/vma/include/dup.h renamed from tools/testing/vma/include/dup.h~b +11 -8
@@ -470,13 +470,13 @@ struct mmap_action {pgprot_t pgprot; } remap;struct { -phys_addr_t start; -unsigned long len; +phys_addr_t start_phys_addr; +unsigned long size; } simple_ioremap;struct {unsigned long start;struct page **pages; -unsigned long num; +unsigned long nr_pages;pgoff_t pgoff; } map_kernel; };
@@ -648,7 +648,7 @@ struct vm_operations_struct {* upon first mapping a VMA. * Context: User context. May sleep. Caller holds mmap_lock. */ -void (*open)(struct vm_area_struct * area); +void (*open)(struct vm_area_struct *vma);/** * @close: Called when the VMA is being removed from the MM. * Context: User context. May sleep. Caller holds mmap_lock.
@@ -672,8 +672,8 @@ struct vm_operations_struct {int (*mapped)(unsigned long start,unsigned long end,pgoff_t pgoff,const struct file *file,void **vm_private_data);/* Called any time before splitting to check if it's allowed */ -int (*may_split)(struct vm_area_struct *area,unsigned long addr); -int (*mremap)(struct vm_area_struct *area); +int (*may_split)(struct vm_area_struct *vma,unsigned long addr); +int (*mremap)(struct vm_area_struct *vma);/* * Called by mprotect() to make driver-specific permission * checks before mprotect() is finalised. The VMA must not
@@ -685,7 +685,7 @@ struct vm_operations_struct {vm_fault_t (*huge_fault)(struct vm_fault *vmf,unsigned int order);vm_fault_t (*map_pages)(struct vm_fault *vmf,pgoff_t start_pgoff,pgoff_t end_pgoff); -unsigned long (*pagesize)(struct vm_area_struct * area); +unsigned long (*pagesize)(struct vm_area_struct *vma);/* notification that a previously read-only page is about to become * writable, if an error is returned it will cause a SIGBUS */
@@ -1288,6 +1288,8 @@ static inline void compat_set_desc_from_const struct file *file, const struct vm_area_struct *vma) { + memset(desc, 0, sizeof(*desc)); + desc->mm = vma->vm_mm; desc->file = (struct file *)file; desc->start = vma->vm_start;
@@ -1342,7 +1344,8 @@ static inline int __compat_vma_mmap(stru/* Update the VMA from the descriptor. */ compat_set_vma_from_desc (vma, desc);/* Complete any specified mmap actions. */ - err =mmap_action_complete (vma, &desc->action); + err =mmap_action_complete (vma, &desc->action, +/*rmap_lock_held=*/ false);if (err)return err;
tools/testing/vma/include/stubs.h renamed from tools/testing/vma/include/stubs.h~b +2 -1
@@ -87,7 +87,8 @@ static inline int mmap_action_prepare(st }static inline int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action) +struct mmap_action *action, +bool rmap_lock_held) {return 0 ; }
_
Re: [PATCH v3 01/16] mm: various small mmap_prepare cleanups
In reply to Lorenzo Stoakes (Oracle)Quoted 22 lines · Lorenzo Stoakes (Oracle)
Rather than passing arbitrary fields, pass a
vm_area_descpointer to mmap prepare functions to mmap prepare, and an action and vma pointer to mmap complete in order to put all the action-specific logic in the function actually doing the work.Additionally, allow mmap prepare functions to return an error so we can error out as soon as possible if there is something logically incorrect in the input.
Update
remap_pfn_range_prepare()to properly check the input range for the CoW case.Also remove
io_remap_pfn_range_complete(), as we can simply set up the fields correctly inio_remap_pfn_range_prepare()and useremap_pfn_range_complete()for this.While we're here, make
remap_pfn_range_prepare_vma()a little neater, and passmmap_actiondirectly tocall_action_complete().Then, update
compat_vma_mmap()to perform its logic directly, as__compat_vma_map()is not used by anything so we don't need to export it.
Quoted 6 lines
Also update
compat_vma_mmap()to usevfs_mmap_prepare()rather than calling themmap_prepareop directly.Finally, update the VMA userland tests to reflect the changes.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
Re: [PATCH v3 02/16] mm: add documentation for the mmap_prepare file operation callback
In reply to Lorenzo Stoakes (Oracle)Quoted 11 lines · Lorenzo Stoakes (Oracle)
This documentation makes it easier for a driver/file system implementer to correctly use this callback.
It covers the fundamentals, whilst intentionally leaving the less lovely possible actions one might take undocumented (for instance - the
success_hook,error_hookfields inmmap_action).The document also covers the new VMA flags implementation which is the only one which will work correctly with
mmap_prepare.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
Re: [PATCH v3 03/16] mm: document vm_operations_struct->open the same as close()
In reply to Lorenzo Stoakes (Oracle)Quoted 7 lines · Lorenzo Stoakes (Oracle)
Describe when the operation is invoked and the context in which it is invoked, matching the description already added for
vm_op->close().While we're here, update all outdated references to an 'area' field for VMAs to the more consistent 'vma'.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
Vlastimil Babka (SUSE)
reviewerRe: [PATCH v3 04/16] mm: add vm_ops->mapped hook
In reply to Lorenzo Stoakes (Oracle)Quoted 33 lines · Lorenzo Stoakes (Oracle)
Previously, when a driver needed to do something like establish a reference count, it could do so in the mmap hook in the knowledge that the mapping would succeed.
With the introduction of
f_op->mmap_preparethis is no longer the case, as it is invoked prior to actually establishing the mapping.
mmap_prepareis not appropriate for this kind of thing as it is called before any merge might take place, and after which an error might occur meaning resources could be leaked.To take this into account, introduce a new
vm_ops->mappedcallback which is invoked when the VMA is first mapped (though notably - not when it is merged - which is correct and mirrors existing mmap/open/close behaviour).We do better that
vm_ops->open()here, as this callback can return an error, at which point the VMA will be unmapped.Note that
vm_ops->mapped()is invoked after any mmap action is complete (such as I/O remapping).We intentionally do not expose the VMA at this point, exposing only the fields that could be used, and an output parameter in case the operation needs to update the
vma->vm_private_datafield.In order to deal with stacked filesystems which invoke inner filesystem's
mmap()invocations, add__compat_vma_mapped()and invoke it onvfs_mmap()(viacompat_vma_mmap()) to ensure that the mapped callback is handled when anmmap()caller invokes a nested filesystem'smmap_prepare()callback.We can now also remove
call_action_complete()and invokemmap_action_complete()directly, as we separate out the rmap lock logic.
Quoted 6 lines
The rmap lock logic, which was added in order to keep hugetlb working (!) to allow for the rmap lock to be held longer, needs to be propagated to the error paths on mmap complete and mapped hook error paths.
This is because
do_munmap()might otherwise deadlock with the rmap being held, so instead we unlock at the point of unmap.
Hmm but that was also true prior to this series? So is this a bugfix? Should it be a stable hotfix done outside of the series before the refactoring?
Quoted 159 lines
This is fine as any reliance on the rmap being held is irrelevant on error.
While we're here, refactor
mmap_action_finish()to avoid a big if (err) branch.We also abstract unmapping of a VMA on mmap action completion into its own helper function,
unmap_vma_locked().Update the
mmap_preparedocumentation to describe the mapped hook and make it clear what its intended use is.Additionally, update VMA userland test headers to reflect the change.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>--- Documentation/filesystems/mmap_prepare.rst | 15 +++ include/linux/fs.h | 9 +- include/linux/mm.h | 20 +++- mm/internal.h | 8 ++ mm/util.c | 129 ++++++++++++++------- mm/vma.c | 35 +++--- tools/testing/vma/include/dup.h | 27 ++++- tools/testing/vma/include/stubs.h | 3 +- 8 files changed, 186 insertions(+), 60 deletions(-)Documentation/filesystems/mmap_prepare.rst +15
-0@@ -25,6 +25,21 @@ That is - no resources should be allocated nor state updated to reflect that a mapping has been established, as the mapping may either be merged, or fail to be mapped after the callback is complete. +Mapped callback +--------------- + +If resources need to be allocated per-mapping, or state such as a reference +count needs to be manipulated, this should be done using the ``vm_ops->mapped`` +hook, which itself should be set by the >mmap_prepare hook. + +This callback is only invoked if a new mapping has been established and was not +merged with any other, and is invoked at a point where no error may occur before +the mapping is established. + +You may return an error to the callback itself, which will cause the mapping to +become unmapped and an error returned to the mmap() caller. This is useful if +resources need to be allocated, and that allocation might fail. + How To Use ==========include/linux/fs.h +8
-1@@ -2059,13 +2059,20 @@ static inline bool can_mmap_file(struct file *file) }int compat_vma_mmap (struct file *file,struct vm_area_struct *vma); +int __vma_check_mmap_hook (struct vm_area_struct *vma);static inline int vfs_mmap (struct file *file,struct vm_area_struct *vma) { +int err; +if (file->f_op->mmap_prepare)return compat_vma_mmap (file, vma); -return file->f_op->mmap (file, vma); + err = file->f_op->mmap (file, vma); +if (err) +return err; + +return __vma_check_mmap_hook (vma); }static inline int vfs_mmap_prepare (struct file *file,struct vm_area_desc *desc)include/linux/mm.h +19
-1@@ -777,6 +777,23 @@ struct vm_operations_struct {* Context: User context. May sleep. Caller holds mmap_lock. */ void (*close)(struct vm_area_struct *vma); +/** +* @mapped: Called when the VMA is first mapped in the MM. Not called if +* the new VMA is merged with an adjacent VMA. +* +* The @vm_private_data field is an output field allowing the user to +* modify vma->vm_private_data as necessary. +* +* ONLY valid if set from f_op->mmap_prepare. Will result in an error if +* set from f_op->mmap. +* +* Returns %0 on success, or an error otherwise. On error, the VMA will +* be unmapped. +* +* Context: User context. May sleep. Caller holds mmap_lock. +*/ +int (*mapped)(unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data);/* Called any time before splitting to check if it's allowed */ int (*may_split)(struct vm_area_struct *vma,unsigned long addr);int (*mremap)(struct vm_area_struct *vma);@@ -4327,7 +4344,8 @@ static inline void mmap_action_ioremap_full(struct vm_area_desc *desc,int mmap_action_prepare (struct vm_area_desc *desc);int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action); +struct mmap_action *action, +bool rmap_lock_held);/* Look up the first VMA which exactly match the interval vm_start ... vm_end */ static inline struct vm_area_struct *find_exact_vma(struct mm_struct *mm,mm/internal.h +8
-0@@ -202,6 +202,14 @@ static inline void vma_close(struct vm_area_struct *vma)/* unmap_vmas is in mm/memory.c */ void unmap_vmas (struct mmu_gather *tlb,struct unmap_desc *unmap); +static inline void unmap_vma_locked (struct vm_area_struct *vma) +{ +const size_t len =vma_pages (vma) <<PAGE_SHIFT ; + +mmap_assert_write_locked (vma->vm_mm); +do_munmap (vma->vm_mm, vma->vm_start, len,NULL ); +} +#ifdef CONFIG_MMU static inline void get_anon_vma (struct anon_vma *anon_vma)mm/util.c +17
-0@@ -1163,6 +1163,54 @@ void flush_dcache_folio(struct folio *folio)EXPORT_SYMBOL (flush_dcache_folio);#endif +static int __compat_vma_mmap (struct file *file,struct vm_area_struct *vma) +{ +struct vm_area_desc desc = { + .mm = vma->vm_mm, + .file = file, + .start = vma->vm_start, + .end = vma->vm_end, + + .pgoff = vma->vm_pgoff, + .vm_file = vma->vm_file, + .vma_flags = vma->flags, + .page_prot = vma->vm_page_prot, + + .action.type =MMAP_NOTHING ,/* Default */ + }; +int err; +
mm/util.c void flush_dcache_folio(struct folio *folio) · line 1192
Quoted 10 lines
mm/util.c +10
-0+ err =vfs_mmap_prepare (file, &desc); +if (err) +return err; + + err =mmap_action_prepare (&desc); +if (err) +return err; + +set_vma_from_desc (vma, &desc); +return mmap_action_complete (vma, &desc.action,/*rmap_lock_held=*/ false);
Patch 1 removed this function and this one reinstates it with some modifications. Could patch 1 only remove the export and otherwise both do only the necessary modifications?
Quoted 72 lines
mm/util.c +33
-20+} + +static int __compat_vma_mapped (struct file *file,struct vm_area_struct *vma) +{ +const struct vm_operations_struct *vm_ops = vma->vm_ops; +void *vm_private_data = vma->vm_private_data; +int err; + +if (!vm_ops || !vm_ops->mapped) +return 0 ; + + err = vm_ops->mapped (vma->vm_start, vma->vm_end, vma->vm_pgoff, file, + &vm_private_data); +if (err) +unmap_vma_locked (vma); +else if (vm_private_data != vma->vm_private_data) + vma->vm_private_data = vm_private_data; + +return err; +} +/** * compat_vma_mmap() - Apply the file's .mmap_prepare() hook to an * existing VMA and execute any requested actions. @@ -1191,34 +1239,26 @@ EXPORT_SYMBOL(flush_dcache_folio);*/ int compat_vma_mmap (struct file *file,struct vm_area_struct *vma) { -struct vm_area_desc desc = { - .mm = vma->vm_mm, - .file = file, - .start = vma->vm_start, - .end = vma->vm_end, - - .pgoff = vma->vm_pgoff, - .vm_file = vma->vm_file, - .vma_flags = vma->flags, - .page_prot = vma->vm_page_prot, - - .action.type =MMAP_NOTHING ,/* Default */ - };int err; - err =vfs_mmap_prepare (file, &desc); + err =__compat_vma_mmap (file, vma);if (err)return err; - err =mmap_action_prepare (&desc); -if (err) -return err; - -set_vma_from_desc (vma, &desc); -return mmap_action_complete (vma, &desc.action); +return __compat_vma_mapped (file, vma); }EXPORT_SYMBOL (compat_vma_mmap); +int __vma_check_mmap_hook (struct vm_area_struct *vma) +{ +/* vm_ops->mapped is not valid if mmap() is specified. */ +if (vma->vm_ops &&WARN_ON_ONCE (vma->vm_ops->mapped)) +return -EINVAL ; + +return 0 ; +} +EXPORT_SYMBOL (__vma_check_mmap_hook); +static void set_ps_flags (struct page_snapshot *ps,const struct folio *folio,const struct page *page) {
mm/util.c void snapshot_page(struct page_snapshot *ps, const struct page *page) · line 1355
Quoted 9 lines
mm/util.c +4
-1@@ -1308,32 +1348,31 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page) }static int mmap_action_finish (struct vm_area_struct *vma, -struct mmap_action *action,int err) +struct mmap_action *action,int err, +bool rmap_lock_held) { +if (rmap_lock_held) +i_mmap_unlock_write (vma->vm_file->f_mapping);
Should this be moved below the if (!err) ?
Otherwise I think we unlock prematurely, and can even try to unlock twice - here and in call_mapped_hook(). And we want to unlock only if we're about to munmap, right?
Quoted 132 lines
mm/util.c +22
-22+ +if (!err) { +if (action->success_hook) +return action->success_hook (vma); +return 0 ; + } +/* * If an error occurs, unmap the VMA altogether and return an error. We * only clear the newly allocated VMA, since this function is only * invoked if we do NOT merge, so we only clean up the VMA we created. */ -if (err) { -const size_t len =vma_pages (vma) <<PAGE_SHIFT ; - -do_munmap (current->mm, vma->vm_start, len,NULL ); - -if (action->error_hook) { -/* We may want to filter the error. */ - err = action->error_hook (err); - -/* The caller should not clear the error. */ -VM_WARN_ON_ONCE (!err); - } -return err; +unmap_vma_locked (vma); +if (action->error_hook) { +/* We may want to filter the error. */ + err = action->error_hook (err); +/* The caller should not clear the error. */ +VM_WARN_ON_ONCE (!err); } - -if (action->success_hook) -return action->success_hook (vma); - -return 0 ; +return err; }#ifdef CONFIG_MMU @@ -1364,13 +1403,15 @@ EXPORT_SYMBOL(mmap_action_prepare);* mmap_action_complete - Execute VMA descriptor action. * @vma: The VMA to perform the action upon. * @action: The action to perform. +* @rmap_lock_held: Is the file rmap lock held? * * Similar to mmap_action_prepare(). * * Return: 0 on success, or error, at which point the VMA will be unmapped. */ int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action) +struct mmap_action *action, +bool rmap_lock_held) {int err =0 ;@@ -1388,7 +1429,8 @@ int mmap_action_complete(struct vm_area_struct *vma, break; } -return mmap_action_finish (vma, action, err); +return mmap_action_finish (vma, action, err, + rmap_lock_held); }EXPORT_SYMBOL (mmap_action_complete);#else @@ -1408,7 +1450,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)EXPORT_SYMBOL (mmap_action_prepare);int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action) +struct mmap_action *action, +bool rmap_lock_held) {int err =0 ;@@ -1423,7 +1466,7 @@ int mmap_action_complete(struct vm_area_struct *vma, break; } -return mmap_action_finish (vma, action, err); +return mmap_action_finish (vma, action, err, rmap_lock_held); }EXPORT_SYMBOL (mmap_action_complete);#endif mm/vma.c +18
-11@@ -2731,21 +2731,28 @@ static bool can_set_ksm_flags_early(struct mmap_state *map)return false; } -static int call_action_complete (struct mmap_state *map, -struct mmap_action *action, -struct vm_area_struct *vma) +static int call_mapped_hook (struct mmap_state *map, +struct vm_area_struct *vma) { -int ret; - - ret =mmap_action_complete (vma, action); +const struct vm_operations_struct *vm_ops = vma->vm_ops; +void *vm_private_data = vma->vm_private_data; +int err; -/* If we held the file rmap we need to release it. */ -if (map->hold_file_rmap_lock) { -struct file *file = vma->vm_file; +if (!vm_ops || !vm_ops->mapped) +return 0 ; + err = vm_ops->mapped (vma->vm_start, vma->vm_end, vma->vm_pgoff, + vma->vm_file, &vm_private_data); +if (err) { +if (map->hold_file_rmap_lock) +i_mmap_unlock_write (vma->vm_file->f_mapping); -i_mmap_unlock_write (file->f_mapping); +unmap_vma_locked (vma); +return err; } -return ret; +/* Update private data if changed. */ +if (vm_private_data != vma->vm_private_data) + vma->vm_private_data = vm_private_data; +return 0 ; } static unsigned long __mmap_region(struct file *file, unsigned long addr,
mm/vma.c static unsigned long __mmap_region(struct file *file, unsigned long addr, · line 2812
Quoted 9 lines
mm/vma.c +4
-2@@ -2799,8 +2806,10 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,__mmap_complete (&map, vma);if (have_mmap_prepare && allocated_new) { - error =call_action_complete (&map, &desc.action, vma); - + error =mmap_action_complete (vma, &desc.action, + map.hold_file_rmap_lock); +if (!error) + error =call_mapped_hook (&map, vma);
And if neither of those above end up doing i_mmap_unlock_write(), we should do it here? I think currently the misplaced unlock in mmap_action_finish() masks the lack of it here, otherwise bots would already notice. Loss of locking coverage (due to premature unlock) or the risk of double unlock is probably harder to trigger. Or maybe a later patch in the series happens to fix the issues, so it's just a bisection hazard here. Or I'm completely wrong.
Quoted 66 lines
mm/vma.c +0
-0if (error)return error; }tools/testing/vma/include/dup.h +26
-1@@ -643,6 +643,23 @@ struct vm_operations_struct {* Context: User context. May sleep. Caller holds mmap_lock. */ void (*close)(struct vm_area_struct *vma); +/** +* @mapped: Called when the VMA is first mapped in the MM. Not called if +* the new VMA is merged with an adjacent VMA. +* +* The @vm_private_data field is an output field allowing the user to +* modify vma->vm_private_data as necessary. +* +* ONLY valid if set from f_op->mmap_prepare. Will result in an error if +* set from f_op->mmap. +* +* Returns %0 on success, or an error otherwise. On error, the VMA will +* be unmapped. +* +* Context: User context. May sleep. Caller holds mmap_lock. +*/ +int (*mapped)(unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data);/* Called any time before splitting to check if it's allowed */ int (*may_split)(struct vm_area_struct *vma,unsigned long addr);int (*mremap)(struct vm_area_struct *vma);@@ -1281,7 +1298,7 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,return err;set_vma_from_desc (vma, &desc); -return mmap_action_complete (vma, &desc.action); +return mmap_action_complete (vma, &desc.action,/*rmap_lock_held=*/ false); } static inline int compat_vma_mmap(struct file *file,@@ -1500,3 +1517,11 @@ static inline pgprot_t vma_get_page_prot(vma_flags_t vma_flags)return vm_get_page_prot (vm_flags); } + +static inline void unmap_vma_locked (struct vm_area_struct *vma) +{ +const size_t len =vma_pages (vma) <<PAGE_SHIFT ; + +mmap_assert_write_locked (vma->vm_mm); +do_munmap (vma->vm_mm, vma->vm_start, len,NULL ); +}tools/testing/vma/include/stubs.h +2
-1@@ -87,7 +87,8 @@ static inline int mmap_action_prepare(struct vm_area_desc *desc) }static inline int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action) +struct mmap_action *action, +bool rmap_lock_held) {return 0 ; }
Vlastimil Babka (SUSE)
reviewerRe: [PATCH v3 05/16] fs: afs: correctly drop reference count on mapping failure
In reply to Lorenzo Stoakes (Oracle)Quoted 7 lines · Lorenzo Stoakes (Oracle)
Commit
9d5403b1036c("fs: convert most other generic_file_*mmap() users to .mmap_prepare()") updated AFS to use themmap_preparecallback in favour of the deprecated mmap callback.However, it did not account for the fact that
mmap_prepareis called pre-merge, and may then be merged, nor thatmmap_preparecan fail to map due to an out of memory error.
So that means a file can become pinned forever? OOM is probably only a problem with fault injection in practice, but the merge case can happen. And 9d5403b1036c is pre-6.18 LTS. Are we going to need Fixes: and Cc: stable then?
Quoted 96 lines
Both of those are cases in which we should not be incrementing a reference count.
With the newly added
vm_ops->mappedcallback available, we can simply defer this operation to that callback which is only invoked once the mapping is successfully in place (but not yet visible to userspace as the mmap and VMA write locks are held).Therefore add
afs_mapped()to implement this callback for AFS, and remove the code doing so inafs_mmap_prepare().Also update
afs_vm_open(),afs_vm_close()andafs_vm_map_pages()to be consistent in how the vnode is accessed.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>--- fs/afs/file.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-)fs/afs/file.c +26
-10@@ -28,6 +28,8 @@ static ssize_t afs_file_splice_read(struct file *in, loff_t *ppos,static void afs_vm_open (struct vm_area_struct *area);static void afs_vm_close (struct vm_area_struct *area);static vm_fault_t afs_vm_map_pages (struct vm_fault *vmf,pgoff_t start_pgoff,pgoff_t end_pgoff); +static int afs_mapped (unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data);const struct file_operations afs_file_operations = { .open = afs_open,@@ -61,6 +63,7 @@ const struct address_space_operations afs_file_aops = { };static const struct vm_operations_struct afs_vm_ops = { + .mapped = afs_mapped, .open = afs_vm_open, .close = afs_vm_close, .fault = filemap_fault,@@ -494,32 +497,45 @@ static void afs_drop_open_mmap(struct afs_vnode *vnode)*/ static int afs_file_mmap_prepare (struct vm_area_desc *desc) { -struct afs_vnode *vnode =AFS_FS_I (file_inode (desc->file));int ret; -afs_add_open_mmap (vnode); - ret =generic_file_mmap_prepare (desc); -if (ret ==0 ) - desc->vm_ops = &afs_vm_ops; -else -afs_drop_open_mmap (vnode); +if (ret) +return ret; + + desc->vm_ops = &afs_vm_ops;return ret; } +static int afs_mapped (unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data) +{ +struct afs_vnode *vnode =AFS_FS_I (file_inode (file)); + +afs_add_open_mmap (vnode); +return 0 ; +} +static void afs_vm_open (struct vm_area_struct *vma) { -afs_add_open_mmap (AFS_FS_I (file_inode (vma->vm_file))); +struct file *file = vma->vm_file; +struct afs_vnode *vnode =AFS_FS_I (file_inode (file)); + +afs_add_open_mmap (vnode); }static void afs_vm_close (struct vm_area_struct *vma) { -afs_drop_open_mmap (AFS_FS_I (file_inode (vma->vm_file))); +struct file *file = vma->vm_file; +struct afs_vnode *vnode =AFS_FS_I (file_inode (file)); + +afs_drop_open_mmap (vnode); }static vm_fault_t afs_vm_map_pages (struct vm_fault *vmf,pgoff_t start_pgoff,pgoff_t end_pgoff) { -struct afs_vnode *vnode =AFS_FS_I (file_inode (vmf->vma->vm_file)); +struct file *file = vmf->vma->vm_file; +struct afs_vnode *vnode =AFS_FS_I (file_inode (file));if (afs_check_validity (vnode))return filemap_map_pages (vmf, start_pgoff, end_pgoff);
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH v3 05/16] fs: afs: correctly drop reference count on mapping failure
Quoted 5 lines · Vlastimil Babka (SUSE)
Quoted 4 lines · Lorenzo Stoakes (Oracle)
Commit
9d5403b1036c("fs: convert most other generic_file_*mmap() users to .mmap_prepare()") updated AFS to use themmap_preparecallback in favour of the deprecated mmap callback.
Quoted 7 lines
Quoted 3 lines
However, it did not account for the fact that
mmap_prepareis called pre-merge, and may then be merged, nor thatmmap_preparecan fail to map due to an out of memory error.So that means a file can become pinned forever? OOM is probably only a problem with fault injection in practice, but the merge case can happen. And
9d5403b1036cis pre-6.18 LTS. Are we going to need Fixes: and Cc: stable then?
That'd require backporting all of the .mapped functionality and half of this series, I don't think that's really practical.
I guess I can do a manual backport of a partial revert.
Thanks, Lorenzo
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH v3 04/16] mm: add vm_ops->mapped hook
In reply to Vlastimil Babka (SUSE)Quoted 209 lines · Vlastimil Babka (SUSE)
Quoted 33 lines · Lorenzo Stoakes (Oracle)
Previously, when a driver needed to do something like establish a reference count, it could do so in the mmap hook in the knowledge that the mapping would succeed.
With the introduction of
f_op->mmap_preparethis is no longer the case, as it is invoked prior to actually establishing the mapping.
mmap_prepareis not appropriate for this kind of thing as it is called before any merge might take place, and after which an error might occur meaning resources could be leaked.To take this into account, introduce a new
vm_ops->mappedcallback which is invoked when the VMA is first mapped (though notably - not when it is merged - which is correct and mirrors existing mmap/open/close behaviour).We do better that
vm_ops->open()here, as this callback can return an error, at which point the VMA will be unmapped.Note that
vm_ops->mapped()is invoked after any mmap action is complete (such as I/O remapping).We intentionally do not expose the VMA at this point, exposing only the fields that could be used, and an output parameter in case the operation needs to update the
vma->vm_private_datafield.In order to deal with stacked filesystems which invoke inner filesystem's
mmap()invocations, add__compat_vma_mapped()and invoke it onvfs_mmap()(viacompat_vma_mmap()) to ensure that the mapped callback is handled when anmmap()caller invokes a nested filesystem'smmap_prepare()callback.We can now also remove
call_action_complete()and invokemmap_action_complete()directly, as we separate out the rmap lock logic.Quoted 6 lines
The rmap lock logic, which was added in order to keep hugetlb working (!) to allow for the rmap lock to be held longer, needs to be propagated to the error paths on mmap complete and mapped hook error paths.
This is because
do_munmap()might otherwise deadlock with the rmap being held, so instead we unlock at the point of unmap.Hmm but that was also true prior to this series? So is this a bugfix? Should it be a stable hotfix done outside of the series before the refactoring?
Quoted 165 lines
This is fine as any reliance on the rmap being held is irrelevant on error.
While we're here, refactor
mmap_action_finish()to avoid a big if (err) branch.We also abstract unmapping of a VMA on mmap action completion into its own helper function,
unmap_vma_locked().Update the
mmap_preparedocumentation to describe the mapped hook and make it clear what its intended use is.Additionally, update VMA userland test headers to reflect the change.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>--- Documentation/filesystems/mmap_prepare.rst | 15 +++ include/linux/fs.h | 9 +- include/linux/mm.h | 20 +++- mm/internal.h | 8 ++ mm/util.c | 129 ++++++++++++++------- mm/vma.c | 35 +++--- tools/testing/vma/include/dup.h | 27 ++++- tools/testing/vma/include/stubs.h | 3 +- 8 files changed, 186 insertions(+), 60 deletions(-)Documentation/filesystems/mmap_prepare.rst +15
-0@@ -25,6 +25,21 @@ That is - no resources should be allocated nor state updated to reflect that a mapping has been established, as the mapping may either be merged, or fail to be mapped after the callback is complete. +Mapped callback +--------------- + +If resources need to be allocated per-mapping, or state such as a reference +count needs to be manipulated, this should be done using the ``vm_ops->mapped`` +hook, which itself should be set by the >mmap_prepare hook. + +This callback is only invoked if a new mapping has been established and was not +merged with any other, and is invoked at a point where no error may occur before +the mapping is established. + +You may return an error to the callback itself, which will cause the mapping to +become unmapped and an error returned to the mmap() caller. This is useful if +resources need to be allocated, and that allocation might fail. + How To Use ==========include/linux/fs.h +8
-1@@ -2059,13 +2059,20 @@ static inline bool can_mmap_file(struct file *file) }int compat_vma_mmap (struct file *file,struct vm_area_struct *vma); +int __vma_check_mmap_hook (struct vm_area_struct *vma);static inline int vfs_mmap (struct file *file,struct vm_area_struct *vma) { +int err; +if (file->f_op->mmap_prepare)return compat_vma_mmap (file, vma); -return file->f_op->mmap (file, vma); + err = file->f_op->mmap (file, vma); +if (err) +return err; + +return __vma_check_mmap_hook (vma); }static inline int vfs_mmap_prepare (struct file *file,struct vm_area_desc *desc)include/linux/mm.h +19
-1@@ -777,6 +777,23 @@ struct vm_operations_struct {* Context: User context. May sleep. Caller holds mmap_lock. */ void (*close)(struct vm_area_struct *vma); +/** +* @mapped: Called when the VMA is first mapped in the MM. Not called if +* the new VMA is merged with an adjacent VMA. +* +* The @vm_private_data field is an output field allowing the user to +* modify vma->vm_private_data as necessary. +* +* ONLY valid if set from f_op->mmap_prepare. Will result in an error if +* set from f_op->mmap. +* +* Returns %0 on success, or an error otherwise. On error, the VMA will +* be unmapped. +* +* Context: User context. May sleep. Caller holds mmap_lock. +*/ +int (*mapped)(unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data);/* Called any time before splitting to check if it's allowed */ int (*may_split)(struct vm_area_struct *vma,unsigned long addr);int (*mremap)(struct vm_area_struct *vma);@@ -4327,7 +4344,8 @@ static inline void mmap_action_ioremap_full(struct vm_area_desc *desc,int mmap_action_prepare (struct vm_area_desc *desc);int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action); +struct mmap_action *action, +bool rmap_lock_held);/* Look up the first VMA which exactly match the interval vm_start ... vm_end */ static inline struct vm_area_struct *find_exact_vma(struct mm_struct *mm,mm/internal.h +8
-0@@ -202,6 +202,14 @@ static inline void vma_close(struct vm_area_struct *vma)/* unmap_vmas is in mm/memory.c */ void unmap_vmas (struct mmu_gather *tlb,struct unmap_desc *unmap); +static inline void unmap_vma_locked (struct vm_area_struct *vma) +{ +const size_t len =vma_pages (vma) <<PAGE_SHIFT ; + +mmap_assert_write_locked (vma->vm_mm); +do_munmap (vma->vm_mm, vma->vm_start, len,NULL ); +} +#ifdef CONFIG_MMU static inline void get_anon_vma (struct anon_vma *anon_vma)mm/util.c +23
-0@@ -1163,6 +1163,54 @@ void flush_dcache_folio(struct folio *folio)EXPORT_SYMBOL (flush_dcache_folio);#endif +static int __compat_vma_mmap (struct file *file,struct vm_area_struct *vma) +{ +struct vm_area_desc desc = { + .mm = vma->vm_mm, + .file = file, + .start = vma->vm_start, + .end = vma->vm_end, + + .pgoff = vma->vm_pgoff, + .vm_file = vma->vm_file, + .vma_flags = vma->flags, + .page_prot = vma->vm_page_prot, + + .action.type =MMAP_NOTHING ,/* Default */ + }; +int err; + + err =vfs_mmap_prepare (file, &desc); +if (err) +return err; + + err =mmap_action_prepare (&desc); +if (err)
Quoted 8 lines
mm/util.c void flush_dcache_folio(struct folio *folio) · line 1192
Quoted 4 lines
mm/util.c +4
-0+return err; + +set_vma_from_desc (vma, &desc); +return mmap_action_complete (vma, &desc.action,/*rmap_lock_held=*/ false);Patch 1 removed this function and this one reinstates it with some modifications. Could patch 1 only remove the export and otherwise both do only the necessary modifications?
No, I'd rather do it this way because the function is now doing something different and for different reasons.
Let's not just churn respins for no reason. If it's not breaking anything let's just leave it as is please.
Quoted 75 lines
Quoted 75 lines
mm/util.c +33
-20+} + +static int __compat_vma_mapped (struct file *file,struct vm_area_struct *vma) +{ +const struct vm_operations_struct *vm_ops = vma->vm_ops; +void *vm_private_data = vma->vm_private_data; +int err; + +if (!vm_ops || !vm_ops->mapped) +return 0 ; + + err = vm_ops->mapped (vma->vm_start, vma->vm_end, vma->vm_pgoff, file, + &vm_private_data); +if (err) +unmap_vma_locked (vma); +else if (vm_private_data != vma->vm_private_data) + vma->vm_private_data = vm_private_data; + +return err; +} +/** * compat_vma_mmap() - Apply the file's .mmap_prepare() hook to an * existing VMA and execute any requested actions. @@ -1191,34 +1239,26 @@ EXPORT_SYMBOL(flush_dcache_folio);*/ int compat_vma_mmap (struct file *file,struct vm_area_struct *vma) { -struct vm_area_desc desc = { - .mm = vma->vm_mm, - .file = file, - .start = vma->vm_start, - .end = vma->vm_end, - - .pgoff = vma->vm_pgoff, - .vm_file = vma->vm_file, - .vma_flags = vma->flags, - .page_prot = vma->vm_page_prot, - - .action.type =MMAP_NOTHING ,/* Default */ - };int err; - err =vfs_mmap_prepare (file, &desc); + err =__compat_vma_mmap (file, vma);if (err)return err; - err =mmap_action_prepare (&desc); -if (err) -return err; - -set_vma_from_desc (vma, &desc); -return mmap_action_complete (vma, &desc.action); +return __compat_vma_mapped (file, vma); }EXPORT_SYMBOL (compat_vma_mmap); +int __vma_check_mmap_hook (struct vm_area_struct *vma) +{ +/* vm_ops->mapped is not valid if mmap() is specified. */ +if (vma->vm_ops &&WARN_ON_ONCE (vma->vm_ops->mapped)) +return -EINVAL ; + +return 0 ; +} +EXPORT_SYMBOL (__vma_check_mmap_hook); +static void set_ps_flags (struct page_snapshot *ps,const struct folio *folio,const struct page *page) {@@ -1308,32 +1348,31 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page) }
Quoted 9 lines
mm/util.c void snapshot_page(struct page_snapshot *ps, const struct page *page) · line 1355
Quoted 7 lines
mm/util.c +4
-1static int mmap_action_finish (struct vm_area_struct *vma, -struct mmap_action *action,int err) +struct mmap_action *action,int err, +bool rmap_lock_held) { +if (rmap_lock_held) +i_mmap_unlock_write (vma->vm_file->f_mapping);Should this be moved below the if (!err) ?
Yup sigh, the success hook needs this hack to be held.
This is all because hugetlb does stupid things.
The one case in which it uses generic hooks rather than stupid 'if (hugetlb) { ... }' branches, and it does stuff it shouldn't (of course! :)
Quoted 4 lines
Otherwise I think we unlock prematurely, and can even try to unlock twice - here and in
call_mapped_hook(). And we want to unlock only if we're about to munmap, right?
No, we want to unlock at the end of the operation regardless. But this can unlock twice because I'm not threading this crap through enough clearly.
Let me rethink this I guess. I hate hugetlb.
Quoted 135 lines
Quoted 135 lines
mm/util.c +22
-22+ +if (!err) { +if (action->success_hook) +return action->success_hook (vma); +return 0 ; + } +/* * If an error occurs, unmap the VMA altogether and return an error. We * only clear the newly allocated VMA, since this function is only * invoked if we do NOT merge, so we only clean up the VMA we created. */ -if (err) { -const size_t len =vma_pages (vma) <<PAGE_SHIFT ; - -do_munmap (current->mm, vma->vm_start, len,NULL ); - -if (action->error_hook) { -/* We may want to filter the error. */ - err = action->error_hook (err); - -/* The caller should not clear the error. */ -VM_WARN_ON_ONCE (!err); - } -return err; +unmap_vma_locked (vma); +if (action->error_hook) { +/* We may want to filter the error. */ + err = action->error_hook (err); +/* The caller should not clear the error. */ +VM_WARN_ON_ONCE (!err); } - -if (action->success_hook) -return action->success_hook (vma); - -return 0 ; +return err; }#ifdef CONFIG_MMU @@ -1364,13 +1403,15 @@ EXPORT_SYMBOL(mmap_action_prepare);* mmap_action_complete - Execute VMA descriptor action. * @vma: The VMA to perform the action upon. * @action: The action to perform. +* @rmap_lock_held: Is the file rmap lock held? * * Similar to mmap_action_prepare(). * * Return: 0 on success, or error, at which point the VMA will be unmapped. */ int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action) +struct mmap_action *action, +bool rmap_lock_held) {int err =0 ;@@ -1388,7 +1429,8 @@ int mmap_action_complete(struct vm_area_struct *vma, break; } -return mmap_action_finish (vma, action, err); +return mmap_action_finish (vma, action, err, + rmap_lock_held); }EXPORT_SYMBOL (mmap_action_complete);#else @@ -1408,7 +1450,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)EXPORT_SYMBOL (mmap_action_prepare);int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action) +struct mmap_action *action, +bool rmap_lock_held) {int err =0 ;@@ -1423,7 +1466,7 @@ int mmap_action_complete(struct vm_area_struct *vma, break; } -return mmap_action_finish (vma, action, err); +return mmap_action_finish (vma, action, err, rmap_lock_held); }EXPORT_SYMBOL (mmap_action_complete);#endif mm/vma.c +18
-11@@ -2731,21 +2731,28 @@ static bool can_set_ksm_flags_early(struct mmap_state *map)return false; } -static int call_action_complete (struct mmap_state *map, -struct mmap_action *action, -struct vm_area_struct *vma) +static int call_mapped_hook (struct mmap_state *map, +struct vm_area_struct *vma) { -int ret; - - ret =mmap_action_complete (vma, action); +const struct vm_operations_struct *vm_ops = vma->vm_ops; +void *vm_private_data = vma->vm_private_data; +int err; -/* If we held the file rmap we need to release it. */ -if (map->hold_file_rmap_lock) { -struct file *file = vma->vm_file; +if (!vm_ops || !vm_ops->mapped) +return 0 ; + err = vm_ops->mapped (vma->vm_start, vma->vm_end, vma->vm_pgoff, + vma->vm_file, &vm_private_data); +if (err) { +if (map->hold_file_rmap_lock) +i_mmap_unlock_write (vma->vm_file->f_mapping); -i_mmap_unlock_write (file->f_mapping); +unmap_vma_locked (vma); +return err; } -return ret; +/* Update private data if changed. */ +if (vm_private_data != vma->vm_private_data) + vma->vm_private_data = vm_private_data; +return 0 ; } static unsigned long __mmap_region(struct file *file, unsigned long addr,@@ -2799,8 +2806,10 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,__mmap_complete (&map, vma);
Quoted 10 lines
mm/vma.c static unsigned long __mmap_region(struct file *file, unsigned long addr, · line 2812
Quoted 7 lines
mm/vma.c +4
-2if (have_mmap_prepare && allocated_new) { - error =call_action_complete (&map, &desc.action, vma); - + error =mmap_action_complete (vma, &desc.action, + map.hold_file_rmap_lock); +if (!error) + error =call_mapped_hook (&map, vma);And if neither of those above end up doing
i_mmap_unlock_write(), we should do it here? I think currently the misplaced unlock inmmap_action_finish()
I don't think it's misplaced, I think it's a much better place for it than here, and otherwise we have yet more hacks upon hacks threaded throughout the code.
We don't actually need the hugetlb hack to hold the lock over .mapped but I guess we should do that anyway just for the sake of consistency.
But obviously call_mapped_hook() is trying to unlock anyway.
Quoted 1 line
masks the lack of it here, otherwise bots would already notice. Loss of
And AI... it would require hitting a .mapped error I guess.
Quoted 3 lines
locking coverage (due to premature unlock) or the risk of double unlock is probably harder to trigger. Or maybe a later patch in the series happens to fix the issues, so it's just a bisection hazard here. Or I'm completely wrong.
Nope I messed it up, will fix.
Quoted 60 lines
Quoted 60 lines
mm/vma.c +0
-0if (error)return error; }tools/testing/vma/include/dup.h +26
-1@@ -643,6 +643,23 @@ struct vm_operations_struct {* Context: User context. May sleep. Caller holds mmap_lock. */ void (*close)(struct vm_area_struct *vma); +/** +* @mapped: Called when the VMA is first mapped in the MM. Not called if +* the new VMA is merged with an adjacent VMA. +* +* The @vm_private_data field is an output field allowing the user to +* modify vma->vm_private_data as necessary. +* +* ONLY valid if set from f_op->mmap_prepare. Will result in an error if +* set from f_op->mmap. +* +* Returns %0 on success, or an error otherwise. On error, the VMA will +* be unmapped. +* +* Context: User context. May sleep. Caller holds mmap_lock. +*/ +int (*mapped)(unsigned long start,unsigned long end,pgoff_t pgoff, +const struct file *file,void **vm_private_data);/* Called any time before splitting to check if it's allowed */ int (*may_split)(struct vm_area_struct *vma,unsigned long addr);int (*mremap)(struct vm_area_struct *vma);@@ -1281,7 +1298,7 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,return err;set_vma_from_desc (vma, &desc); -return mmap_action_complete (vma, &desc.action); +return mmap_action_complete (vma, &desc.action,/*rmap_lock_held=*/ false); } static inline int compat_vma_mmap(struct file *file,@@ -1500,3 +1517,11 @@ static inline pgprot_t vma_get_page_prot(vma_flags_t vma_flags)return vm_get_page_prot (vm_flags); } + +static inline void unmap_vma_locked (struct vm_area_struct *vma) +{ +const size_t len =vma_pages (vma) <<PAGE_SHIFT ; + +mmap_assert_write_locked (vma->vm_mm); +do_munmap (vma->vm_mm, vma->vm_start, len,NULL ); +}tools/testing/vma/include/stubs.h +0
-0@@ -87,7 +87,8 @@ static inline int mmap_action_prepare(struct vm_area_desc *desc) }
Quoted 8 lines
tools/testing/vma/include/stubs.h static inline int mmap_action_prepare(struct vm_area_desc *desc) · line 94
Quoted 7 lines
tools/testing/vma/include/stubs.h +2
-1static inline int mmap_action_complete (struct vm_area_struct *vma, -struct mmap_action *action) +struct mmap_action *action, +bool rmap_lock_held) {return 0 ; }
Thanks, Lorenzo
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH v3 04/16] mm: add vm_ops->mapped hook
In reply to Vlastimil Babka (SUSE)Quoted 38 lines · Vlastimil Babka (SUSE)
Quoted 37 lines · Lorenzo Stoakes (Oracle)
Previously, when a driver needed to do something like establish a reference count, it could do so in the mmap hook in the knowledge that the mapping would succeed.
With the introduction of
f_op->mmap_preparethis is no longer the case, as it is invoked prior to actually establishing the mapping.
mmap_prepareis not appropriate for this kind of thing as it is called before any merge might take place, and after which an error might occur meaning resources could be leaked.To take this into account, introduce a new
vm_ops->mappedcallback which is invoked when the VMA is first mapped (though notably - not when it is merged - which is correct and mirrors existing mmap/open/close behaviour).We do better that
vm_ops->open()here, as this callback can return an error, at which point the VMA will be unmapped.Note that
vm_ops->mapped()is invoked after any mmap action is complete (such as I/O remapping).We intentionally do not expose the VMA at this point, exposing only the fields that could be used, and an output parameter in case the operation needs to update the
vma->vm_private_datafield.In order to deal with stacked filesystems which invoke inner filesystem's
mmap()invocations, add__compat_vma_mapped()and invoke it onvfs_mmap()(viacompat_vma_mmap()) to ensure that the mapped callback is handled when anmmap()caller invokes a nested filesystem'smmap_prepare()callback.We can now also remove
call_action_complete()and invokemmap_action_complete()directly, as we separate out the rmap lock logic.The rmap lock logic, which was added in order to keep hugetlb working (!) to allow for the rmap lock to be held longer, needs to be propagated to the error paths on mmap complete and mapped hook error paths.
Quoted 5 lines
Quoted 2 lines
This is because
do_munmap()might otherwise deadlock with the rmap being held, so instead we unlock at the point of unmap.Hmm but that was also true prior to this series? So is this a bugfix? Should it be a stable hotfix done outside of the series before the refactoring?
Yup, will send a hotfix.
Thanks, Lorenzo
Re: [PATCH v3 15/16] mm: add mmap_action_map_kernel_pages[_full]()
In reply to Lorenzo Stoakes (Oracle)Hi Lorenzo,
Quoted 24 lines · Lorenzo Stoakes (Oracle)
A user can invoke
mmap_action_map_kernel_pages()to specify that the mapping should map kernel pages starting fromdesc->startof a specified number of pages specified in an array.In order to implement this, adjust
mmap_action_prepare()to be able to return an error code, as it makes sense to assert that the specified parameters are valid as quickly as possible as well as updating the VMA flags to includeVMA_MIXEDMAP_BITas necessary.This provides an
mmap_prepareequivalent ofvm_insert_pages(). We additionally update the existingvm_insert_pages()code to userange_in_vma()and add a newrange_in_vma_desc()helper function for themmap_preparecase, sharing the code between the two inrange_is_subset().We add both
mmap_action_map_kernel_pages()andmmap_action_map_kernel_pages_full()to allow for both partial and full VMA mappings.We update the documentation to reflect the new features.
Finally, we update the VMA tests accordingly to reflect the changes.
Reviewed-by Suren Baghdasaryan <[email protected]>Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
...
Quoted 24 lines
mm/util.c +5
-0@@ -1446,6 +1446,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)return io_remap_pfn_range_prepare (desc);case MMAP_SIMPLE_IO_REMAP :return simple_ioremap_prepare (desc); +case MMAP_MAP_KERNEL_PAGES : +return map_kernel_pages_prepare (desc); }WARN_ON_ONCE (1 );@@ -1476,6 +1478,9 @@ int mmap_action_complete(struct vm_area_struct *vma,case MMAP_REMAP_PFN : err =remap_pfn_range_complete (vma, action);break ; +case MMAP_MAP_KERNEL_PAGES : + err =map_kernel_pages_complete (vma, action); +break ;case MMAP_IO_REMAP_PFN :case MMAP_SIMPLE_IO_REMAP :/* Should have been delegated. */
mm/util.c int mmap_action_prepare(struct vm_area_desc *desc) · line 1508
Quoted 7 lines
mm/util.c +1
-0@@ -1497,6 +1502,7 @@ int mmap_action_prepare(struct vm_area_desc *desc)case MMAP_REMAP_PFN :case MMAP_IO_REMAP_PFN :case MMAP_SIMPLE_IO_REMAP : +case MMAP_MAP_KERNEL_PAGES :WARN_ON_ONCE (1 );/* nommu cannot handle these. */ break ; }
Not sure if it has been reported/addressed yet but it looks like mmap_action_complete() was missed here, as pointed out by clang:
$ make -skj"$(nproc)" ARCH=arm LLVM=1 mrproper allnoconfig mm/util.o
mm/util.c:1520:10: warning: enumeration value 'MMAP_MAP_KERNEL_PAGES' not handled in switch [-Wswitch]
1520 | switch (action->type) {
| ^~~~~~~~~~~~I assume
mm/util.c +1 -0
@@ -1523,6 +1523,7 @@ int mmap_action_complete(struct vm_area_struct *vma,case MMAP_REMAP_PFN :case MMAP_IO_REMAP_PFN :case MMAP_SIMPLE_IO_REMAP : +case MMAP_MAP_KERNEL_PAGES :WARN_ON_ONCE (1 );/* nommu cannot handle this. */ err = -EINVAL ;
--
should be the fix?
Cheers,
Nathan
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH v3 15/16] mm: add mmap_action_map_kernel_pages[_full]()
Quoted 84 lines · Nathan Chancellor
Hi Lorenzo,
Quoted 24 lines · Lorenzo Stoakes (Oracle)
A user can invoke
mmap_action_map_kernel_pages()to specify that the mapping should map kernel pages starting fromdesc->startof a specified number of pages specified in an array.In order to implement this, adjust
mmap_action_prepare()to be able to return an error code, as it makes sense to assert that the specified parameters are valid as quickly as possible as well as updating the VMA flags to includeVMA_MIXEDMAP_BITas necessary.This provides an
mmap_prepareequivalent ofvm_insert_pages(). We additionally update the existingvm_insert_pages()code to userange_in_vma()and add a newrange_in_vma_desc()helper function for themmap_preparecase, sharing the code between the two inrange_is_subset().We add both
mmap_action_map_kernel_pages()andmmap_action_map_kernel_pages_full()to allow for both partial and full VMA mappings.We update the documentation to reflect the new features.
Finally, we update the VMA tests accordingly to reflect the changes.
Reviewed-by Suren Baghdasaryan <[email protected]>Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>...
Quoted 24 lines
mm/util.c +5
-0@@ -1446,6 +1446,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)return io_remap_pfn_range_prepare (desc);case MMAP_SIMPLE_IO_REMAP :return simple_ioremap_prepare (desc); +case MMAP_MAP_KERNEL_PAGES : +return map_kernel_pages_prepare (desc); }WARN_ON_ONCE (1 );@@ -1476,6 +1478,9 @@ int mmap_action_complete(struct vm_area_struct *vma,case MMAP_REMAP_PFN : err =remap_pfn_range_complete (vma, action);break ; +case MMAP_MAP_KERNEL_PAGES : + err =map_kernel_pages_complete (vma, action); +break ;case MMAP_IO_REMAP_PFN :case MMAP_SIMPLE_IO_REMAP :/* Should have been delegated. */ mm/util.c int mmap_action_prepare(struct vm_area_desc *desc) · line 1508
Quoted 7 lines
mm/util.c +1
-0@@ -1497,6 +1502,7 @@ int mmap_action_prepare(struct vm_area_desc *desc)case MMAP_REMAP_PFN :case MMAP_IO_REMAP_PFN :case MMAP_SIMPLE_IO_REMAP : +case MMAP_MAP_KERNEL_PAGES :WARN_ON_ONCE (1 );/* nommu cannot handle these. */ break ; }Not sure if it has been reported/addressed yet but it looks like
mmap_action_complete()was missed here, as pointed out by clang:$ make -skj"$(nproc)" ARCH=arm LLVM=1 mrproper allnoconfig mm/util.o mm/util.c:1520:10: warning: enumeration value 'MMAP_MAP_KERNEL_PAGES' not handled in switch [-Wswitch] 1520 | switch (action->type) { | ^~~~~~~~~~~~I assume
mm/util.c +1
-1@@ -1523,6 +1523,7 @@ int mmap_action_complete(struct vm_area_struct *vma,case MMAP_REMAP_PFN :case MMAP_IO_REMAP_PFN :case MMAP_SIMPLE_IO_REMAP : +case MMAP_MAP_KERNEL_PAGES :WARN_ON_ONCE (1 );/* nommu cannot handle this. */ err = -EINVAL ; --
Quoted 4 lines
should be the fix?
Cheers,
Nathan
Thanks, will fix, working on a respin now anyway :)
Cheers, Lorenzo