On 17 Sep 2026, at 12:22, Lorenzo Stoakes (ARM) wrote:
> The map->file_doesnt_need_get flag is confusing and the existing
> implementation has holes.
>
> Drivers are permitted to change the owning file of a mapping. If they do
> so, they are required to take a reference on that file.
>
> The mmap() operation which ultimately invokes __mmap_region() is guaranteed
> to drop the refcount for the original file the mapping was made under, but
> this is not true for the replaced file.
>
> This has been addressed so far by tracking map->file_doesnt_need_get, which
> is rather poorly named and unfortunately fails to correctly track whether
> or not an additional put were needed in a number of cases.
>
> Make life easier by removing this flag, and instead drop the reference for
> both mmap_prepare and the deprecated mmap callback in a new function
> put_map().
>
> Track whether this needs to be done by aligning mmap_state with
> vm_area_desc and store the original file in the map->file field, keeping
> the updated file in map->vm_file.
>
> In order to have the same behaviour for both types of hooks, only drop the
> reference __mmap_new_file_vma() itself took in its error path, deferring
> the replaced file's reference to put_map().
>
> To make this work correctly, map->vm_file has to be updated before any
> error handling, so update __mmap_new_file_vma() and call_mmap_prepare() to
> set this field first.
>
> Also when mmap_prepare() changes the file and is then merged, the reference
> count also must be decremented, so update the logic to call put_map() in
> this case too.
>
> Also update __compat_vma_mmap() to manually perform this step for stacked
> file systems using the compatibility layer, and update
> compat_set_vma_from_desc() to replace vma_set_file() with a correct
> refcount/file update.
>
> No in-tree driver is impacted by the incorrect implementation of this
> currently (no driver that does this is mergeable for one), so this does not
> need to be a fix.
>
> Signed-off-by: Lorenzo Stoakes (ARM) <[email protected]>
> ---
> mm/internal.h | 1 +
> mm/util.c | 5 +++-
> mm/vma.c | 83
> +++++++++++++++++++++++++++++++++++------------------------
> mm/vma.h | 6 +++--
> 4 files changed, 59 insertions(+), 36 deletions(-)
>
> diff --git a/mm/internal.h b/mm/internal.h
> index 0dca33db068f..fe576d468af4 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -7,6 +7,7 @@
> #ifndef __MM_INTERNAL_H
> #define __MM_INTERNAL_H
>
> +#include <linux/file.h>
> #include <linux/fs.h>
> #include <linux/khugepaged.h>
> #include <linux/mm.h>
> diff --git a/mm/util.c b/mm/util.c
> index bf0513d1d3d0..016932780925 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -1228,8 +1228,11 @@ int __compat_vma_mmap(struct vm_area_desc *desc,
>
> /* Perform any preparatory tasks for mmap action. */
> err = mmap_action_prepare(desc);
> - if (err)
> + if (err) {
> + if (desc->vm_file != vma->vm_file)
> + fput(desc->vm_file);
> return err;
> + }
> /* Update the VMA from the descriptor. */
> compat_set_vma_from_desc(vma, desc);
> /* Complete any specified mmap actions. */
> diff --git a/mm/vma.c b/mm/vma.c
> index 55917d097933..fa784f069da4 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -24,7 +24,8 @@ struct mmap_state {
> vm_flags_t vm_flags;
> vma_flags_t vma_flags;
> };
> - struct file *file;
> + struct file *file; /* mmap()-specified file. */
IIUC, file is never assigned other than MMAP_STATE() and should not
change after mmap(). Could it be made const to prevent any change?
I assume mmap_state will not need to handle the nesting issue like
vm_area_desc, so file can be const.
> + struct file *vm_file; /* May be updated by mmap_prepare. */
> pgprot_t page_prot;
>
> /* User-defined fields, perhaps updated by .mmap_prepare(). */
> @@ -43,8 +44,6 @@ struct mmap_state {
>
> /* Determine if we can check KSM flags early in mmap() logic. */
> bool check_ksm_early :1;
> - /* If .mmap_prepare changed the file, we don't need to pin. */
> - bool file_doesnt_need_get :1;
> };
>
> #define MMAP_STATE(name, mm_, vmi_, addr_, len_, pgoff_, anon_pgoff_,
> vma_flags_, file_) \
> @@ -58,6 +57,7 @@ struct mmap_state {
> .pglen = PHYS_PFN(len_), \
> .vma_flags = vma_flags_, \
> .file = file_, \
> + .vm_file = file_, \
> .page_prot = vma_flags_to_page_prot(vma_flags_), \
> }
>
Best Regards,
Yan, Zi
