Re: [PATCH hotfix 6.12 v2] mm/mlock: set the correct prev on failure

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Sun, Oct 27, 2024 at 12:33:21PM +0000, Wei Yang wrote:
> After commit 94d7d9233951 ("mm: abstract the vma_merge()/split_vma()
> pattern for mprotect() et al."), if vma_modify_flags() return error, the
> vma is set to an error code. This will lead to an invalid prev be
> returned.
>
> Generally this shouldn't matter as the caller should treat an error as
> indicating state is now invalidated, however unfortunately
> apply_mlockall_flags() does not check for errors and assumes that
> mlock_fixup() correctly maintains prev even if an error were to occur.
>
> This patch fixes that assumption.
>
> [lorenzo: provide a better fix and rephrase the log]
>
> Fixes: 94d7d9233951 ("mm: abstract the vma_merge()/split_vma() pattern for mprotect() et al.")
>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> CC: Liam R. Howlett <Liam.Howlett@Oracle.com>
> CC: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> CC: Vlastimil Babka <vbabka@suse.cz>
> CC: Jann Horn <jannh@google.com>
> Cc: <stable@vger.kernel.org>

Looks good to me,

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

>
> ---
> v2:
>    rearrange the fix and change log per Lorenzo's suggestion
>    add fix tag and cc stable
>
> ---
>  mm/mlock.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/mm/mlock.c b/mm/mlock.c
> index e3e3dc2b2956..cde076fa7d5e 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -725,14 +725,17 @@ static int apply_mlockall_flags(int flags)
>  	}
>
>  	for_each_vma(vmi, vma) {
> +		int error;
>  		vm_flags_t newflags;
>
>  		newflags = vma->vm_flags & ~VM_LOCKED_MASK;
>  		newflags |= to_add;
>
> -		/* Ignore errors */
> -		mlock_fixup(&vmi, vma, &prev, vma->vm_start, vma->vm_end,
> -			    newflags);
> +		error = mlock_fixup(&vmi, vma, &prev, vma->vm_start, vma->vm_end,
> +				    newflags);
> +		/* Ignore errors, but prev needs fixing up. */
> +		if (error)
> +			prev = vma;
>  		cond_resched();
>  	}
>  out:
> --
> 2.34.1
>