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.

Von: "Lorenzo Stoakes (Oracle)" <[email protected]>

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.

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
drivers/mtd/mtdchar.c | 21 +++------------------

1 file changed, 3 insertions(+), 18 deletions(-)

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.

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
 Documentation/filesystems/mmap_prepare.rst |  3 +
 include/linux/mm.h                         | 24 +++++-
 include/linux/mm_types.h                   |  6 +-
 mm/internal.h                              |  2 +
 mm/memory.c                                | 87 +++++++++++++++-------
 mm/util.c                                  | 12 +++
 tools/testing/vma/include/dup.h            |  6 +-
 7 files changed, 112 insertions(+), 28 deletions(-)

Documentation/filesystems/mmap_prepare.rst +3 -0

index 20db474915da..be76ae475b9c 100644
@@ -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

index ad1b8c3c0cfd..df8fa6e6402b 100644
@@ -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

index 4a229cc0a06b..50685cf29792 100644
@@ -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 +2 -0

index f5774892071e..0eaca2f0eb6a 100644
@@ -1804,6 +1804,8 @@ 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);
+/* No simple_ioremap_complete, is ultimately handled by remap complete. */
 
 static inline int io_remap_pfn_range_prepare(struct vm_area_desc *desc)
 {

mm/memory.c +0 -0

index 9dec67a18116..f3f4046aee97 100644

mm/memory.c +39 -0

+                                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 */
+       if (start_phys + size < start_phys)
+               return -EINVAL;
+       /*
+        * You *really* shouldn't map things that aren't page-aligned,
+        * but we've historically allowed it because IO memory might
+        * just have smaller alignment.
+        */
+       size += start_phys & ~PAGE_MASK;
+       pfn = start_phys >> PAGE_SHIFT;
+       pages = (size + ~PAGE_MASK) >> PAGE_SHIFT;
+       if (pfn + pages < pfn)
+               return -EINVAL;
+
+       /* We start the mapping 'vm_pgoff' pages into the area */
+       if (vm_pgoff > pages)
+               return -EINVAL;
+       pfn += vm_pgoff;
+       pages -= vm_pgoff;
+
+       /* Can we fit all of the mapping? */
+       if ((vm_len >> PAGE_SHIFT) > pages)
+               return -EINVAL;
+
+       *pfnp = pfn;
+       return 0;
+}
+
+int simple_ioremap_prepare(struct vm_area_desc *desc)
+{
+       struct mmap_action *action = &desc->action;
+       const phys_addr_t start = action->simple_ioremap.start_phys_addr;
+       const unsigned long size = action->simple_ioremap.size;

mm/memory.c +11 -25

+}
+
 /**
  * vm_iomap_memory - remap memory to userspace
  * @vma: user vma to map to

@@ -3187,32 +3240,16 @@ int remap_pfn_range_complete(struct vm_area_struct *vma,
  */
 int vm_iomap_memory(struct vm_area_struct *vma, phys_addr_t start, unsigned long len)
 {
-       unsigned long vm_len, pfn, pages;
-
-       /* Check that the physical memory area passed in looks valid */
-       if (start + len < start)
-               return -EINVAL;
-       /*
-        * You *really* shouldn't map things that aren't page-aligned,
-        * but we've historically allowed it because IO memory might
-        * just have smaller alignment.
-        */
-       len += start & ~PAGE_MASK;
-       pfn = start >> PAGE_SHIFT;
-       pages = (len + ~PAGE_MASK) >> PAGE_SHIFT;
-       if (pfn + pages < pfn)
-               return -EINVAL;
-
-       /* We start the mapping 'vm_pgoff' pages into the area */
-       if (vma->vm_pgoff > pages)
-               return -EINVAL;
-       pfn += vma->vm_pgoff;
-       pages -= vma->vm_pgoff;
+       const unsigned long vm_start = vma->vm_start;
+       const unsigned long vm_end = vma->vm_end;
+       const unsigned long vm_len = vm_end - vm_start;
+       unsigned long pfn;
+       int err;
 
-       /* Can we fit all of the mapping? */
-       vm_len = vma->vm_end - vma->vm_start;
-       if (vm_len >> PAGE_SHIFT > pages)
-               return -EINVAL;
+       err = __simple_ioremap_prep(vm_start, vm_end, vma->vm_pgoff, start,
+                                   len, &pfn);
+       if (err)
+               return err;
 
        /* Ok, let it rip */
        return io_remap_pfn_range(vma, vma->vm_start, pfn, vm_len, vma->vm_page_prot);

mm/util.c +12 -0

index cdfba09e50d7..aa92e471afe1 100644
@@ -1390,6 +1390,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);

