2023-10-12 17:04:50

by Lorenzo Stoakes

[permalink] [raw]
Subject: [PATCH v4 0/3] permit write-sealed memfd read-only shared mappings

The man page for fcntl() describing memfd file seals states the following
about F_SEAL_WRITE:-

Furthermore, trying to create new shared, writable memory-mappings via
mmap(2) will also fail with EPERM.

With emphasis on 'writable'. In turns out in fact that currently the kernel
simply disallows all new shared memory mappings for a memfd with
F_SEAL_WRITE applied, rendering this documentation inaccurate.

This matters because users are therefore unable to obtain a shared mapping
to a memfd after write sealing altogether, which limits their
usefulness. This was reported in the discussion thread [1] originating from
a bug report [2].

This is a product of both using the struct address_space->i_mmap_writable
atomic counter to determine whether writing may be permitted, and the
kernel adjusting this counter when any VM_SHARED mapping is performed and
more generally implicitly assuming VM_SHARED implies writable.

It seems sensible that we should only update this mapping if VM_MAYWRITE is
specified, i.e. whether it is possible that this mapping could at any point
be written to.

If we do so then all we need to do to permit write seals to function as
documented is to clear VM_MAYWRITE when mapping read-only. It turns out
this functionality already exists for F_SEAL_FUTURE_WRITE - we can
therefore simply adapt this logic to do the same for F_SEAL_WRITE.

We then hit a chicken and egg situation in mmap_region() where the check
for VM_MAYWRITE occurs before we are able to clear this flag. To work
around this, perform this check after we invoke call_mmap(), with careful
consideration of error paths.

Thanks to Andy Lutomirski for the suggestion!

[1]:https://lore.kernel.org/all/[email protected]/
[2]:https://bugzilla.kernel.org/show_bug.cgi?id=217238

v4:
- Revert to performing the writable check _after_ the call_mmap()
invocation, as the only impact should be internal mm checks, rather than
call_mmap(), as suggested by Jan Kara.
- Additionally, fixup error handling paths, which resulted in an i915 test
failure previously erroneously double-decrement the i_mmap_writable
counter. We do this by tracking whether we have in fact marked the mapping
writable. This is based on Jan's feedback also.

v3:
- Don't defer the writable check until after call_mmap() in case this
breaks f_ops->mmap() callbacks which assume this has been done
first. Instead, separate the check and enforcement of it across the call,
allowing for it to change vma->vm_flags in the meanwhile.
- Improve/correct commit messages and comments throughout.
https://lore.kernel.org/all/[email protected]

v2:
- Removed RFC tag.
- Correct incorrect goto pointed out by Jan.
- Reworded cover letter as suggested by Jan.
https://lore.kernel.org/all/[email protected]/

v1:
https://lore.kernel.org/all/[email protected]/

Lorenzo Stoakes (3):
mm: drop the assumption that VM_SHARED always implies writable
mm: update memfd seal write check to include F_SEAL_WRITE
mm: perform the mapping_map_writable() check after call_mmap()

fs/hugetlbfs/inode.c | 2 +-
include/linux/fs.h | 4 ++--
include/linux/mm.h | 26 +++++++++++++++++++-------
kernel/fork.c | 2 +-
mm/filemap.c | 2 +-
mm/madvise.c | 2 +-
mm/mmap.c | 27 ++++++++++++++++-----------
mm/shmem.c | 2 +-
8 files changed, 42 insertions(+), 25 deletions(-)

--
2.42.0


2023-10-12 17:04:53

by Lorenzo Stoakes

[permalink] [raw]
Subject: [PATCH v4 1/3] mm: drop the assumption that VM_SHARED always implies writable

There is a general assumption that VMAs with the VM_SHARED flag set are
writable. If the VM_MAYWRITE flag is not set, then this is simply not the
case.

Update those checks which affect the struct address_space->i_mmap_writable
field to explicitly test for this by introducing [vma_]is_shared_maywrite()
helper functions.

This remains entirely conservative, as the lack of VM_MAYWRITE guarantees
that the VMA cannot be written to.

Suggested-by: Andy Lutomirski <[email protected]>
Signed-off-by: Lorenzo Stoakes <[email protected]>
---
include/linux/fs.h | 4 ++--
include/linux/mm.h | 11 +++++++++++
kernel/fork.c | 2 +-
mm/filemap.c | 2 +-
mm/madvise.c | 2 +-
mm/mmap.c | 12 ++++++------
6 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 92a9c6157de1..e9c03fb00d5c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -454,7 +454,7 @@ extern const struct address_space_operations empty_aops;
* It is also used to block modification of page cache contents through
* memory mappings.
* @gfp_mask: Memory allocation flags to use for allocating pages.
- * @i_mmap_writable: Number of VM_SHARED mappings.
+ * @i_mmap_writable: Number of VM_SHARED, VM_MAYWRITE mappings.
* @nr_thps: Number of THPs in the pagecache (non-shmem only).
* @i_mmap: Tree of private and shared mappings.
* @i_mmap_rwsem: Protects @i_mmap and @i_mmap_writable.
@@ -557,7 +557,7 @@ static inline int mapping_mapped(struct address_space *mapping)

