| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <vagk6hufcszqzrdqfszj23acsqb3xydxadmak7alprt4lvnt3l@dfyn5jtbazjr>
Date: Wed, 8 Jan 2025 10:01:18 -0500
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Suren Baghdasaryan <surenb@...gle.com>
Cc: akpm@...ux-foundation.org, peterz@...radead.org, willy@...radead.org,
lorenzo.stoakes@...cle.com, mhocko@...e.com, vbabka@...e.cz,
hannes@...xchg.org, mjguzik@...il.com, oliver.sang@...el.com,
mgorman@...hsingularity.net, david@...hat.com, peterx@...hat.com,
oleg@...hat.com, dave@...olabs.net, paulmck@...nel.org,
brauner@...nel.org, dhowells@...hat.com, hdanton@...a.com,
hughd@...gle.com, lokeshgidra@...gle.com, minchan@...gle.com,
jannh@...gle.com, shakeel.butt@...ux.dev, souravpanda@...gle.com,
pasha.tatashin@...een.com, klarasmodin@...il.com, corbet@....net,
linux-doc@...r.kernel.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH v7 03/17] mm: mark vma as detached until it's added into
vma tree
* Suren Baghdasaryan <surenb@...gle.com> [241226 12:07]:
> Current implementation does not set detached flag when a VMA is first
> allocated. This does not represent the real state of the VMA, which is
> detached until it is added into mm's VMA tree. Fix this by marking new
> VMAs as detached and resetting detached flag only after VMA is added into
> a tree.
>
> Introduce vma_mark_attached() to make the API more readable and to
> simplify possible future cleanup when vma->vm_mm might be used to indicate
> detached vma and vma_mark_attached() will need an additional mm parameter.
>
> Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
> Reviewed-by: Shakeel Butt <shakeel.butt@...ux.dev>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> Reviewed-by: Vlastimil Babka <vbabka@...e.cz>
Reviewed-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> ---
> include/linux/mm.h | 27 ++++++++++++++++++++-------
> kernel/fork.c | 4 ++++
> mm/memory.c | 2 +-
> mm/vma.c | 6 +++---
> mm/vma.h | 2 ++
> tools/testing/vma/vma_internal.h | 17 ++++++++++++-----
> 6 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index f3f92ba8f5fe..081178b0eec4 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -821,12 +821,21 @@ static inline void vma_assert_locked(struct vm_area_struct *vma)
> vma_assert_write_locked(vma);
> }
>
> -static inline void vma_mark_detached(struct vm_area_struct *vma, bool detached)
> +static inline void vma_mark_attached(struct vm_area_struct *vma)
> +{
> + vma->detached = false;
> +}
> +
> +static inline void vma_mark_detached(struct vm_area_struct *vma)
> {
> /* When detaching vma should be write-locked */
> - if (detached)
> - vma_assert_write_locked(vma);
> - vma->detached = detached;
> + vma_assert_write_locked(vma);
> + vma->detached = true;
> +}
> +
> +static inline bool is_vma_detached(struct vm_area_struct *vma)
> +{
> + return vma->detached;
> }
>
> static inline void release_fault_lock(struct vm_fault *vmf)
> @@ -857,8 +866,8 @@ static inline void vma_end_read(struct vm_area_struct *vma) {}
> static inline void vma_start_write(struct vm_area_struct *vma) {}
> static inline void vma_assert_write_locked(struct vm_area_struct *vma)
> { mmap_assert_write_locked(vma->vm_mm); }
> -static inline void vma_mark_detached(struct vm_area_struct *vma,
> - bool detached) {}
> +static inline void vma_mark_attached(struct vm_area_struct *vma) {}
> +static inline void vma_mark_detached(struct vm_area_struct *vma) {}
>
> static inline struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
> unsigned long address)
> @@ -891,7 +900,10 @@ static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
> vma->vm_mm = mm;
> vma->vm_ops = &vma_dummy_vm_ops;
> INIT_LIST_HEAD(&vma->anon_vma_chain);
> - vma_mark_detached(vma, false);
> +#ifdef CONFIG_PER_VMA_LOCK
> + /* vma is not locked, can't use vma_mark_detached() */
> + vma->detached = true;
> +#endif
> vma_numab_state_init(vma);
> vma_lock_init(vma);
> }
> @@ -1086,6 +1098,7 @@ static inline int vma_iter_bulk_store(struct vma_iterator *vmi,
> if (unlikely(mas_is_err(&vmi->mas)))
> return -ENOMEM;
>
> + vma_mark_attached(vma);
> return 0;
> }
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 40a8e615499f..f2f9e7b427ad 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -465,6 +465,10 @@ struct vm_area_struct *vm_area_dup(struct vm_area_struct *orig)
> data_race(memcpy(new, orig, sizeof(*new)));
> vma_lock_init(new);
> INIT_LIST_HEAD(&new->anon_vma_chain);
> +#ifdef CONFIG_PER_VMA_LOCK
> + /* vma is not locked, can't use vma_mark_detached() */
> + new->detached = true;
> +#endif
> vma_numab_state_init(new);
> dup_anon_vma_name(orig, new);
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 2a20e3810534..d0dee2282325 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -6349,7 +6349,7 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
> goto inval;
>
> /* Check if the VMA got isolated after we found it */
> - if (vma->detached) {
> + if (is_vma_detached(vma)) {
> vma_end_read(vma);
> count_vm_vma_lock_event(VMA_LOCK_MISS);
> /* The area was replaced with another one */
> diff --git a/mm/vma.c b/mm/vma.c
> index 0caaeea899a9..476146c25283 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -327,7 +327,7 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
>
> if (vp->remove) {
> again:
> - vma_mark_detached(vp->remove, true);
> + vma_mark_detached(vp->remove);
> if (vp->file) {
> uprobe_munmap(vp->remove, vp->remove->vm_start,
> vp->remove->vm_end);
> @@ -1220,7 +1220,7 @@ static void reattach_vmas(struct ma_state *mas_detach)
>
> mas_set(mas_detach, 0);
> mas_for_each(mas_detach, vma, ULONG_MAX)
> - vma_mark_detached(vma, false);
> + vma_mark_attached(vma);
>
> __mt_destroy(mas_detach->tree);
> }
> @@ -1295,7 +1295,7 @@ static int vms_gather_munmap_vmas(struct vma_munmap_struct *vms,
> if (error)
> goto munmap_gather_failed;
>
> - vma_mark_detached(next, true);
> + vma_mark_detached(next);
> nrpages = vma_pages(next);
>
> vms->nr_pages += nrpages;
> diff --git a/mm/vma.h b/mm/vma.h
> index 61ed044b6145..24636a2b0acf 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -157,6 +157,7 @@ static inline int vma_iter_store_gfp(struct vma_iterator *vmi,
> if (unlikely(mas_is_err(&vmi->mas)))
> return -ENOMEM;
>
> + vma_mark_attached(vma);
> return 0;
> }
>
> @@ -389,6 +390,7 @@ static inline void vma_iter_store(struct vma_iterator *vmi,
>
> __mas_set_range(&vmi->mas, vma->vm_start, vma->vm_end - 1);
> mas_store_prealloc(&vmi->mas, vma);
> + vma_mark_attached(vma);
> }
>
> static inline unsigned long vma_iter_addr(struct vma_iterator *vmi)
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index d19ce6fcab83..2a624f9304da 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -465,13 +465,17 @@ static inline void vma_lock_init(struct vm_area_struct *vma)
> vma->vm_lock_seq = UINT_MAX;
> }
>
> +static inline void vma_mark_attached(struct vm_area_struct *vma)
> +{
> + vma->detached = false;
> +}
> +
> static inline void vma_assert_write_locked(struct vm_area_struct *);
> -static inline void vma_mark_detached(struct vm_area_struct *vma, bool detached)
> +static inline void vma_mark_detached(struct vm_area_struct *vma)
> {
> /* When detaching vma should be write-locked */
> - if (detached)
> - vma_assert_write_locked(vma);
> - vma->detached = detached;
> + vma_assert_write_locked(vma);
> + vma->detached = true;
> }
>
> extern const struct vm_operations_struct vma_dummy_vm_ops;
> @@ -484,7 +488,8 @@ static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
> vma->vm_mm = mm;
> vma->vm_ops = &vma_dummy_vm_ops;
> INIT_LIST_HEAD(&vma->anon_vma_chain);
> - vma_mark_detached(vma, false);
> + /* vma is not locked, can't use vma_mark_detached() */
> + vma->detached = true;
> vma_lock_init(vma);
> }
>
> @@ -510,6 +515,8 @@ static inline struct vm_area_struct *vm_area_dup(struct vm_area_struct *orig)
> memcpy(new, orig, sizeof(*new));
> vma_lock_init(new);
> INIT_LIST_HEAD(&new->anon_vma_chain);
> + /* vma is not locked, can't use vma_mark_detached() */
> + new->detached = true;
Annoying that this is done in different locations, but that's a clean up
for another time.
>
> return new;
> }
> --
> 2.47.1.613.gc27f4b7a9f-goog
>
Powered by blists - more mailing lists