@@ -1421,6 +1423,14 @@ int mmap_action_complete(struct vm_area_struct *vma,
        case MMAP_IO_REMAP_PFN:
                err = io_remap_pfn_range_complete(vma, action);
                break;
+       case MMAP_SIMPLE_IO_REMAP:
+               /*
+                * The simple I/O remap should have been delegated to an I/O
+                * remap.
+                */
+               WARN_ON_ONCE(1);
+               err = -EINVAL;
+               break;
        }
 
        return mmap_action_finish(vma, action, err);

@@ -1434,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;
        }

@@ -1452,6 +1463,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 -2

index 4570ec77f153..114daaef4f73 100644
@@ -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;
+                       unsigned long len;
+               } simple_ioremap;
        };
        enum mmap_action_type type;
 
--

2.53.0

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().

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
 drivers/misc/open-dice.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

drivers/misc/open-dice.c +11 -9

index 24c29e0f00ef..45060fb4ea27 100644
@@ -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)
--

2.53.0

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().

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
 drivers/char/hpet.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/char/hpet.c +7 -6

index 60dd09a56f50..8f128cc40147 100644
@@ -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)
--

2.53.0

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.

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
 drivers/hwtracing/stm/core.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

drivers/hwtracing/stm/core.c +21 -11

index 37584e786bb5..f48c6a8a0654 100644
@@ -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,
 };
--

2.53.0

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.

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
 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

index c0b475369de0..7111999abc14 100644
@@ -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

index f10a00c05f12..7220aba7b919 100644
@@ -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

index 797e9940fdd1..b6413605ea49 100644
@@ -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 +3 -7

index d95dd7d9190a..11e25c2f6b0a 100644
@@ -446,24 +446,14 @@ static void vme_user_vm_close(struct vm_area_struct *vma)
        kfree(vma_priv);
 }
 
-static const struct vm_operations_struct vme_user_vm_ops = {
-       .open = vme_user_vm_open,
-       .close = vme_user_vm_close,
-};
-
-static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma)
+static int vme_user_vm_mapped(unsigned long start, unsigned long end, pgoff_t pgoff,
+                             const struct file *file, void **vm_private_data)
 {
-       int err;
+       const unsigned int minor = iminor(file_inode(file));

drivers/staging/vme_user/vme_user.c +27 -9

        vma_priv = kmalloc_obj(*vma_priv);
        if (!vma_priv) {
                mutex_unlock(&image[minor].mutex);

@@ -472,22 +462,41 @@ static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma)
 
        vma_priv->minor = minor;
        refcount_set(&vma_priv->refcnt, 1);
-       vma->vm_ops = &vme_user_vm_ops;
-       vma->vm_private_data = vma_priv;
-
+       *vm_private_data = vma_priv;
        image[minor].mmap_count++;
 
        mutex_unlock(&image[minor].mutex);
-
        return 0;
 }
 
-static int vme_user_mmap(struct file *file, struct vm_area_struct *vma)
+static const struct vm_operations_struct vme_user_vm_ops = {
+       .mapped = vme_user_vm_mapped,
+       .open = vme_user_vm_open,
+       .close = vme_user_vm_close,
+};
+
+static int vme_user_master_mmap_prepare(unsigned int minor,
+                                       struct vm_area_desc *desc)
+{
+       int err;
+
+       mutex_lock(&image[minor].mutex);
+
+       err = vme_master_mmap_prepare(image[minor].resource, desc);
+       if (!err)
+               desc->vm_ops = &vme_user_vm_ops;
+
+       mutex_unlock(&image[minor].mutex);
+       return err;
+}
+
+static int vme_user_mmap_prepare(struct vm_area_desc *desc)
 {
-       unsigned int minor = iminor(file_inode(file));
+       const struct file *file = desc->file;
+       const unsigned int minor = iminor(file_inode(file));
 
        if (type[minor] == MASTER_MINOR)
-               return vme_user_master_mmap(minor, vma);
+               return vme_user_master_mmap_prepare(minor, desc);
 
        return -ENODEV;
 }

