mm/mseal: update VMA end correctly on merge
Subsystems (3)
Lorenzo Stoakes (Oracle)
maintainerPATCH 1/1 mm/mseal: update VMA end correctly on merge
Previously we stored the end of the current VMA in curr_end, and then upon iterating to the next VMA updated curr_start to curr_end to advance to the next VMA.
However, this doesn't take into account the fact that a VMA might be updated due to a merge by vma_modify_flags(), which can result in curr_end being stale and thus, upon setting curr_start to curr_end, ending up with an incorrect curr_start on the next iteration.
Resolve the issue by setting curr_end to vma->vm_end unconditionally to ensure this value remains updated should this occur.
--- mm/mseal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
mm/mseal.c +2 -1
@@ -66,7 +66,7 @@ static int mseal_apply(struct mm_struct *mm, prev = vma;for_each_vma_range (vmi, vma, end) { -const unsigned long curr_end =MIN (vma->vm_end, end); +unsigned long curr_end =MIN (vma->vm_end, end);if (!(vma->vm_flags &VM_SEALED )) {vm_flags_t vm_flags = vma->vm_flags |VM_SEALED ;
@@ -76,6 +76,7 @@ static int mseal_apply(struct mm_struct *mm,if (IS_ERR (vma))return PTR_ERR (vma);vm_flags_set (vma,VM_SEALED ); + curr_end = vma->vm_end;/* Merge may have updated. */ } prev = vma;
--
2.53.0
Quoted 9 lines · Lorenzo Stoakes (Oracle)
Previously we stored the end of the current VMA in
curr_end, and then upon iterating to the next VMA updatedcurr_starttocurr_endto advance to the next VMA.However, this doesn't take into account the fact that a VMA might be updated due to a merge by
vma_modify_flags(), which can result incurr_endbeing stale and thus, upon settingcurr_starttocurr_end, ending up with an incorrectcurr_starton the next iteration.
Quoted 8 lines
Resolve the issue by setting
curr_endtovma->vm_endunconditionally to ensure this value remains updated should this occur.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>Fixes 6c2da14ae1e0 ("mm/mseal: rework mseal apply logic")Cc <[email protected]>Reported-by Antonius <[email protected]>
Signature · 1 line
Pedro
Lorenzo Stoakes (Oracle)
maintainerQuoted 9 lines · Lorenzo Stoakes (Oracle)
Previously we stored the end of the current VMA in
curr_end, and then upon iterating to the next VMA updatedcurr_starttocurr_endto advance to the next VMA.However, this doesn't take into account the fact that a VMA might be updated due to a merge by
vma_modify_flags(), which can result incurr_endbeing stale and thus, upon settingcurr_starttocurr_end, ending up with an incorrectcurr_starton the next iteration.
Quoted 8 lines
Resolve the issue by setting
curr_endtovma->vm_endunconditionally to ensure this value remains updated should this occur.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>Fixes 6c2da14ae1e0 ("mm/mseal: rework mseal apply logic")Cc <[email protected]>Reported-by Antonius <[email protected]>
Oops, Closes should be:
https://lore.kernel.org/linux-mm/CAK8a0jwWGj9-SgFk0yKFh7i8jMkwKm5b0ao9=kmXWjO54veX2g@mail.gmail.com/
Cheers, Lorenzo
Quoted 27 lines
--- mm/mseal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)mm/mseal.c +2
-2@@ -66,7 +66,7 @@ static int mseal_apply(struct mm_struct *mm, prev = vma;for_each_vma_range (vmi, vma, end) { -const unsigned long curr_end =MIN (vma->vm_end, end); +unsigned long curr_end =MIN (vma->vm_end, end);if (!(vma->vm_flags &VM_SEALED )) {vm_flags_t vm_flags = vma->vm_flags |VM_SEALED ;@@ -76,6 +76,7 @@ static int mseal_apply(struct mm_struct *mm,if (IS_ERR (vma))return PTR_ERR (vma);vm_flags_set (vma,VM_SEALED ); + curr_end = vma->vm_end;/* Merge may have updated. */ } prev = vma; --2.53.0
Quoted 9 lines · Lorenzo Stoakes (Oracle)
Previously we stored the end of the current VMA in
curr_end, and then upon iterating to the next VMA updatedcurr_starttocurr_endto advance to the next VMA.However, this doesn't take into account the fact that a VMA might be updated due to a merge by
vma_modify_flags(), which can result incurr_endbeing stale and thus, upon settingcurr_starttocurr_end, ending up with an incorrectcurr_starton the next iteration.
Quoted 8 lines
Resolve the issue by setting
curr_endtovma->vm_endunconditionally to ensure this value remains updated should this occur.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>Fixes 6c2da14ae1e0 ("mm/mseal: rework mseal apply logic")Cc <[email protected]>Reported-by Antonius <[email protected]>
Thanks!
Quoted 27 lines
--- mm/mseal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)mm/mseal.c +2
-2@@ -66,7 +66,7 @@ static int mseal_apply(struct mm_struct *mm, prev = vma;for_each_vma_range (vmi, vma, end) { -const unsigned long curr_end =MIN (vma->vm_end, end); +unsigned long curr_end =MIN (vma->vm_end, end);if (!(vma->vm_flags &VM_SEALED )) {vm_flags_t vm_flags = vma->vm_flags |VM_SEALED ;@@ -76,6 +76,7 @@ static int mseal_apply(struct mm_struct *mm,if (IS_ERR (vma))return PTR_ERR (vma);vm_flags_set (vma,VM_SEALED ); + curr_end = vma->vm_end;/* Merge may have updated. */ } prev = vma; --2.53.0
Andrew Morton
maintainerQuoted 12 lines · On Fri, 27 Mar 2026 09:06:40 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
mm/mseal.c +1
-1@@ -66,7 +66,7 @@ static int mseal_apply(struct mm_struct *mm, prev = vma;for_each_vma_range (vmi, vma, end) { -const unsigned long curr_end =MIN (vma->vm_end, end); +unsigned long curr_end =MIN (vma->vm_end, end);if (!(vma->vm_flags &VM_SEALED )) {vm_flags_t vm_flags = vma->vm_flags |VM_SEALED ;
mm/mseal.c static int mseal_apply(struct mm_struct *mm, · line 82
Quoted 7 lines
mm/mseal.c +1
-0@@ -76,6 +76,7 @@ static int mseal_apply(struct mm_struct *mm,if (IS_ERR (vma))return PTR_ERR (vma);vm_flags_set (vma,VM_SEALED ); + curr_end = vma->vm_end;/* Merge may have updated. */ } prev = vma;
This led to some rework in your "mm/vma: convert vma_modify_flags[_uffd]() to use vma_flags_t". Please check my handiwork.
reject:
mseal.c renamed from mseal.c~mm-vma-convert-vma_modify_flags-to-use-vma_flags_t +7 -4
@@ -68,14 +68,17 @@ static int mseal_apply(struct mm_structfor_each_vma_range (vmi, vma, end) {const unsigned long curr_end =MIN (vma->vm_end, end); -if (!(vma->vm_flags &VM_SEALED )) { -vm_flags_t vm_flags = vma->vm_flags |VM_SEALED ; +if (!vma_test (vma,VMA_SEALED_BIT )) { +vma_flags_t vma_flags = vma->flags; + +vma_flags_set (&vma_flags,VMA_SEALED_BIT ); vma =vma_modify_flags (&vmi, prev, vma, curr_start, - curr_end, &vm_flags); + curr_end, &vma_flags);if (IS_ERR (vma))return PTR_ERR (vma); -vm_flags_set (vma,VM_SEALED ); +vma_start_write (vma); +vma_set_flags (vma,VMA_SEALED_BIT ); } prev = vma;
resolution:
static int mseal_apply(struct mm_struct *mm,
unsigned long start, unsigned long end)
{
struct vm_area_struct *vma, *prev;
unsigned long curr_start = start;
VMA_ITERATOR(vmi, mm, start);
/* We know there are no gaps so this will be non-NULL. */
vma = vma_iter_load(&vmi);
prev = vma_prev(&vmi);
if (start > vma->vm_start)
prev = vma;
for_each_vma_range(vmi, vma, end) {
unsigned long curr_end = MIN(vma->vm_end, end);
if (!vma_test(vma, VMA_SEALED_BIT)) {
vma_flags_t vma_flags = vma->flags;
vma_flags_set(&vma_flags, VMA_SEALED_BIT);
vma = vma_modify_flags(&vmi, prev, vma, curr_start,
curr_end, &vma_flags);
if (IS_ERR(vma))
return PTR_ERR(vma);
vma_start_write(vma);
vma_set_flags(vma, VMA_SEALED_BIT);
curr_end = vma->vm_end; /* Merge may have updated. */
}
prev = vma;
curr_start = curr_end;
}
return 0;
}Lorenzo Stoakes (Oracle)
maintainerQuoted 83 lines · Andrew Morton
Quoted 12 lines · On Fri, 27 Mar 2026 09:06:40 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
mm/mseal.c +1
-1@@ -66,7 +66,7 @@ static int mseal_apply(struct mm_struct *mm, prev = vma;for_each_vma_range (vmi, vma, end) { -const unsigned long curr_end =MIN (vma->vm_end, end); +unsigned long curr_end =MIN (vma->vm_end, end);if (!(vma->vm_flags &VM_SEALED )) {vm_flags_t vm_flags = vma->vm_flags |VM_SEALED ;mm/mseal.c static int mseal_apply(struct mm_struct *mm, · line 82
Quoted 7 lines
mm/mseal.c +1
-0@@ -76,6 +76,7 @@ static int mseal_apply(struct mm_struct *mm,if (IS_ERR (vma))return PTR_ERR (vma);vm_flags_set (vma,VM_SEALED ); + curr_end = vma->vm_end;/* Merge may have updated. */ } prev = vma;This led to some rework in your "mm/vma: convert vma_modify_flags[_uffd]() to use
vma_flags_t". Please check my handiwork.reject:
mseal.c renamed from mseal.c~mm-vma-convert-vma_modify_flags-to-use-vma_flags_t +7
-4@@ -68,14 +68,17 @@ static int mseal_apply(struct mm_structfor_each_vma_range (vmi, vma, end) {const unsigned long curr_end =MIN (vma->vm_end, end); -if (!(vma->vm_flags &VM_SEALED )) { -vm_flags_t vm_flags = vma->vm_flags |VM_SEALED ; +if (!vma_test (vma,VMA_SEALED_BIT )) { +vma_flags_t vma_flags = vma->flags; + +vma_flags_set (&vma_flags,VMA_SEALED_BIT ); vma =vma_modify_flags (&vmi, prev, vma, curr_start, - curr_end, &vm_flags); + curr_end, &vma_flags);if (IS_ERR (vma))return PTR_ERR (vma); -vm_flags_set (vma,VM_SEALED ); +vma_start_write (vma); +vma_set_flags (vma,VMA_SEALED_BIT ); } prev = vma;resolution:
static int mseal_apply(struct
mm_struct*mm,
unsigned long start, unsigned long end){ struct vm_area_struct *vma, *prev; unsigned long curr_start = start; VMA_ITERATOR(vmi, mm, start); /* We know there are no gaps so this will be non-NULL. */ vma = vma_iter_load(&vmi); prev = vma_prev(&vmi); if (start > vma->vm_start) prev = vma; for_each_vma_range(vmi, vma, end) { unsigned long curr_end = MIN(vma->vm_end, end); if (!vma_test(vma, VMA_SEALED_BIT)) { vma_flags_t vma_flags = vma->flags; vma_flags_set(&vma_flags, VMA_SEALED_BIT); vma = vma_modify_flags(&vmi, prev, vma, curr_start, curr_end, &vma_flags); if (IS_ERR(vma)) return PTR_ERR(vma); vma_start_write(vma); vma_set_flags(vma, VMA_SEALED_BIT); curr_end = vma->vm_end; /* Merge may have updated. */
Quoted 9 lines
} prev = vma; curr_start = curr_end; } return 0; }
Thanks that looks correct!
Cheers, Lorenzo
Quoted 35 lines · Lorenzo Stoakes (Oracle)
Previously we stored the end of the current VMA in
curr_end, and then upon iterating to the next VMA updatedcurr_starttocurr_endto advance to the next VMA.However, this doesn't take into account the fact that a VMA might be updated due to a merge by
vma_modify_flags(), which can result incurr_endbeing stale and thus, upon settingcurr_starttocurr_end, ending up with an incorrectcurr_starton the next iteration.Resolve the issue by setting
curr_endtovma->vm_endunconditionally to ensure this value remains updated should this occur.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>Fixes 6c2da14ae1e0 ("mm/mseal: rework mseal apply logic")Cc <[email protected]>Reported-by Antonius <[email protected]>--- mm/mseal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)mm/mseal.c +1
-1@@ -66,7 +66,7 @@ static int mseal_apply(struct mm_struct *mm, prev = vma;for_each_vma_range (vmi, vma, end) { -const unsigned long curr_end =MIN (vma->vm_end, end); +unsigned long curr_end =MIN (vma->vm_end, end);if (!(vma->vm_flags &VM_SEALED )) {vm_flags_t vm_flags = vma->vm_flags |VM_SEALED ;
mm/mseal.c static int mseal_apply(struct mm_struct *mm, · line 80
Quoted 5 lines
mm/mseal.c +1
-0@@ -76,6 +76,7 @@ static int mseal_apply(struct mm_struct *mm,if (IS_ERR (vma))return PTR_ERR (vma);vm_flags_set (vma,VM_SEALED ); + curr_end = vma->vm_end;/* Merge may have updated. */ }
I was a bit confused why curr_start is allowed to not start within the VMA, but before it. Then I recalled that range_contains_unmapped() checks for no holes.
Would the following also sort out the problem and even simplify the code?
mm/mseal.c +1 -2
@@ -56,7 +56,6 @@ static int mseal_apply(struct mm_struct *mm, unsigned long start, unsigned long end) {struct vm_area_struct *vma, *prev; -unsigned long curr_start = start;VMA_ITERATOR (vmi, mm, start);/* We know there are no gaps so this will be non-NULL. */
@@ -66,6 +65,7 @@ static int mseal_apply(struct mm_struct *mm, prev = vma;for_each_vma_range (vmi, vma, end) { +const unsigned long curr_start =MAX (vma->vm_start, start);const unsigned long curr_end =MIN (vma->vm_end, end);if (!vma_test (vma,VMA_SEALED_BIT )) {
@@ -82,7 +82,6 @@ static int mseal_apply(struct mm_struct *mm, } prev = vma; - curr_start = curr_end; }return 0 ;
mm/page_alloc.c +0 -0
I might be wrong about that, I've been staring at the screen for too long today.
Signature · 3 lines
Cheers, David
Lorenzo Stoakes (Oracle)
maintainerQuoted 62 lines · David Hildenbrand (Arm)
Quoted 35 lines · Lorenzo Stoakes (Oracle)
Previously we stored the end of the current VMA in
curr_end, and then upon iterating to the next VMA updatedcurr_starttocurr_endto advance to the next VMA.However, this doesn't take into account the fact that a VMA might be updated due to a merge by
vma_modify_flags(), which can result incurr_endbeing stale and thus, upon settingcurr_starttocurr_end, ending up with an incorrectcurr_starton the next iteration.Resolve the issue by setting
curr_endtovma->vm_endunconditionally to ensure this value remains updated should this occur.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>Fixes 6c2da14ae1e0 ("mm/mseal: rework mseal apply logic")Cc <[email protected]>Reported-by Antonius <[email protected]>--- mm/mseal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)mm/mseal.c +1
-1@@ -66,7 +66,7 @@ static int mseal_apply(struct mm_struct *mm, prev = vma;for_each_vma_range (vmi, vma, end) { -const unsigned long curr_end =MIN (vma->vm_end, end); +unsigned long curr_end =MIN (vma->vm_end, end);if (!(vma->vm_flags &VM_SEALED )) {vm_flags_t vm_flags = vma->vm_flags |VM_SEALED ;mm/mseal.c static int mseal_apply(struct mm_struct *mm, · line 80
Quoted 5 lines
mm/mseal.c +1
-0@@ -76,6 +76,7 @@ static int mseal_apply(struct mm_struct *mm,if (IS_ERR (vma))return PTR_ERR (vma);vm_flags_set (vma,VM_SEALED ); + curr_end = vma->vm_end;/* Merge may have updated. */ }I was a bit confused why
curr_startis allowed to not start within the VMA, but before it. Then I recalled thatrange_contains_unmapped()checks for no holes.Would the following also sort out the problem and even simplify the code?
mm/mseal.c +0
-1@@ -56,7 +56,6 @@ static int mseal_apply(struct mm_struct *mm, unsigned long start, unsigned long end) {struct vm_area_struct *vma, *prev; -unsigned long curr_start = start;VMA_ITERATOR (vmi, mm, start);/* We know there are no gaps so this will be non-NULL. */
mm/mseal.c static int mseal_apply(struct mm_struct *mm, · line 68
Quoted 4 lines
mm/mseal.c +1
-0@@ -66,6 +65,7 @@ static int mseal_apply(struct mm_struct *mm, prev = vma;for_each_vma_range (vmi, vma, end) { +const unsigned long curr_start =MAX (vma->vm_start, start);
Yeah that's nice :)
Quoted 14 lines
mm/mseal.c +0
-1const unsigned long curr_end =MIN (vma->vm_end, end);if (!vma_test (vma,VMA_SEALED_BIT )) {@@ -82,7 +82,6 @@ static int mseal_apply(struct mm_struct *mm, } prev = vma; - curr_start = curr_end; }return 0 ;mm/page_alloc.c +0
-0
Quoted 1 line
I might be wrong about that, I've been staring at the screen for too long today.
No this is better, I've mulled this over, this is much better :)
Sorry to be a pain Andrew - let me respin this right now.
I will retain tags as this is functionally equivalent, I have checked it locally, confirmed repro solved, checked against AI review also.
Quoted 6 lines
--
Cheers,David
Cheers, Lorenzo