/*
* Might pages of this file have been modified in userspace?
- * Note that i_mmap_writable counts all VM_SHARED vmas: do_mmap
+ * Note that i_mmap_writable counts all VM_SHARED, VM_MAYWRITE vmas: do_mmap
* marks vma as VM_SHARED if it is shared, and the file was opened for
* writing i.e. vma may be mprotected writable even if now readonly.
*
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 74d7547ffb70..bae234d18d81 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -937,6 +937,17 @@ static inline bool vma_is_accessible(struct vm_area_struct *vma)
return vma->vm_flags & VM_ACCESS_FLAGS;
}

+static inline bool is_shared_maywrite(vm_flags_t vm_flags)
+{
+ return (vm_flags & (VM_SHARED | VM_MAYWRITE)) ==
+ (VM_SHARED | VM_MAYWRITE);
+}
+
+static inline bool vma_is_shared_maywrite(struct vm_area_struct *vma)
+{
+ return is_shared_maywrite(vma->vm_flags);
+}
+
static inline
struct vm_area_struct *vma_find(struct vma_iterator *vmi, unsigned long max)
{
diff --git a/kernel/fork.c b/kernel/fork.c
index e45a4457ba83..1e6c656e0857 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -733,7 +733,7 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,

get_file(file);
i_mmap_lock_write(mapping);
- if (tmp->vm_flags & VM_SHARED)
+ if (vma_is_shared_maywrite(tmp))
mapping_allow_writable(mapping);
flush_dcache_mmap_lock(mapping);
/* insert tmp into the share list, just after mpnt */
diff --git a/mm/filemap.c b/mm/filemap.c
index 9ef49255f1a5..9710f43a89ac 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3618,7 +3618,7 @@ int generic_file_mmap(struct file *file, struct vm_area_struct *vma)
*/
int generic_file_readonly_mmap(struct file *file, struct vm_area_struct *vma)
{
- if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
+ if (vma_is_shared_maywrite(vma))
return -EINVAL;
return generic_file_mmap(file, vma);
}
diff --git a/mm/madvise.c b/mm/madvise.c
index 70dafc99ff1e..6214a1ab5654 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -981,7 +981,7 @@ static long madvise_remove(struct vm_area_struct *vma,
return -EINVAL;
}

- if ((vma->vm_flags & (VM_SHARED|VM_WRITE)) != (VM_SHARED|VM_WRITE))
+ if (!vma_is_shared_maywrite(vma))
return -EACCES;

