linux-kernel - Re: [PATCH 6.12.y] mm/mmap: fix __mmap_region() error handling in rare merge failure case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <qmmd4lujbzwyhxmjf3wagmfakbirjleufgkh6ozh5wbled3zp7@2z6trp6xlci7>
Date: Mon, 18 Nov 2024 15:32:14 -0500
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
        Jann Horn <jannh@...gle.com>,
        syzbot+bc6bfc25a68b7a020ee1@...kaller.appspotmail.com,
        Vlastimil Babka <vbabka@...e.cz>, stable@...r.kernel.org
Subject: Re: [PATCH 6.12.y] mm/mmap: fix __mmap_region() error handling in
 rare merge failure case

Okay, before I get yelled at...

This commit is only necessary for 6.12.y until Lorenzo's other fixes to
older stables land (and I'll have to figure out what to do in each).

The commit will not work on mm-unstable, because it doesn't exist due to
refactoring.

The commit does not have a tag about "upstream commit" because there
isn't one - the closest thing I could point to does not have a stable
git id.

So here I am with a fix for a kernel that was released a few hours ago
that is not necessary in v6.13, for a bug that's out there on syzkaller.

Also, it's very unlikely to happen unless you inject failures like
syzkaller.  But hey, pretty decent turn-around on finding a fix - so
that's a rosy outlook.


* Liam R. Howlett <Liam.Howlett@...cle.com> [241118 14:41]:
> From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
> 
> The mmap_region() function tries to install a new vma, which requires a
> pre-allocation for the maple tree write due to the complex locking
> scenarios involved.
> 
> Recent efforts to simplify the error recovery required the relocation of
> the preallocation of the maple tree nodes (via vma_iter_prealloc()
> calling mas_preallocate()) higher in the function.
> 
> The relocation of the preallocation meant that, if there was a file
> associated with the vma and the driver call (mmap_file()) modified the
> vma flags, then a new merge of the new vma with existing vmas is
> attempted.
> 
> During the attempt to merge the existing vma with the new vma, the vma
> iterator is used - the same iterator that would be used for the next
> write attempt to the tree.  In the event of needing a further allocation
> and if the new allocations fails, the vma iterator (and contained maple
> state) will cleaned up, including freeing all previous allocations and
> will be reset internally.
> 
> Upon returning to the __mmap_region() function, the error reason is lost
> and the function sets the vma iterator limits, and then tries to
> continue to store the new vma using vma_iter_store() - which expects
> preallocated nodes.
> 
> A preallocation should be performed in case the allocations were lost
> during the failure scenario - there is no risk of over allocating.  The
> range is already set in the vma_iter_store() call below, so it is not
> necessary.
> 
> Reported-by: syzbot+bc6bfc25a68b7a020ee1@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/x/log.txt?x=17b0ace8580000
> Fixes: 5de195060b2e2 ("mm: resolve faulty mmap_region() error path behaviour")
> Signed-off-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> Cc: Vlastimil Babka <vbabka@...e.cz>
> Cc: Jann Horn <jannh@...gle.com>
> Cc: <stable@...r.kernel.org>
> ---
>  mm/mmap.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 79d541f1502b2..5cef9a1981f1b 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1491,7 +1491,10 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
>  				vm_flags = vma->vm_flags;
>  				goto file_expanded;
>  			}
> -			vma_iter_config(&vmi, addr, end);
> +			if (vma_iter_prealloc(&vmi, vma)) {
> +				error = -ENOMEM;
> +				goto unmap_and_free_file_vma;
> +			}
>  		}
>  
>  		vm_flags = vma->vm_flags;
> -- 
> 2.43.0
>