It seems vma merging with uffd paths is broken with either register/unregister, where right now we can feed wrong parameters to vma_merge() and it's found by recent patch which moved asserts upwards in vma_merge() by Lorenzo Stoakes:
https://lore.kernel.org/all/ZFunF7DmMdK05MoF@FVFF77S0Q05N.cambridge.arm.com/
It's possible that "start" is contained within vma but not clamped to its start. We need to convert this into either "cannot merge" case or "can merge" case 4 which permits subdivision of prev by assigning vma to prev. As we loop, each subsequent VMA will be clamped to the start.
This patch will eliminate the report and make sure vma_merge() calls will become legal again.
One thing to mention is that the "Fixes: 29417d292bd0" below is there only to help explain where the warning can start to trigger, the real commit to fix should be 69dbe6daf104. Commit 29417d292bd0 helps us to identify the issue, but unfortunately we may want to keep it in Fixes too just to ease kernel backporters for easier tracking.
Cc: Lorenzo Stoakes lstoakes@gmail.com Cc: Mike Rapoport (IBM) rppt@kernel.org Cc: Liam R. Howlett Liam.Howlett@oracle.com Reported-by: Mark Rutland mark.rutland@arm.com Reviewed-by: Lorenzo Stoakes lstoakes@gmail.com Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Fixes: 29417d292bd0 ("mm/mmap/vma_merge: always check invariants") Fixes: 69dbe6daf104 ("userfaultfd: use maple tree iterator to iterate VMAs") Closes: https://lore.kernel.org/all/ZFunF7DmMdK05MoF@FVFF77S0Q05N.cambridge.arm.com/ Cc: linux-stable stable@vger.kernel.org Signed-off-by: Peter Xu peterx@redhat.com --- fs/userfaultfd.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 0fd96d6e39ce..17c8c345dac4 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -1459,6 +1459,8 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
vma_iter_set(&vmi, start); prev = vma_prev(&vmi); + if (vma->vm_start < start) + prev = vma;
ret = 0; for_each_vma_range(vmi, vma, end) { @@ -1625,6 +1627,9 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
vma_iter_set(&vmi, start); prev = vma_prev(&vmi); + if (vma->vm_start < start) + prev = vma; + ret = 0; for_each_vma_range(vmi, vma, end) { cond_resched();
On Wed, 17 May 2023 15:09:15 -0400 Peter Xu peterx@redhat.com wrote:
It seems vma merging with uffd paths is broken with either register/unregister, where right now we can feed wrong parameters to vma_merge() and it's found by recent patch which moved asserts upwards in vma_merge() by Lorenzo Stoakes:
https://lore.kernel.org/all/ZFunF7DmMdK05MoF@FVFF77S0Q05N.cambridge.arm.com/
It's possible that "start" is contained within vma but not clamped to its start. We need to convert this into either "cannot merge" case or "can merge" case 4 which permits subdivision of prev by assigning vma to prev. As we loop, each subsequent VMA will be clamped to the start.
This patch will eliminate the report and make sure vma_merge() calls will become legal again.
One thing to mention is that the "Fixes: 29417d292bd0" below is there only to help explain where the warning can start to trigger, the real commit to fix should be 69dbe6daf104. Commit 29417d292bd0 helps us to identify the issue, but unfortunately we may want to keep it in Fixes too just to ease kernel backporters for easier tracking.
Cc: Lorenzo Stoakes lstoakes@gmail.com Cc: Mike Rapoport (IBM) rppt@kernel.org Cc: Liam R. Howlett Liam.Howlett@oracle.com Reported-by: Mark Rutland mark.rutland@arm.com Reviewed-by: Lorenzo Stoakes lstoakes@gmail.com Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Fixes: 29417d292bd0 ("mm/mmap/vma_merge: always check invariants") Fixes: 69dbe6daf104 ("userfaultfd: use maple tree iterator to iterate VMAs")
I don't know how -stable maintainers are to handle more than a single Fixes: target, given that Fixes: means "kernels which have that patch need this one". Can we narrow this down to a single commit for this purpose?
On Wed, May 17, 2023 at 01:23:21PM -0700, Andrew Morton wrote:
On Wed, 17 May 2023 15:09:15 -0400 Peter Xu peterx@redhat.com wrote:
It seems vma merging with uffd paths is broken with either register/unregister, where right now we can feed wrong parameters to vma_merge() and it's found by recent patch which moved asserts upwards in vma_merge() by Lorenzo Stoakes:
https://lore.kernel.org/all/ZFunF7DmMdK05MoF@FVFF77S0Q05N.cambridge.arm.com/
It's possible that "start" is contained within vma but not clamped to its start. We need to convert this into either "cannot merge" case or "can merge" case 4 which permits subdivision of prev by assigning vma to prev. As we loop, each subsequent VMA will be clamped to the start.
This patch will eliminate the report and make sure vma_merge() calls will become legal again.
One thing to mention is that the "Fixes: 29417d292bd0" below is there only to help explain where the warning can start to trigger, the real commit to fix should be 69dbe6daf104. Commit 29417d292bd0 helps us to identify the issue, but unfortunately we may want to keep it in Fixes too just to ease kernel backporters for easier tracking.
Cc: Lorenzo Stoakes lstoakes@gmail.com Cc: Mike Rapoport (IBM) rppt@kernel.org Cc: Liam R. Howlett Liam.Howlett@oracle.com Reported-by: Mark Rutland mark.rutland@arm.com Reviewed-by: Lorenzo Stoakes lstoakes@gmail.com Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Fixes: 29417d292bd0 ("mm/mmap/vma_merge: always check invariants") Fixes: 69dbe6daf104 ("userfaultfd: use maple tree iterator to iterate VMAs")
I don't know how -stable maintainers are to handle more than a single Fixes: target, given that Fixes: means "kernels which have that patch need this one". Can we narrow this down to a single commit for this purpose?
Please just keep:
Fixes: 69dbe6daf104 ("userfaultfd: use maple tree iterator to iterate VMAs")
I just noticed 29417d292bd0 is only in rc1 so no backport needed anyway. We definitely need 69dbe6daf104 marked Fixes for backport till 6.1+.
Thanks,
linux-stable-mirror@lists.linaro.org