@@ -498,7 +507,7 @@ static const struct file_operations vme_user_fops = {
        .llseek = vme_user_llseek,
        .unlocked_ioctl = vme_user_unlocked_ioctl,
        .compat_ioctl = compat_ptr_ioctl,
-       .mmap = vme_user_mmap,
+       .mmap_prepare = vme_user_mmap_prepare,
 };
 
 static int vme_user_match(struct vme_dev *vdev)
--

2.53.0

Quoted 122 lines · 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.

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
 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

index c0b475369de0..7111999abc14 100644
@@ -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

index f10a00c05f12..7220aba7b919 100644
@@ -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

index 797e9940fdd1..b6413605ea49 100644
@@ -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 +3 -7

index d95dd7d9190a..11e25c2f6b0a 100644
@@ -446,24 +446,14 @@ static void vme_user_vm_close(struct vm_area_struct *vma)
        kfree(vma_priv);
 }
 
-static const struct vm_operations_struct vme_user_vm_ops = {
-       .open = vme_user_vm_open,
-       .close = vme_user_vm_close,
-};
-
-static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma)
+static int vme_user_vm_mapped(unsigned long start, unsigned long end, pgoff_t pgoff,
+                             const struct file *file, void **vm_private_data)
 {
-       int err;
+       const unsigned int minor = iminor(file_inode(file));

drivers/staging/vme_user/vme_user.c static void vme_user_vm_close(struct vm_area_struct *vma) · line 466

Quoted 10 lines

drivers/staging/vme_user/vme_user.c +0 -6

        struct vme_user_vma_priv *vma_priv;
 
        mutex_lock(&image[minor].mutex);
 
-       err = vme_master_mmap(image[minor].resource, vma);
-       if (err) {
-               mutex_unlock(&image[minor].mutex);
-               return err;
-       }
-

Ok, this changes the set of the operations performed under image[minor].mutex. Before we had:

mutex_lock(&image[minor].mutex);
vme_master_mmap();
<some final adjustments>
mutex_unlock(&image[minor].mutex);

Now we have:

mutex_lock(&image[minor].mutex);
vme_master_mmap_prepare()
mutex_unlock(&image[minor].mutex);
vm_iomap_memory();
mutex_lock(&image[minor].mutex);
vme_user_vm_mapped(); // <some final adjustments>
mutex_unlock(&image[minor].mutex);

I think as long as image[minor] does not change while we are not holding the mutex we should be safe, and looking at the code it seems to be the case. But I'm not familiar with this driver and might be wrong. Worth double-checking.

Quoted 65 lines

drivers/staging/vme_user/vme_user.c +27 -9

        vma_priv = kmalloc_obj(*vma_priv);
        if (!vma_priv) {
                mutex_unlock(&image[minor].mutex);

@@ -472,22 +462,41 @@ static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma)
 
        vma_priv->minor = minor;
        refcount_set(&vma_priv->refcnt, 1);
-       vma->vm_ops = &vme_user_vm_ops;
-       vma->vm_private_data = vma_priv;
-
+       *vm_private_data = vma_priv;
        image[minor].mmap_count++;
 
        mutex_unlock(&image[minor].mutex);
-
        return 0;
 }
 
-static int vme_user_mmap(struct file *file, struct vm_area_struct *vma)
+static const struct vm_operations_struct vme_user_vm_ops = {
+       .mapped = vme_user_vm_mapped,
+       .open = vme_user_vm_open,
+       .close = vme_user_vm_close,
+};
+
+static int vme_user_master_mmap_prepare(unsigned int minor,
+                                       struct vm_area_desc *desc)
+{
+       int err;
+
+       mutex_lock(&image[minor].mutex);
+
+       err = vme_master_mmap_prepare(image[minor].resource, desc);
+       if (!err)
+               desc->vm_ops = &vme_user_vm_ops;
+
+       mutex_unlock(&image[minor].mutex);
+       return err;
+}
+
+static int vme_user_mmap_prepare(struct vm_area_desc *desc)
 {
-       unsigned int minor = iminor(file_inode(file));
+       const struct file *file = desc->file;
+       const unsigned int minor = iminor(file_inode(file));
 
        if (type[minor] == MASTER_MINOR)
-               return vme_user_master_mmap(minor, vma);
+               return vme_user_master_mmap_prepare(minor, desc);
 
        return -ENODEV;
 }

