drivers/char/mem: eliminate unnecessary use of success_hook

Patch series "remove mmap_action success, error hooks", v3.

The mmap_action->success_hook was a strange beast added to enable code
which appeared to absolutely require access to a VMA pointer to work
correctly.

Primarily this was for hugetlb, however a different approach will be taken
there, as clearly more work is required to figure out a sensible way of
converting hugetlb to use mmap_prepare.

The other user was the memory char driver, specifically /dev/zero which
has the unusual property of explicitly setting file-backed VMAs anonymous.

Providing the success hook was always foolish, as it allowed drivers a way
to workaround the restriction that they should not access a pointer to a
not-yet-correctly-initialised VMA - which defeats the purpose of the
mmap_prepare work.

We can achieve the same thing in memory char driver without needing the
success hook, so this series removes that, then removes the success hook
altogether.

The error hook is also unnecessary - the motivation for this was for
functions which need to override the error code when performing an mmap
action in order to avoid breaking userspace.

We can achieve this by just providing a field for the error code.  Doing
this means we don't have to worry about the hook doing anything odd.

We also add a check to ensure the error code is in fact valid.

Again the memory char driver is the only current user of this, so this
series updates it to use that.

After this change mmap_action has no custom hooks at all, which seems
rather more cromulent than before.

This patch (of 3):

/dev/zero, uniquely, marks memory mapped there as anonymous.  This is
currently achieved using the mmap_action->success_hook.

However this hook circumvents the abstraction of VMA initialisation so
it's preferable to do things a different way.

To achieve this, this patch firstly defaults the VMA descriptor's vm_ops
field to the dummy VMA operations, which is what file-backed VMAs default
this field to.

That way, we can detect whether a driver sets this field to NULL in order
to mark it anonymous.

We then introduce vma_desc_set_anonymous() to do this explicitly, and
invoke it in mmap_zero_prepare().

This way, any driver which does not explicitly set desc->vm_ops, retains
the dummy vm_ops as they would previously.

We also update set_vma_user_defined_fields() to make clear that we are
either setting vma->vm_ops to what is provided by the driver (or
defaulting to dummy_vm_ops if not set), or setting the VMA anonymous.

This lays the groundwork for removing the success hook.

Link: https://lore.kernel.org/cover.1780397980.git.ljs@kernel.org
Link: https://lore.kernel.org/010579cca6787cf7bb057ab1f7228978b10601c8.1780397980.git.ljs@kernel.org
Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jann Horn <jannh@google.com>
Cc: Liam R. Howlett <liam@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Pedro Falcato <pfalcato@suse.de>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Vlastimil Babka <vbabka@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 5fd421e48c04beb99056c93d02ea8516aa163aad..a4297eb39887f9493ec7efaee426cab495d15641 100644 (file)
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -504,17 +504,6 @@ static ssize_t read_zero(struct file *file, char __user *buf,
        return cleared;
 }
 
-static int mmap_zero_private_success(const struct vm_area_struct *vma)
-{
-       /*
-        * This is a highly unique situation where we mark a MAP_PRIVATE mapping
-        * of /dev/zero anonymous, despite it not being.
-        */
-       vma_set_anonymous((struct vm_area_struct *)vma);
-
-       return 0;
-}
-
 static int mmap_zero_prepare(struct vm_area_desc *desc)
 {
 #ifndef CONFIG_MMU
@@ -523,7 +512,11 @@ static int mmap_zero_prepare(struct vm_area_desc *desc)
        if (vma_desc_test(desc, VMA_SHARED_BIT))
                return shmem_zero_setup_desc(desc);
 
-       desc->action.success_hook = mmap_zero_private_success;
+       /*
+        * This is a highly unique situation where we mark a MAP_PRIVATE mapping
+        * of /dev/zero anonymous, despite it not being.
+        */
+       vma_desc_set_anonymous(desc);
        return 0;
 }
 

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 11f440e9d7cdeae7d15eccb256714ed2c5347c7b..0f2612a70fb1e2b66917c6720a53c2fa19f0652c 100644 (file)
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1489,6 +1489,11 @@ static inline void vma_set_anonymous(struct vm_area_struct *vma)
        vma->vm_ops = NULL;
 }
 
+static inline void vma_desc_set_anonymous(struct vm_area_desc *desc)
+{
+       desc->vm_ops = NULL;
+}
+
 static inline bool vma_is_anonymous(struct vm_area_struct *vma)
 {
        return !vma->vm_ops;

diff --git a/mm/util.c b/mm/util.c
index 3cc949a0b7ed4f5d3ea5194eb18344997726f495..2b2a9df689d7cee802838b7d6a93bab8085c05b8 100644 (file)
--- a/mm/util.c
+++ b/mm/util.c
@@ -1192,6 +1192,7 @@ void compat_set_desc_from_vma(struct vm_area_desc *desc,
        desc->vm_file = vma->vm_file;
        desc->vma_flags = vma->flags;
        desc->page_prot = vma->vm_page_prot;
+       desc->vm_ops = vma->vm_ops;
 
        /* Default. */
        desc->action.type = MMAP_NOTHING;

diff --git a/mm/vma.c b/mm/vma.c
index d90791b00a7b81b72b1679d80be25f98b1c31824..9eea2850818a8541160e56aae2a4d02f8f2a88b0 100644 (file)
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2697,6 +2697,8 @@ static void set_vma_user_defined_fields(struct vm_area_struct *vma,
 {
        if (map->vm_ops)
                vma->vm_ops = map->vm_ops;
+       else    /* Only /dev/zero should do this. */
+               vma_set_anonymous(vma);
        vma->vm_private_data = map->vm_private_data;
 }
 
@@ -2744,6 +2746,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
                .action = {
                        .type = MMAP_NOTHING, /* Default to no further action. */
                },
+               .vm_ops = &vma_dummy_vm_ops,
        };
        bool allocated_new = false;
        int error;

diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index 9e0dfd3a85b0e2212ae8d1f8e31132e110588dd6..306171d061e70a616ffefc6d5869cdeb312a258e 100644 (file)
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -1303,6 +1303,7 @@ static inline void compat_set_desc_from_vma(struct vm_area_desc *desc,
        desc->vm_file = vma->vm_file;
        desc->vma_flags = vma->flags;
        desc->page_prot = vma->vm_page_prot;
+       desc->vm_ops = vma->vm_ops;
 
        /* Default. */
        desc->action.type = MMAP_NOTHING;