| From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> |
| Subject: mm/mremap: refactor initial parameter sanity checks |
| Date: Thu, 17 Jul 2025 17:55:52 +0100 |
| |
| We are currently checking some things later, and some things immediately. |
| Aggregate the checks and avoid ones that need not be made. |
| |
| Simplify things by aligning lengths immediately. Defer setting the delta |
| parameter until later, which removes some duplicate code in the hugetlb |
| case. |
| |
| We can safely perform the checks moved from mremap_to() to |
| check_mremap_params() because: |
| |
| * If we set a new address via vrm_set_new_addr(), then this is guaranteed |
| to not overlap nor to position the new VMA past TASK_SIZE, so there's no |
| need to check these later. |
| |
| * We can simply page align lengths immediately. We do not need to check for |
| overlap nor TASK_SIZE sanity after hugetlb alignment as this asserts |
| addresses are huge-aligned, then huge-aligns lengths, rounding down. This |
| means any existing overlap would have already been caught. |
| |
| Moving things around like this lays the groundwork for subsequent changes |
| to permit operations on batches of VMAs. |
| |
| No functional change intended. |
| |
| Link: https://lkml.kernel.org/r/c862d625c98b1abd861c406f2bfad8baf3287f83.1752770784.git.lorenzo.stoakes@oracle.com |
| Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> |
| Reviewed-by: Vlastimil Babka <vbabka@suse.cz> |
| Cc: Al Viro <viro@zeniv.linux.org.uk> |
| Cc: Christian Brauner <brauner@kernel.org> |
| Cc: Jan Kara <jack@suse.cz> |
| Cc: Jann Horn <jannh@google.com> |
| Cc: Liam Howlett <liam.howlett@oracle.com> |
| Cc: Peter Xu <peterx@redhat.com> |
| Cc: Rik van Riel <riel@surriel.com> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| --- |
| |
| mm/mremap.c | 29 ++++++++++++++--------------- |
| 1 file changed, 14 insertions(+), 15 deletions(-) |
| |
| --- a/mm/mremap.c~mm-mremap-refactor-initial-parameter-sanity-checks |
| +++ a/mm/mremap.c |
| @@ -1413,14 +1413,6 @@ static unsigned long mremap_to(struct vm |
| struct mm_struct *mm = current->mm; |
| unsigned long err; |
| |
| - /* Is the new length or address silly? */ |
| - if (vrm->new_len > TASK_SIZE || |
| - vrm->new_addr > TASK_SIZE - vrm->new_len) |
| - return -EINVAL; |
| - |
| - if (vrm_overlaps(vrm)) |
| - return -EINVAL; |
| - |
| if (vrm->flags & MREMAP_FIXED) { |
| /* |
| * In mremap_to(). |
| @@ -1525,7 +1517,12 @@ static unsigned long check_mremap_params |
| * for DOS-emu "duplicate shm area" thing. But |
| * a zero new-len is nonsensical. |
| */ |
| - if (!PAGE_ALIGN(vrm->new_len)) |
| + if (!vrm->new_len) |
| + return -EINVAL; |
| + |
| + /* Is the new length or address silly? */ |
| + if (vrm->new_len > TASK_SIZE || |
| + vrm->new_addr > TASK_SIZE - vrm->new_len) |
| return -EINVAL; |
| |
| /* Remainder of checks are for cases with specific new_addr. */ |
| @@ -1544,6 +1541,10 @@ static unsigned long check_mremap_params |
| if (flags & MREMAP_DONTUNMAP && vrm->old_len != vrm->new_len) |
| return -EINVAL; |
| |
| + /* Target VMA must not overlap source VMA. */ |
| + if (vrm_overlaps(vrm)) |
| + return -EINVAL; |
| + |
| /* |
| * move_vma() need us to stay 4 maps below the threshold, otherwise |
| * it will bail out at the very beginning. |
| @@ -1620,8 +1621,6 @@ static bool align_hugetlb(struct vma_rem |
| if (vrm->new_len > vrm->old_len) |
| return false; |
| |
| - vrm_set_delta(vrm); |
| - |
| return true; |
| } |
| |
| @@ -1721,14 +1720,13 @@ static unsigned long do_mremap(struct vm |
| struct vm_area_struct *vma; |
| unsigned long res; |
| |
| + vrm->old_len = PAGE_ALIGN(vrm->old_len); |
| + vrm->new_len = PAGE_ALIGN(vrm->new_len); |
| + |
| res = check_mremap_params(vrm); |
| if (res) |
| return res; |
| |
| - vrm->old_len = PAGE_ALIGN(vrm->old_len); |
| - vrm->new_len = PAGE_ALIGN(vrm->new_len); |
| - vrm_set_delta(vrm); |
| - |
| if (mmap_write_lock_killable(mm)) |
| return -EINTR; |
| vrm->mmap_locked = true; |
| @@ -1751,6 +1749,7 @@ static unsigned long do_mremap(struct vm |
| goto out; |
| } |
| |
| + vrm_set_delta(vrm); |
| vrm->remap_type = vrm_remap_type(vrm); |
| |
| /* Actually execute mremap. */ |
| _ |