@@ -498,7 +507,7 @@ static const struct file_operations vme_user_fops = {
        .llseek = vme_user_llseek,
        .unlocked_ioctl = vme_user_unlocked_ioctl,
        .compat_ioctl = compat_ptr_ioctl,
-       .mmap = vme_user_mmap,
+       .mmap_prepare = vme_user_mmap_prepare,
 };
 
 static int vme_user_match(struct vme_dev *vdev)
--

2.53.0

While the conversion of mmap hooks to mmap_prepare is underway, we wil

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_mapped() 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_vm(&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.

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
 include/linux/fs.h              |   3 +
 include/linux/mm_types.h        |   4 +-
 mm/util.c                       | 111 +++++++++++++++++++++++---------
 mm/vma.h                        |   2 +-
 tools/testing/vma/include/dup.h | 111 ++++++++++++++++++++------------
 5 files changed, 157 insertions(+), 74 deletions(-)

include/linux/fs.h +3 -0

index c390f5c667e3..0bdccfa70b44 100644
@@ -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

index 50685cf29792..7538d64f8848 100644
@@ -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 +80 -31

index aa92e471afe1..a166c48fe894 100644
@@ -1163,34 +1163,38 @@ 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)
+/**
+ * compat_set_desc_from_vma() - assigns VMA descriptor @desc fields from a VMA.
+ * @desc: A VMA descriptor whose fields need to be set.
+ * @file: The file object describing the file being mmap()'d.
+ * @vma: The VMA whose fields we wish to assign to @desc.
+ *
+ * This is a compatibility function to allow an mmap() hook to call
+ * mmap_prepare() hooks when drivers nest these. This function specifically
+ * allows the construction of a vm_area_desc value, @desc, from a VMA @vma for
+ * the purposes of doing this.
+ *
+ * Once the conversion of drivers is complete this function will no longer be
+ * required and will be removed.
+ */
+void compat_set_desc_from_vma(struct vm_area_desc *desc,
+                             const struct file *file,
+                             const 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;
+       desc->mm = vma->vm_mm;
+       desc->file = (struct file *)file;
+       desc->start = vma->vm_start;
+       desc->end = vma->vm_end;
 
-       err = vfs_mmap_prepare(file, &desc);
-       if (err)
-               return err;
+       desc->pgoff = vma->vm_pgoff;
+       desc->vm_file = vma->vm_file;
+       desc->vma_flags = vma->flags;
+       desc->page_prot = vma->vm_page_prot;
 
-       err = mmap_action_prepare(&desc);
-       if (err)
-               return err;
-
-       set_vma_from_desc(vma, &desc);
-       return mmap_action_complete(vma, &desc.action);
+       /* Default. */
+       desc->action.type = MMAP_NOTHING;
 }
+EXPORT_SYMBOL(compat_set_desc_from_vma);
 
 static int __compat_vma_mapped(struct file *file, struct vm_area_struct *vma)
 {

@@ -1211,6 +1215,49 @@ static int __compat_vma_mapped(struct file *file, struct vm_area_struct *vma)
        return err;
 }
 
