On Mon, Oct 28, 2024 at 11:00:43AM -0400, Liam R. Howlett wrote:
>* Wei Yang <richard.weiyang@xxxxxxxxx> [241027 08:34]:
>> 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>
>>
>> ---
>> 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;
>
>I don't think we need a local variable for the error since it's not used
>for anything besides ensuring there was a non-zero return here, but it
>probably doesn't make a difference. I'd have to check the assembly to
>be sure.
>
>Either way,
>
>Reviewed-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
>
Thanks
>> cond_resched();
>> }
>> out:
>> --
>> 2.34.1
>>
--
Wei Yang
Help you, Help me