offset = (loff_t)(start - vma->vm_start)
diff --git a/mm/mmap.c b/mm/mmap.c
index 3ea52451623b..0041e3631f6c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -107,7 +107,7 @@ void vma_set_page_prot(struct vm_area_struct *vma)
static void __remove_shared_vm_struct(struct vm_area_struct *vma,
struct file *file, struct address_space *mapping)
{
- if (vma->vm_flags & VM_SHARED)
+ if (vma_is_shared_maywrite(vma))
mapping_unmap_writable(mapping);

flush_dcache_mmap_lock(mapping);
@@ -384,7 +384,7 @@ static unsigned long count_vma_pages_range(struct mm_struct *mm,
static void __vma_link_file(struct vm_area_struct *vma,
struct address_space *mapping)
{
- if (vma->vm_flags & VM_SHARED)
+ if (vma_is_shared_maywrite(vma))
mapping_allow_writable(mapping);

flush_dcache_mmap_lock(mapping);
@@ -2846,7 +2846,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
vma->vm_pgoff = pgoff;

if (file) {
- if (vm_flags & VM_SHARED) {
+ if (is_shared_maywrite(vm_flags)) {
error = mapping_map_writable(file->f_mapping);
if (error)
goto free_vma;
@@ -2920,7 +2920,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
mm->map_count++;
if (vma->vm_file) {
i_mmap_lock_write(vma->vm_file->f_mapping);
- if (vma->vm_flags & VM_SHARED)
+ if (vma_is_shared_maywrite(vma))
mapping_allow_writable(vma->vm_file->f_mapping);

flush_dcache_mmap_lock(vma->vm_file->f_mapping);
@@ -2937,7 +2937,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,

/* Once vma denies write, undo our temporary denial count */
unmap_writable:
- if (file && vm_flags & VM_SHARED)
+ if (file && is_shared_maywrite(vm_flags))
mapping_unmap_writable(file->f_mapping);
file = vma->vm_file;
ksm_add_vma(vma);
@@ -2985,7 +2985,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
unmap_region(mm, &vmi.mas, vma, prev, next, vma->vm_start,
vma->vm_end, vma->vm_end, true);
}
- if (file && (vm_flags & VM_SHARED))
+ if (file && is_shared_maywrite(vm_flags))
mapping_unmap_writable(file->f_mapping);
free_vma:
vm_area_free(vma);
--
2.42.0

2023-10-12 17:04:55

by Lorenzo Stoakes

[permalink] [raw]
Subject: [PATCH v4 3/3] mm: perform the mapping_map_writable() check after call_mmap()

In order for a F_SEAL_WRITE sealed memfd mapping to have an opportunity to
clear VM_MAYWRITE, we must be able to invoke the appropriate vm_ops->mmap()
handler to do so. We would otherwise fail the mapping_map_writable() check
before we had the opportunity to avoid it.

This patch moves this check after the call_mmap() invocation. Only memfd
actively denies write access causing a potential failure here (in
memfd_add_seals()), so there should be no impact on non-memfd cases.

This patch makes the userland-visible change that MAP_SHARED, PROT_READ
mappings of an F_SEAL_WRITE sealed memfd mapping will now succeed.

There is a delicate situation with cleanup paths assuming that a writable
mapping must have occurred in circumstances where it may now not have. In
order to ensure we do not accidentally mark a writable file unwritable by
mistake, we explicitly track whether we have a writable mapping and
unmap only if we do.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217238
Signed-off-by: Lorenzo Stoakes <[email protected]>
---
mm/mmap.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 0041e3631f6c..7f45a08e7973 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2752,6 +2752,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
unsigned long charged = 0;
unsigned long end = addr + len;
unsigned long merge_start = addr, merge_end = end;
+ bool writable_file_mapping = false;
pgoff_t vm_pgoff;
int error;
VMA_ITERATOR(vmi, mm, addr);
@@ -2846,17 +2847,19 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
vma->vm_pgoff = pgoff;

if (file) {
- if (is_shared_maywrite(vm_flags)) {
- error = mapping_map_writable(file->f_mapping);
- if (error)
- goto free_vma;
- }
-
vma->vm_file = get_file(file);
error = call_mmap(file, vma);
if (error)
goto unmap_and_free_vma;

+ if (vma_is_shared_maywrite(vma)) {
+ error = mapping_map_writable(file->f_mapping);
+ if (error)
+ goto close_and_free_vma;
+
+ writable_file_mapping = true;
+ }
+
/*
* Expansion is handled above, merging is handled below.
* Drivers should not alter the address of the VMA.
@@ -2920,8 +2923,10 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
mm->map_count++;
if (vma->vm_file) {
i_mmap_lock_write(vma->vm_file->f_mapping);
- if (vma_is_shared_maywrite(vma))
+ if (vma_is_shared_maywrite(vma)) {
mapping_allow_writable(vma->vm_file->f_mapping);
+ writable_file_mapping = true;
+ }

flush_dcache_mmap_lock(vma->vm_file->f_mapping);
vma_interval_tree_insert(vma, &vma->vm_file->f_mapping->i_mmap);
@@ -2937,7 +2942,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,

/* Once vma denies write, undo our temporary denial count */
unmap_writable:
- if (file && is_shared_maywrite(vm_flags))
+ if (writable_file_mapping)
mapping_unmap_writable(file->f_mapping);
file = vma->vm_file;
ksm_add_vma(vma);
@@ -2985,7 +2990,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
unmap_region(mm, &vmi.mas, vma, prev, next, vma->vm_start,
vma->vm_end, vma->vm_end, true);
}
- if (file && is_shared_maywrite(vm_flags))
+ if (writable_file_mapping)
mapping_unmap_writable(file->f_mapping);
free_vma:
vm_area_free(vma);
--
2.42.0

2023-10-12 17:06:05

by Lorenzo Stoakes

[permalink] [raw]
Subject: [PATCH v4 2/3] mm: update memfd seal write check to include F_SEAL_WRITE

The seal_check_future_write() function is called by shmem_mmap() or
hugetlbfs_file_mmap() to disallow any future writable mappings of an memfd
sealed this way.

The F_SEAL_WRITE flag is not checked here, as that is handled via the
mapping->i_mmap_writable mechanism and so any attempt at a mapping would
fail before this could be run.

However we intend to change this, meaning this check can be performed for
F_SEAL_WRITE mappings also.

The logic here is equally applicable to both flags, so update this function
to accommodate both and rename it accordingly.

Signed-off-by: Lorenzo Stoakes <[email protected]>
---
fs/hugetlbfs/inode.c | 2 +-
include/linux/mm.h | 15 ++++++++-------
mm/shmem.c | 2 +-
3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 06693bb1153d..5c333373dcc9 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -112,7 +112,7 @@ static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
vm_flags_set(vma, VM_HUGETLB | VM_DONTEXPAND);
vma->vm_ops = &hugetlb_vm_ops;

- ret = seal_check_future_write(info->seals, vma);
+ ret = seal_check_write(info->seals, vma);
if (ret)
return ret;

diff --git a/include/linux/mm.h b/include/linux/mm.h
index bae234d18d81..26d7dc3b342b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4078,25 +4078,26 @@ static inline void mem_dump_obj(void *object) {}
#endif

/**
- * seal_check_future_write - Check for F_SEAL_FUTURE_WRITE flag and handle it
+ * seal_check_write - Check for F_SEAL_WRITE or F_SEAL_FUTURE_WRITE flags and
+ * handle them.
* @seals: the seals to check
* @vma: the vma to operate on
*
- * Check whether F_SEAL_FUTURE_WRITE is set; if so, do proper check/handling on
- * the vma flags. Return 0 if check pass, or <0 for errors.
+ * Check whether F_SEAL_WRITE or F_SEAL_FUTURE_WRITE are set; if so, do proper
+ * check/handling on the vma flags. Return 0 if check pass, or <0 for errors.
*/
-static inline int seal_check_future_write(int seals, struct vm_area_struct *vma)
+static inline int seal_check_write(int seals, struct vm_area_struct *vma)
{
- if (seals & F_SEAL_FUTURE_WRITE) {
+ if (seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE)) {
/*
* New PROT_WRITE and MAP_SHARED mmaps are not allowed when
- * "future write" seal active.
+ * write seals are active.
*/
if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
return -EPERM;

/*
- * Since an F_SEAL_FUTURE_WRITE sealed memfd can be mapped as
+ * Since an F_SEAL_[FUTURE_]WRITE sealed memfd can be mapped as
* MAP_SHARED and read-only, take care to not allow mprotect to
* revert protections on such mappings. Do this only for shared
* mappings. For private mappings, don't need to mask
diff --git a/mm/shmem.c b/mm/shmem.c
index 6503910b0f54..cab053831fea 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2405,7 +2405,7 @@ static int shmem_mmap(struct file *file, struct vm_area_struct *vma)
struct shmem_inode_info *info = SHMEM_I(inode);
int ret;

- ret = seal_check_future_write(info->seals, vma);
+ ret = seal_check_write(info->seals, vma);
if (ret)
return ret;

--
2.42.0

2023-10-13 10:32:25

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] mm: update memfd seal write check to include F_SEAL_WRITE

On Thu 12-10-23 18:04:29, Lorenzo Stoakes wrote:
> The seal_check_future_write() function is called by shmem_mmap() or
> hugetlbfs_file_mmap() to disallow any future writable mappings of an memfd
> sealed this way.
>
> The F_SEAL_WRITE flag is not checked here, as that is handled via the
> mapping->i_mmap_writable mechanism and so any attempt at a mapping would
> fail before this could be run.
>
> However we intend to change this, meaning this check can be performed for
> F_SEAL_WRITE mappings also.
>
> The logic here is equally applicable to both flags, so update this function
> to accommodate both and rename it accordingly.
>
> Signed-off-by: Lorenzo Stoakes <[email protected]>

For some reason only this one patch landed in my inbox but I've checked all
three on lore and they look good to me. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

to all of them. Thanks!

Honza

> ---
> fs/hugetlbfs/inode.c | 2 +-
> include/linux/mm.h | 15 ++++++++-------
> mm/shmem.c | 2 +-
> 3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 06693bb1153d..5c333373dcc9 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -112,7 +112,7 @@ static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
> vm_flags_set(vma, VM_HUGETLB | VM_DONTEXPAND);
> vma->vm_ops = &hugetlb_vm_ops;
>
> - ret = seal_check_future_write(info->seals, vma);
> + ret = seal_check_write(info->seals, vma);
> if (ret)
> return ret;
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index bae234d18d81..26d7dc3b342b 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4078,25 +4078,26 @@ static inline void mem_dump_obj(void *object) {}
> #endif
>
> /**
> - * seal_check_future_write - Check for F_SEAL_FUTURE_WRITE flag and handle it
> + * seal_check_write - Check for F_SEAL_WRITE or F_SEAL_FUTURE_WRITE flags and
> + * handle them.
> * @seals: the seals to check
> * @vma: the vma to operate on
> *
> - * Check whether F_SEAL_FUTURE_WRITE is set; if so, do proper check/handling on
> - * the vma flags. Return 0 if check pass, or <0 for errors.
> + * Check whether F_SEAL_WRITE or F_SEAL_FUTURE_WRITE are set; if so, do proper
> + * check/handling on the vma flags. Return 0 if check pass, or <0 for errors.
> */
> -static inline int seal_check_future_write(int seals, struct vm_area_struct *vma)
> +static inline int seal_check_write(int seals, struct vm_area_struct *vma)
> {
> - if (seals & F_SEAL_FUTURE_WRITE) {
> + if (seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE)) {
> /*
> * New PROT_WRITE and MAP_SHARED mmaps are not allowed when
> - * "future write" seal active.
> + * write seals are active.
> */
> if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
> return -EPERM;
>
> /*
> - * Since an F_SEAL_FUTURE_WRITE sealed memfd can be mapped as
> + * Since an F_SEAL_[FUTURE_]WRITE sealed memfd can be mapped as
> * MAP_SHARED and read-only, take care to not allow mprotect to
> * revert protections on such mappings. Do this only for shared
> * mappings. For private mappings, don't need to mask
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 6503910b0f54..cab053831fea 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2405,7 +2405,7 @@ static int shmem_mmap(struct file *file, struct vm_area_struct *vma)
> struct shmem_inode_info *info = SHMEM_I(inode);
> int ret;
>
> - ret = seal_check_future_write(info->seals, vma);
> + ret = seal_check_write(info->seals, vma);
> if (ret)
> return ret;
>
> --
> 2.42.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-10-16 16:28:08

by Lorenzo Stoakes

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] mm: perform the mapping_map_writable() check after call_mmap()

On Thu, Oct 12, 2023 at 06:04:30PM +0100, Lorenzo Stoakes wrote:
> In order for a F_SEAL_WRITE sealed memfd mapping to have an opportunity to
> clear VM_MAYWRITE, we must be able to invoke the appropriate vm_ops->mmap()
> handler to do so. We would otherwise fail the mapping_map_writable() check
> before we had the opportunity to avoid it.
>
> This patch moves this check after the call_mmap() invocation. Only memfd
> actively denies write access causing a potential failure here (in
> memfd_add_seals()), so there should be no impact on non-memfd cases.
>
> This patch makes the userland-visible change that MAP_SHARED, PROT_READ
> mappings of an F_SEAL_WRITE sealed memfd mapping will now succeed.
>
> There is a delicate situation with cleanup paths assuming that a writable
> mapping must have occurred in circumstances where it may now not have. In
> order to ensure we do not accidentally mark a writable file unwritable by
> mistake, we explicitly track whether we have a writable mapping and
> unmap only if we do.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217238
> Signed-off-by: Lorenzo Stoakes <[email protected]>
> ---
> mm/mmap.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
[snip]

Andrew, could you apply the following -fix patch to this? As a bug was
detected in the implementation [0] - I was being over-zealous in setting
the writable_file_mapping flag and had falsely assumed vma->vm_file == file
in all instances of the cleanup. The fix is to only set it in one place.

[0]: https://lore.kernel.org/all/CA+G9fYtL7wK-dE-Tnz4t-GWmQb50EPYa=TWGjpgYU2Z=oeAO_w@mail.gmail.com/

----8<----
From 7feea6faada5b10a872c24755cc630220cba619a Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <[email protected]>
Date: Mon, 16 Oct 2023 17:17:13 +0100
Subject: [PATCH] mm: perform the mapping_map_writable() check after
call_mmap()

Do not set writable_file_mapping in an instance where it is not appropriate
to do so.

Signed-off-by: Lorenzo Stoakes <[email protected]>
---
mm/mmap.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 7f45a08e7973..8b57e42fd980 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2923,10 +2923,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
mm->map_count++;
if (vma->vm_file) {
i_mmap_lock_write(vma->vm_file->f_mapping);
- if (vma_is_shared_maywrite(vma)) {
+ if (vma_is_shared_maywrite(vma))
mapping_allow_writable(vma->vm_file->f_mapping);
- writable_file_mapping = true;
- }

flush_dcache_mmap_lock(vma->vm_file->f_mapping);
vma_interval_tree_insert(vma, &vma->vm_file->f_mapping->i_mmap);
--
2.42.0