+/**
+ * __compat_vma_mmap() - Similar to compat_vma_mmap(), only it allows
+ * flexibility as to how the mmap_prepare callback is invoked, which is useful
+ * for drivers which invoke nested mmap_prepare callbacks in an mmap() hook.
+ * @desc: A VMA descriptor upon which an mmap_prepare() hook has already been
+ * executed.
+ * @vma: The VMA to which @desc should be applied.
+ *
+ * The function assumes that you have obtained a VMA descriptor @desc from
+ * compt_set_desc_from_vma(), and already executed the mmap_prepare() hook upon
+ * it.
+ *
+ * It then performs any specified mmap actions, and invokes the vm_ops->mapped()
+ * hook if one is present.
+ *
+ * See the description of compat_vma_mmap() for more details.
+ *
+ * Once the conversion of drivers is complete this function will no longer be
+ * required and will be removed.
+ *
+ * Returns: 0 on success or error.
+ */
+int __compat_vma_mmap(struct vm_area_desc *desc,
+                     struct vm_area_struct *vma)
+{
+       int err;
+
+       /* Perform any preparatory tasks for mmap action. */
+       err = mmap_action_prepare(desc);
+       if (err)
+               return err;
+       /* 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);
+       if (err)
+               return err;
+
+       /* Invoke vm_ops->mapped callback. */
+       return __compat_vma_mapped(desc->file, vma);
+}
+EXPORT_SYMBOL(__compat_vma_mmap);
+
 /**
  * compat_vma_mmap() - Apply the file's .mmap_prepare() hook to an
  * existing VMA and execute any requested actions.

@@ -1218,10 +1265,10 @@ static int __compat_vma_mapped(struct file *file, struct vm_area_struct *vma)
  * @vma: The VMA to apply the .mmap_prepare() hook to.
  *
  * Ordinarily, .mmap_prepare() is invoked directly upon mmap(). However, certain
- * stacked filesystems invoke a nested mmap hook of an underlying file.
+ * stacked drivers invoke a nested mmap hook of an underlying file.
  *
- * Until all filesystems are converted to use .mmap_prepare(), we must be
- * conservative and continue to invoke these stacked filesystems using the
+ * Until all drivers are converted to use .mmap_prepare(), we must be
+ * conservative and continue to invoke these stacked drivers using the
  * deprecated .mmap() hook.
  *
  * However we have a problem if the underlying file system possesses an

@@ -1232,20 +1279,22 @@ static int __compat_vma_mapped(struct file *file, struct vm_area_struct *vma)
  * establishes a struct vm_area_desc descriptor, passes to the underlying
  * .mmap_prepare() hook and applies any changes performed by it.
  *
- * Once the conversion of filesystems is complete this function will no longer
- * be required and will be removed.
+ * Once the conversion of drivers is complete this function will no longer be
+ * required and will be removed.
  *
  * Returns: 0 on success or error.
  */
 int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
 {
+       struct vm_area_desc desc;
        int err;
 
-       err = __compat_vma_mmap(file, vma);
+       compat_set_desc_from_vma(&desc, file, vma);
+       err = vfs_mmap_prepare(file, &desc);
        if (err)
                return err;
 
-       return __compat_vma_mapped(file, vma);
+       return __compat_vma_mmap(&desc, vma);
 }
 EXPORT_SYMBOL(compat_vma_mmap);
 

mm/vma.h +1 -1

index adc18f7dd9f1..a76046c39b14 100644
@@ -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 +71 -41

index 114daaef4f73..6658df26698a 100644
@@ -519,8 +519,8 @@ enum vma_operation {
  */
 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;
 

@@ -1272,43 +1272,92 @@ static inline void vma_set_anonymous(struct vm_area_struct *vma)
 }
 
 /* Declared in vma.h. */
-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);
 
-static inline int __compat_vma_mmap(const struct file_operations *f_op,
-               struct file *file, struct vm_area_struct *vma)
+static inline void compat_set_desc_from_vma(struct vm_area_desc *desc,
+                             const struct file *file,
+                             const struct vm_area_struct *vma)
 {
-       struct vm_area_desc desc = {
-               .mm = vma->vm_mm,
-               .file = file,
-               .start = vma->vm_start,
-               .end = vma->vm_end,
+       desc->mm = vma->vm_mm;
+       desc->file = (struct file *)file;
+       desc->start = vma->vm_start;
+       desc->end = vma->vm_end;
 
-               .pgoff = vma->vm_pgoff,
-               .vm_file = vma->vm_file,
-               .vma_flags = vma->flags,
-               .page_prot = vma->vm_page_prot,
+       desc->pgoff = vma->vm_pgoff;
+       desc->vm_file = vma->vm_file;
+       desc->vma_flags = vma->flags;
+       desc->page_prot = vma->vm_page_prot;
 
-               .action.type = MMAP_NOTHING, /* Default */
-       };
+       /* Default. */
+       desc->action.type = MMAP_NOTHING;
+}
+
+static inline unsigned long vma_pages(const struct vm_area_struct *vma)
+{
+       return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+}
+
+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);
+}
+
+static inline int __compat_vma_mapped(struct file *file, struct vm_area_struct *vma)
+{
+       const struct vm_operations_struct *vm_ops = vma->vm_ops;
        int err;
 
-       err = f_op->mmap_prepare(&desc);
+       if (!vm_ops->mapped)
+               return 0;
+
+       err = vm_ops->mapped(vma->vm_start, vma->vm_end, vma->vm_pgoff, file,
+                            &vma->vm_private_data);
        if (err)
-               return err;
+               unmap_vma_locked(vma);
+       return err;
+}
 
