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@xxxxxxxxx>
> CC: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> CC: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> CC: Vlastimil Babka <vbabka@xxxxxxx>
> CC: Jann Horn <jannh@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
Looks good to me,
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
>
> ---
> 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
>
