linux-kernel - Re: [PATCH v3] mm: use per_vma lock for MADV

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20250607103012.25332-1-21cnbao@gmail.com>
Date: Sat,  7 Jun 2025 22:30:12 +1200
From: Barry Song <21cnbao@...il.com>
To: lorenzo.stoakes@...cle.com
Cc: 21cnbao@...il.com,
	Liam.Howlett@...cle.com,
	akpm@...ux-foundation.org,
	david@...hat.com,
	jannh@...gle.com,
	linux-kernel@...r.kernel.org,
	linux-mm@...ck.org,
	lokeshgidra@...gle.com,
	surenb@...gle.com,
	v-songbaohua@...o.com,
	vbabka@...e.cz,
	zhengqi.arch@...edance.com,
	zhengtangquan@...o.com
Subject: Re: [PATCH v3] mm: use per_vma lock for MADV_DONTNEED

> Sort of a nice-to-have/thought but:
>
> Actually, when I proposed the refactor I wondered whether we'd use more state in
> madv_behaviour here but turns out we don't so we may as well just switch back to
> using int behavior here?
>
> If we do that then we can adjust process_madvise_remote_valid() with:
>
>  static bool process_madvise_remote_valid(int behavior)
>  {
> +       /* Due to lack of address untag atomicity, we need mmap lock. */
> +       VM_WARN_ON_ONCE(madvise_lock(behavior) != MADVISE_VMA_READ_LOCK);


process_madvise_remote_valid() is called before vector_madvise(), so I'm not
sure what this code is supposed to do. Are you trying to do something like:

VM_WARN_ON_ONCE(get_lock_mode(behavior) == MADVISE_VMA_READ_LOCK);

If so, that seems problematic — the same madvise operation might be allowed
to use the per-VMA lock for local processes, but disallowed for remote ones.

I suppose this will only start to make sense after we support per-VMA locking
for remote madvise operations such as "case MADV_XXX":

diff --git a/mm/madvise.c b/mm/madvise.c
index 8382614b71d1..9815445284d5 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1641,7 +1641,8 @@ int madvise_set_anon_name(struct mm_struct *mm, unsigned long start,
  * take mmap_lock for writing. Others, which simply traverse vmas, need
  * to only take it for reading.
  */
-static enum madvise_lock_mode get_lock_mode(struct madvise_behavior *madv_behavior)
+static enum madvise_lock_mode get_lock_mode(struct mm_struct *mm,
+		struct madvise_behavior *madv_behavior)
 {
 	int behavior = madv_behavior->behavior;
 
@@ -1659,6 +1660,9 @@ static enum madvise_lock_mode get_lock_mode(struct madvise_behavior *madv_behavi
 	case MADV_COLLAPSE:
 	case MADV_GUARD_INSTALL:
 	case MADV_GUARD_REMOVE:
		...
+ 	case MADV_XXX:
+		return current->mm == mm ? MADVISE_VMA_READ_LOCK :
+				MADVISE_MMAP_READ_LOCK;
 	case MADV_DONTNEED:
 	case MADV_DONTNEED_LOCKED:

Thanks
Barry