-       err = mmap_action_prepare(&desc);
+static inline int __compat_vma_mmap(struct vm_area_desc *desc,
+               struct vm_area_struct *vma)
+{
+       int err;
+
+       /* Perform any preparatory tasks for mmap action. */
+       err = mmap_action_prepare(desc);
+       if (err)
+               return err;
+       /* 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);
        if (err)
                return err;
 
-       set_vma_from_desc(vma, &desc);
-       return mmap_action_complete(vma, &desc.action);
+       /* Invoke vm_ops->mapped callback. */
+       return __compat_vma_mapped(desc->file, vma);
+}
+
+static inline int vfs_mmap_prepare(struct file *file, struct vm_area_desc *desc)
+{
+       return file->f_op->mmap_prepare(desc);
 }
 
 static inline int compat_vma_mmap(struct file *file,
                struct vm_area_struct *vma)
 {
-       return __compat_vma_mmap(file->f_op, file, vma);
+       struct vm_area_desc desc;
+       int err;
+
+       compat_set_desc_from_vma(&desc, file, vma);
+       err = vfs_mmap_prepare(file, &desc);
+       if (err)
+               return err;
+
+       return __compat_vma_mmap(&desc, vma);
 }
 
 

@@ -1318,11 +1367,6 @@ static inline void vma_iter_init(struct vma_iterator *vmi,
        mas_init(&vmi->mas, &mm->mm_mt, addr);
 }
 
-static inline unsigned long vma_pages(struct vm_area_struct *vma)
-{
-       return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
-}
-
 static inline void mmap_assert_locked(struct mm_struct *);
 static inline struct vm_area_struct *find_vma_intersection(struct mm_struct *mm,
                                                unsigned long start_addr,

@@ -1492,11 +1536,6 @@ static inline int vfs_mmap(struct file *file, struct vm_area_struct *vma)
        return file->f_op->mmap(file, vma);
 }
 
-static inline int vfs_mmap_prepare(struct file *file, struct vm_area_desc *desc)
-{
-       return file->f_op->mmap_prepare(desc);
-}
-
 static inline void vma_set_file(struct vm_area_struct *vma, struct file *file)
 {
        /* Changing an anonymous vma with this is illegal */

@@ -1521,11 +1560,3 @@ 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);
-}
--

2.53.0

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 also add mmap_action_map_kernel_pages_discontig() to allow for discontiguous mapping of kernel pages should the need arise.

We update the documentation to reflect the new features.

Finally, we update the VMA tests accordingly to reflect the changes.

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
 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

index be76ae475b9c..e810aa4134eb 100644
@@ -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 +0 -0

index df8fa6e6402b..6f0a3edb24e1 100644

