* Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> [241027 07:43]:
> + Vlastimil, Liam, Jann as this is VMA-related.
>
> We really need to bring all VMA-ish files under the VMA MAINTAINERS
> block... will maybe address that once things around that file... calm down
> a bit.
>
> But please cc all of us on anything that even vaguely relates to VMAs,
> thanks!
>
> On Sun, Oct 27, 2024 at 02:56:29AM +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.
>
> This is a great spot, but this commit message is missing critical
> details. This is only meaningful for apply_mlockall_flags() which is both
> ignoring errors AND assuming mlock_fixup(), even on error, is correctly
> updating the prev state. Which is imo wrong.
>
> So I'd _add_ a bit more information here like:
>
> 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.
>
> We'll also need to backport this, so a:
>
> Fixes: 94d7d9233951 ("mm: abstract the vma_merge()/split_vma() pattern for mprotect() et al.")
> Cc: <stable@xxxxxxxxxxxxxxx>
>
> Needs to be added, and make the next revision [PATCH hotfix 6.12 v2] to
> make it clear this needs to go to 6.12.
>
> >
> > Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
> > CC: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> > ---
> > mm/mlock.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/mm/mlock.c b/mm/mlock.c
> > index e3e3dc2b2956..8c3f9cf8f960 100644
> > --- a/mm/mlock.c
> > +++ b/mm/mlock.c
> > @@ -478,11 +478,12 @@ static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,
> > /* don't set VM_LOCKED or VM_LOCKONFAULT and don't count */
> > goto out;
> >
> > - vma = vma_modify_flags(vmi, *prev, vma, start, end, newflags);
> > - if (IS_ERR(vma)) {
> > - ret = PTR_ERR(vma);
> > + *prev = vma_modify_flags(vmi, *prev, vma, start, end, newflags);
> > + if (IS_ERR(*prev)) {
> > + ret = PTR_ERR(*prev);
> > goto out;
> > }
> > + vma = *prev;
If we move the assignment *prev = vma to the start of this function,
then we can just get rid of the "out:" label and return on errors.
But the v2 seems fine,
Liam
