| 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: <F2B8BED7-E2A9-4D0C-BCDC-5B83CC58302A@nvidia.com>
Date: Fri, 16 Jan 2026 15:47:23 -0500
From: Zi Yan <ziy@...dia.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
David Hildenbrand <david@...nel.org>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
Shakeel Butt <shakeel.butt@...ux.dev>, Jann Horn <jannh@...gle.com>,
linux-mm@...ck.org, linux-kernel@...r.kernel.org,
linux-rt-devel@...ts.linux.dev, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
Boqun Feng <boqun.feng@...il.com>, Waiman Long <longman@...hat.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Clark Williams <clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH RESEND 3/3] mm: add + use vma_is_stabilised(),
vma_assert_stabilised() helpers
Reply again using the right email. Sorry for the noise.
On 16 Jan 2026, at 8:36, Lorenzo Stoakes wrote:
> Sometimes we wish to assert that a VMA is stable, that is - the VMA cannot
> be changed underneath us. This will be the case if EITHER the VMA lock or
> the mmap lock is held.
>
> In order to be able to do so this patch adds a vma_is_stabilised()
> predicate.
>
> We specify this differently based on whether CONFIG_PER_VMA_LOCK is
> specified - if it is then naturally we check both whether a VMA lock is
> held or an mmap lock held, otherwise we need only check the mmap lock.
>
> Note that we only trigger the assert is CONFIG_DEBUG_VM is set, as having
> this lock unset would indicate a programmatic error, so a release kernel
> runtime assert doesn't make much sense.
>
> There are a couple places in the kernel where we already do this check -
> the anon_vma_name() helper in mm/madvise.c and vma_flag_set_atomic() in
> include/linux/mm.h, which we update to use vma_assert_stabilised().
>
> These were in fact implemented incorrectly - if neither the mmap lock nor
> the VMA lock were held, these asserts did not fire.
>
> However since these asserts are debug-only, and a large number of test
> configurations will have CONFIG_PER_VMA_LOCK set, it has likely had no
> real-world impact.
>
> This change corrects this mistake at any rate.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> ---
> include/linux/mm.h | 4 +---
> include/linux/mmap_lock.h | 23 ++++++++++++++++++++++-
> mm/madvise.c | 4 +---
> 3 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 44a2a9c0a92f..8707059f4d37 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1008,9 +1008,7 @@ static inline void vma_flag_set_atomic(struct vm_area_struct *vma,
> {
> unsigned long *bitmap = ACCESS_PRIVATE(&vma->flags, __vma_flags);
>
> - /* mmap read lock/VMA read lock must be held. */
> - if (!rwsem_is_locked(&vma->vm_mm->mmap_lock))
> - vma_assert_locked(vma);
Ideally, this should have been converted to use mmap_is_locked(vma->vm_mm)
in Patch 2. But this is a bug fix here, so that churn is not necessary.
> + vma_assert_stabilised(vma);
>
> if (__vma_flag_atomic_valid(vma, bit))
> set_bit((__force int)bit, bitmap);
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index 9f6932ffaaa0..711885cb5372 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
> @@ -66,7 +66,6 @@ static inline void __mmap_lock_trace_released(struct mm_struct *mm, bool write)
>
> #endif /* CONFIG_TRACING */
>
> -
> static inline bool mmap_lock_is_contended(struct mm_struct *mm)
> {
> return rwsem_is_contended(&mm->mmap_lock);
> @@ -272,6 +271,11 @@ static inline bool vma_is_locked(struct vm_area_struct *vma)
> return vma_is_read_locked(vma) || vma_is_write_locked(vma);
> }
>
> +static inline bool vma_is_stabilised(struct vm_area_struct *vma)
> +{
> + return vma_is_locked(vma) || mmap_is_locked(vma->vm_mm);
> +}
> +
> static inline void vma_assert_write_locked(struct vm_area_struct *vma)
> {
> VM_BUG_ON_VMA(!vma_is_write_locked(vma), vma);
> @@ -358,6 +362,11 @@ static inline struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
> return NULL;
> }
>
> +static inline bool vma_is_stabilised(struct vm_area_struct *vma)
> +{
> + return mmap_is_locked(vma->vm_mm);
> +}
> +
> static inline void vma_assert_locked(struct vm_area_struct *vma)
> {
> mmap_assert_locked(vma->vm_mm);
> @@ -463,4 +472,16 @@ static inline void mmap_read_unlock_non_owner(struct mm_struct *mm)
> up_read_non_owner(&mm->mmap_lock);
> }
>
> +/**
> + * vma_assert_stabilised() - assert that this VMA cannot be changed from
> + * underneath us either by having a VMA or mmap lock held.
> + * @vma: The VMA whose stability we wish to assess.
> + *
> + * Note that this will only trigger an assert if CONFIG_DEBUG_VM is set.
> + */
> +static inline void vma_assert_stabilised(struct vm_area_struct *vma)
> +{
> + VM_BUG_ON_VMA(!vma_is_stabilised(vma), vma);
> +}
> +
> #endif /* _LINUX_MMAP_LOCK_H */
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 4bf4c8c38fd3..1f3040688f04 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -109,9 +109,7 @@ void anon_vma_name_free(struct kref *kref)
>
> struct anon_vma_name *anon_vma_name(struct vm_area_struct *vma)
> {
> - if (!rwsem_is_locked(&vma->vm_mm->mmap_lock))
> - vma_assert_locked(vma);
> -
> + vma_assert_stabilised(vma);
> return vma->anon_name;
> }
>
LGTM.
Reviewed-by: Zi Yan <ziy@...dia.com>
Best Regards,
Yan, Zi
Powered by blists - more mailing lists