linux-kernel - Re: [PATCH hotfix 6.12] mm/mmap: correct error handling in mmap

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: <84837835-b483-4d15-9e0f-1719c350423e@suse.cz>
Date: Tue, 1 Oct 2024 18:42:47 +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>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, Bert Karwatzki <spasswolf@....de>,
 Vegard Nossum <vegard.nossum@...cle.com>
Subject: Re: [PATCH hotfix 6.12] mm/mmap: correct error handling in
 mmap_region()

On 10/1/24 15:37, Lorenzo Stoakes wrote:
> Commit f8d112a4e657 ("mm/mmap: avoid zeroing vma tree in mmap_region()")
> changed how error handling is performed in mmap_region().
> 
> The error value defaults to -ENOMEM, but then gets reassigned immediately
> to the result of vms_gather_munmap_vmas() if we are performing a MAP_FIXED
> mapping over existing VMAs (and thus unmapping them).
> 
> This overwrites the error value, potentially clearing it.
> 
> After this, we invoke may_expand_vm() and possibly vm_area_alloc(), and
> check to see if they failed. If they do so, then we perform error-handling
> logic, but importantly, we do NOT update the error code.
> 
> This means that, if vms_gather_munmap_vmas() succeeds, but one of these
> calls does not, the function will return indicating no error, but rather an
> address value of zero, which is entirely incorrect.
> 
> Correct this and avoid future confusion by strictly setting error on each
> and every occasion we jump to the error handling logic, and set the error
> code immediately prior to doing so.
> 
> This way we can see at a glance that the error code is always correct.
> 
> Many thanks to Vegard Nossum who spotted this issue in discussion around
> this problem.
> 
> Reported-by: Bert Karwatzki <spasswolf@....de>
> Link: https://lore.kernel.org/all/20241001023402.3374-1-spasswolf@web.de/

I'd be surprised if that fixed the reported issue, but yeah this is a bug to
fix anyway.

> Suggested-by: Vegard Nossum <vegard.nossum@...cle.com>
> Fixes: f8d112a4e657 ("mm/mmap: avoid zeroing vma tree in mmap_region()")
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>

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