include/linux/mm.h +92 -1

  */
 static inline int folio_expected_ref_count(const struct folio *folio)
 {

@@ -4364,6 +4364,45 @@ static inline void mmap_action_simple_ioremap(struct vm_area_desc *desc,
        action->type = MMAP_SIMPLE_IO_REMAP;
 }
 
+/**
+ * mmap_action_map_kernel_pages - helper for mmap_prepare hook to specify that
+ * @num kernel pages contained in the @pages array should be mapped to userland
+ * starting at virtual address @start.
+ * @desc: The VMA descriptor for the VMA requiring kernel pags to be mapped.
+ * @start: The virtual address from which to map them.
+ * @pages: An array of struct page pointers describing the memory to map.
+ * @nr_pages: The number of entries in the @pages aray.
+ */
+static inline void mmap_action_map_kernel_pages(struct vm_area_desc *desc,
+               unsigned long start, struct page **pages,
+               unsigned long nr_pages)
+{
+       struct mmap_action *action = &desc->action;
+
+       action->type = MMAP_MAP_KERNEL_PAGES;
+       action->map_kernel.start = start;
+       action->map_kernel.pages = pages;
+       action->map_kernel.nr_pages = nr_pages;
+       action->map_kernel.pgoff = desc->pgoff;
+}
+
+/**
+ * mmap_action_map_kernel_pages_full - helper for mmap_prepare hook to specify that
+ * kernel pages contained in the @pages array should be mapped to userland
+ * from @desc->start to @desc->end.
+ * @desc: The VMA descriptor for the VMA requiring kernel pags to be mapped.
+ * @pages: An array of struct page pointers describing the memory to map.
+ *
+ * The caller must ensure that @pages contains sufficient entries to cover the
+ * entire range described by @desc.
+ */
+static inline void mmap_action_map_kernel_pages_full(struct vm_area_desc *desc,
+               struct page **pages)
+{
+       mmap_action_map_kernel_pages(desc, desc->start, pages,
+                                    vma_desc_pages(desc));
+}
+
 int mmap_action_prepare(struct vm_area_desc *desc);
 int mmap_action_complete(struct vm_area_struct *vma,
                         struct mmap_action *action);

@@ -4380,10 +4419,59 @@ static inline struct vm_area_struct *find_exact_vma(struct mm_struct *mm,
        return vma;
 }
 
+/**
+ * range_is_subset - Is the specified inner range a subset of the outer range?
+ * @outer_start: The start of the outer range.
+ * @outer_end: The exclusive end of the outer range.
+ * @inner_start: The start of the inner range.
+ * @inner_end: The exclusive end of the inner range.
+ *
+ * Returns: %true if [inner_start, inner_end) is a subset of [outer_start,
+ * outer_end), otherwise %false.
+ */
+static inline bool range_is_subset(unsigned long outer_start,
+                                  unsigned long outer_end,
+                                  unsigned long inner_start,
+                                  unsigned long inner_end)
+{
+       return outer_start <= inner_start && inner_end <= outer_end;
+}
+
+/**
+ * range_in_vma - is the specified [@start, @end) range a subset of the VMA?
+ * @vma: The VMA against which we want to check [@start, @end).
+ * @start: The start of the range we wish to check.
+ * @end: The exclusive end of the range we wish to check.
+ *
+ * Returns: %true if [@start, @end) is a subset of [@vma->vm_start,
+ * @vma->vm_end), %false otherwise.
+ */
 static inline bool range_in_vma(const struct vm_area_struct *vma,
                                unsigned long start, unsigned long end)
 {
-       return (vma && vma->vm_start <= start && end <= vma->vm_end);
+       if (!vma)
+               return false;
+
+       return range_is_subset(vma->vm_start, vma->vm_end, start, end);
+}
+
+/**
+ * range_in_vma_desc - is the specified [@start, @end) range a subset of the VMA
+ * described by @desc, a VMA descriptor?
+ * @desc: The VMA descriptor against which we want to check [@start, @end).
+ * @start: The start of the range we wish to check.
+ * @end: The exclusive end of the range we wish to check.
+ *
+ * Returns: %true if [@start, @end) is a subset of [@desc->start, @desc->end),
+ * %false otherwise.
+ */
+static inline bool range_in_vma_desc(const struct vm_area_desc *desc,
+                                    unsigned long start, unsigned long end)
+{
+       if (!desc)
+               return false;
+
+       return range_is_subset(desc->start, desc->end, start, end);
 }
 
 #ifdef CONFIG_MMU

@@ -4427,6 +4515,9 @@ int remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
 int vm_insert_page(struct vm_area_struct *, unsigned long addr, struct page *);
 int vm_insert_pages(struct vm_area_struct *vma, unsigned long addr,
                        struct page **pages, unsigned long *num);
+int map_kernel_pages_prepare(struct vm_area_desc *desc);
+int map_kernel_pages_complete(struct vm_area_struct *vma,
+                             struct mmap_action *action);
 int vm_map_pages(struct vm_area_struct *vma, struct page **pages,
                                unsigned long num);
 int vm_map_pages_zero(struct vm_area_struct *vma, struct page **pages,

include/linux/mm_types.h +7 -0

index 7538d64f8848..c46224020a46 100644
@@ -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

index f3f4046aee97..849d5d9eeb83 100644
@@ -2484,13 +2484,14 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,
 int vm_insert_pages(struct vm_area_struct *vma, unsigned long addr,
                        struct page **pages, unsigned long *num)
 {
-       const unsigned long end_addr = addr + (*num * PAGE_SIZE) - 1;
+       const unsigned long nr_pages = *num;
+       const unsigned long end = addr + PAGE_SIZE * nr_pages;
 
-       if (addr < vma->vm_start || end_addr >= vma->vm_end)
+       if (!range_in_vma(vma, addr, end))
                return -EFAULT;
        if (!(vma->vm_flags & VM_MIXEDMAP)) {
-               BUG_ON(mmap_read_trylock(vma->vm_mm));
-               BUG_ON(vma->vm_flags & VM_PFNMAP);
+               VM_WARN_ON_ONCE(mmap_read_trylock(vma->vm_mm));
+               VM_WARN_ON_ONCE(vma->vm_flags & VM_PFNMAP);
                vm_flags_set(vma, VM_MIXEDMAP);
        }
        /* Defer page refcount checking till we're about to map that page. */

@@ -2498,6 +2499,39 @@ int vm_insert_pages(struct vm_area_struct *vma, unsigned long addr,
 }
 EXPORT_SYMBOL(vm_insert_pages);
 
+int map_kernel_pages_prepare(struct vm_area_desc *desc)
+{
+       const struct mmap_action *action = &desc->action;
+       const unsigned long addr = action->map_kernel.start;
+       unsigned long nr_pages, end;
+
+       if (!vma_desc_test(desc, VMA_MIXEDMAP_BIT)) {
+               VM_WARN_ON_ONCE(mmap_read_trylock(desc->mm));
+               VM_WARN_ON_ONCE(vma_desc_test(desc, VMA_PFNMAP_BIT));
+               vma_desc_set_flags(desc, VMA_MIXEDMAP_BIT);
+       }
+
+       nr_pages = action->map_kernel.nr_pages;
+       end = addr + PAGE_SIZE * nr_pages;
+       if (!range_in_vma_desc(desc, addr, end))
+               return -EFAULT;
+
+       return 0;
+}
+EXPORT_SYMBOL(map_kernel_pages_prepare);
+
+int map_kernel_pages_complete(struct vm_area_struct *vma,
+                             struct mmap_action *action)
+{
+       unsigned long nr_pages;
+
+       nr_pages = action->map_kernel.nr_pages;
+       return insert_pages(vma, action->map_kernel.start,
+                           action->map_kernel.pages,
+                           &nr_pages, vma->vm_page_prot);
+}
+EXPORT_SYMBOL(map_kernel_pages_complete);
+
 /**
  * vm_insert_page - insert single page into user vma
  * @vma: user vma to map to

mm/util.c +6 -0

index a166c48fe894..dea590e7a26c 100644
@@ -1441,6 +1441,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);

@@ -1472,6 +1474,9 @@ int mmap_action_complete(struct vm_area_struct *vma,
        case MMAP_IO_REMAP_PFN:
                err = io_remap_pfn_range_complete(vma, action);
                break;
+       case MMAP_MAP_KERNEL_PAGES:
+               err = map_kernel_pages_complete(vma, action);
+               break;
        case MMAP_SIMPLE_IO_REMAP:
                /*
                 * The simple I/O remap should have been delegated to an I/O

@@ -1494,6 +1499,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 -1

index 6658df26698a..4407caf207ad 100644
@@ -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;
                        unsigned long len;
                } simple_ioremap;
+               struct {
+                       unsigned long start;
+                       struct page **pages;
+                       unsigned long num;
+                       pgoff_t pgoff;
+               } map_kernel;
        };
        enum mmap_action_type type;
 
--

2.53.0

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.

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

---
 mm/memory.c | 3 +++
 1 file changed, 3 insertions(+)

mm/memory.c +3 -1

index 849d5d9eeb83..de0dd17759e2 100644
@@ -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)
--

2.53.0

Quoted 21 lines · 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.

Quoted 3 lines

Finally, we update the VMA tests to reflect this change.

Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>

A couple of nits, but otherwise LGTM.

Reviewed-by Suren Baghdasaryan <[email protected]>

Quoted 114 lines

---
 Documentation/filesystems/mmap_prepare.rst |  3 +
 include/linux/mm.h                         | 24 +++++-
 include/linux/mm_types.h                   |  6 +-
 mm/internal.h                              |  2 +
 mm/memory.c                                | 87 +++++++++++++++-------
 mm/util.c                                  | 12 +++
 tools/testing/vma/include/dup.h            |  6 +-
 7 files changed, 112 insertions(+), 28 deletions(-)

Documentation/filesystems/mmap_prepare.rst +3 -0

index 20db474915da..be76ae475b9c 100644
@@ -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

index ad1b8c3c0cfd..df8fa6e6402b 100644
@@ -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

index 4a229cc0a06b..50685cf29792 100644
@@ -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 +2 -0

index f5774892071e..0eaca2f0eb6a 100644
@@ -1804,6 +1804,8 @@ 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);
+/* No simple_ioremap_complete, is ultimately handled by remap complete. */
 
 static inline int io_remap_pfn_range_prepare(struct vm_area_desc *desc)
 {

mm/memory.c +0 -0

index 9dec67a18116..f3f4046aee97 100644
@@ -3170,6 +3170,59 @@ int remap_pfn_range_complete(struct vm_area_struct *vma,
        return do_remap_pfn_range(vma, start, pfn, size, prot);
 }