Re: [PATCH hotfix 6.12] mm/mmap: correct error handling in mmap_region()

[Vlastimil Babka]

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@xxxxxx>
> Link: https://lore.kernel.org/all/20241001023402.3374-1-spasswolf@xxxxxx/

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@xxxxxxxxxx>
> Fixes: f8d112a4e657 ("mm/mmap: avoid zeroing vma tree in mmap_region()")
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>

Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>