Re: [PATCH 2/2] mm: lock newly mapped VMA which can be modified after it becomes visible

[Liam R. Howlett]

* Suren Baghdasaryan <surenb@xxxxxxxxxx> [230707 00:32]:
> mmap_region adds a newly created VMA into VMA tree and might modify it
> afterwards before dropping the mmap_lock. This poses a problem for page
> faults handled under per-VMA locks because they don't take the mmap_lock
> and can stumble on this VMA while it's still being modified. Currently
> this does not pose a problem since post-addition modifications are done
> only for file-backed VMAs, which are not handled under per-VMA lock.
> However, once support for handling file-backed page faults with per-VMA
> locks is added, this will become a race.
> Fix this by write-locking the VMA before inserting it into the VMA tree.
> Other places where a new VMA is added into VMA tree do not modify it
> after the insertion, so do not need the same locking.
> 
> Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> ---
>  mm/mmap.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index c66e4622a557..84c71431a527 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2812,6 +2812,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
>  	if (vma->vm_file)
>  		i_mmap_lock_write(vma->vm_file->f_mapping);
>  
> +	/* Lock the VMA since it is modified after insertion into VMA tree */

So it is modified, but that i_mmap_lock_write() directly above this
comment is potentially moving below the insert and that is why this lock
is needed.

> +	vma_start_write(vma);
>  	vma_iter_store(&vmi, vma);
>  	mm->map_count++;
>  	if (vma->vm_file) {
> -- 
> 2.41.0.255.g8b1d071c50-goog
>