linux-kernel - Re: [PATCH] mm/vma: reset VMA iterator on commit

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: <10e36f9f-5318-40bf-9f64-e254c7ccfbb9@suse.cz>
Date: Fri, 6 Jun 2025 18:16:16 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Andrew Morton <akpm@...ux-foundation.org>
Cc: "Liam R . Howlett" <Liam.Howlett@...cle.com>, Jann Horn
 <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/vma: reset VMA iterator on commit_merge() OOM failure

On 6/6/25 14:50, Lorenzo Stoakes wrote:
> While an OOM failure in commit_merge() isn't really feasible due to the
> allocation which might fail (a maple tree pre-allocation) being 'too small
> to fail', we do need to handle this case correctly regardless.
> 
> In vma_merge_existing_range(), we can theoretically encounter failures
> which result in an OOM error in two ways - firstly dup_anon_vma() might
> fail with an OOM error, and secondly commit_merge() failing, ultimately, to
> pre-allocate a maple tree node.
> 
> The abort logic for dup_anon_vma() resets the VMA iterator to the initial
> range, ensuring that any logic looping on this iterator will correctly
> proceed to the next VMA.
> 
> However the commit_merge() abort logic does not do the same thing. This
> resulted in a syzbot report occurring because mlockall() iterates through
> VMAs, is tolerant of errors, but ended up with an incorrect previous VMA
> being specified due to incorrect iterator state.
> 
> While making this change, it became apparent we are duplicating logic - the
> logic introduced in commit 41e6ddcaa0f1 ("mm/vma: add give_up_on_oom option
> on modify/merge, use in uffd release") duplicates the vmg->give_up_on_oom
> check in both abort branches.
> 
> Additionally, we observe that we can perform the anon_dup check safely on
> dup_anon_vma() failure, as this will not be modified should this call fail.
> 
> Finally, we need to reset the iterator in both cases, so now we can simply
> use the exact same code to abort for both.
> 
> We remove the VM_WARN_ON(err != -ENOMEM) as it would be silly for this to
> be otherwise and it allows us to implement the abort check more neatly.
> 
> Reported-by: syzbot+d16409ea9ecc16ed261a@...kaller.appspotmail.com
> Closes: https://lore.kernel.org/linux-mm/6842cc67.a00a0220.29ac89.003b.GAE@google.com/
> Fixes: 47b16d0462a4 ("mm: abort vma_modify() on merge out of memory failure")
> Cc: stable@...r.kernel.org
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>

Nice.

Reviewed-by: Vlastimil Babka <vbabka@...e.cz>