linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v1] mm/kasan: Fix incorrect unpoisoning in vrealloc for KASAN
@ 2025-12-03  7:30 Maciej Wieczór-Retman
  0 siblings, 0 replies; 10+ messages in thread
From: Maciej Wieczór-Retman @ 2025-12-03  7:30 UTC (permalink / raw)
  To: Jiayuan Chen
  Cc: Maciej Wieczor-Retman, linux-mm, syzbot+997752115a851cb0cf36,
	Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, Vincenzo Frascino, Andrew Morton,
	Uladzislau Rezki, Danilo Krummrich, Kees Cook, kasan-dev,
	linux-kernel

On 2025-12-03 at 02:05:11 +0000, Jiayuan Chen wrote:
>December 3, 2025 at 04:48, "Maciej Wieczor-Retman"
><maciej.wieczor-retman@intel.com
>mailto:maciej.wieczor-retman@intel.com?to=%22Maciej%20Wieczor-Retman%22%20%3Cmaciej.wieczor-retman%40intel.com%3E
>> wrote:
>> 
>> Hi, I'm working on [1]. As Andrew pointed out to me the patches are quite
>> similar. I was wondering if you mind if the reuse_tag was an actual tag value?
>> Instead of just bool toggling the usage of kasan_random_tag()?
>> 
>> I tested the problem I'm seeing, with your patch and the tags end up being reset.
>> That's because the vms[area] pointers that I want to unpoison don't have a tag
>> set, but generating a different random tag for each vms[] pointer crashes the
>> kernel down the line. So __kasan_unpoison_vmalloc() needs to be called on each
>> one but with the same tag.
>> 
>> Arguably I noticed my series also just resets the tags right now, but I'm
>> working to correct it at the moment. I can send a fixed version tomorrow. Just
>> wanted to ask if having __kasan_unpoison_vmalloc() set an actual predefined tag
>> is a problem from your point of view?
>> 
>> [1] https://lore.kernel.org/all/cover.1764685296.git.m.wieczorretman@pm.me/
>> 
>
>
>Hi Maciej,
>
>It seems we're focusing on different issues, but feel free to reuse or modify the 'reuse_tag'.
>It's intended to preserve the tag in one 'vma'.
>
>I'd also be happy to help reproduce and test your changes to ensure the issue I encountered
>isn't regressed once you send a patch based on mine. 
>
>Thanks.

Yes, the final issues are different, just we both want to use
__kasan_unpoison_vmalloc slightly differently.

Okay, then I'll rebase my patches onto your patch, restest on my end and
resubmit my series. I'll add you to CC and reply in this thread too.
Thanks :)

kind regards
Maciej Wieczór-Retman



^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v1] mm/kasan: Fix incorrect unpoisoning in vrealloc for KASAN
@ 2025-12-04 13:55 Maciej Wieczor-Retman
  2025-12-04 14:38 ` Jiayuan Chen
  0 siblings, 1 reply; 10+ messages in thread
From: Maciej Wieczor-Retman @ 2025-12-04 13:55 UTC (permalink / raw)
  To: Jiayuan Chen
  Cc: Maciej Wieczor-Retman, linux-mm, syzbot+997752115a851cb0cf36,
	Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, Vincenzo Frascino, Andrew Morton,
	Uladzislau Rezki, Danilo Krummrich, Kees Cook, kasan-dev,
	linux-kernel

On 2025-12-03 at 02:05:11 +0000, Jiayuan Chen wrote:
>December 3, 2025 at 04:48, "Maciej Wieczor-Retman" <maciej.wieczor-retman@intel.com mailto:maciej.wieczor-retman@intel.com?to=%22Maciej%20Wieczor-Retman%22%20%3Cmaciej.wieczor-retman%40intel.com%3E > wrote:
>> 
>> Hi, I'm working on [1]. As Andrew pointed out to me the patches are quite
>> similar. I was wondering if you mind if the reuse_tag was an actual tag value?
>> Instead of just bool toggling the usage of kasan_random_tag()?
>> 
>> I tested the problem I'm seeing, with your patch and the tags end up being reset.
>> That's because the vms[area] pointers that I want to unpoison don't have a tag
>> set, but generating a different random tag for each vms[] pointer crashes the
>> kernel down the line. So __kasan_unpoison_vmalloc() needs to be called on each
>> one but with the same tag.
>> 
>> Arguably I noticed my series also just resets the tags right now, but I'm
>> working to correct it at the moment. I can send a fixed version tomorrow. Just
>> wanted to ask if having __kasan_unpoison_vmalloc() set an actual predefined tag
>> is a problem from your point of view?
>> 
>> [1] https://lore.kernel.org/all/cover.1764685296.git.m.wieczorretman@pm.me/
>> 
>
>Hi Maciej,
>
>It seems we're focusing on different issues, but feel free to reuse or modify the 'reuse_tag'.
>It's intended to preserve the tag in one 'vma'.
>
>I'd also be happy to help reproduce and test your changes to ensure the issue I encountered
>isn't regressed once you send a patch based on mine. 
>
>Thanks.

After reading Andrey's comments on your patches and mine I tried applying all
the changes to test the flag approach. Now my patches don't modify any vrealloc
related code. I came up with something like this below from your patch. Just
tested it and it works fine on my end, does it look okay to you?

---
 include/linux/kasan.h | 1 +
 mm/kasan/hw_tags.c    | 3 ++-
 mm/kasan/shadow.c     | 4 +++-
 mm/vmalloc.c          | 6 ++++--
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 03e263fb9fa1..068f62d07122 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -28,6 +28,7 @@ typedef unsigned int __bitwise kasan_vmalloc_flags_t;
 #define KASAN_VMALLOC_INIT		((__force kasan_vmalloc_flags_t)0x01u)
 #define KASAN_VMALLOC_VM_ALLOC		((__force kasan_vmalloc_flags_t)0x02u)
 #define KASAN_VMALLOC_PROT_NORMAL	((__force kasan_vmalloc_flags_t)0x04u)
+#define KASAN_VMALLOC_KEEP_TAG		((__force kasan_vmalloc_flags_t)0x08u)
 
 #define KASAN_VMALLOC_PAGE_RANGE 0x1 /* Apply exsiting page range */
 #define KASAN_VMALLOC_TLB_FLUSH  0x2 /* TLB flush */
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index 1c373cc4b3fa..e6d7ee544c28 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -361,7 +361,8 @@ void *__kasan_unpoison_vmalloc(const void *start, unsigned long size,
 		return (void *)start;
 	}
 
-	tag = kasan_random_tag();
+	tag = (flags & KASAN_VMALLOC_KEEP_TAG) ? get_tag(start) :
+						 kasan_random_tag();
 	start = set_tag(start, tag);
 
 	/* Unpoison and initialize memory up to size. */
diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
index 5d2a876035d6..6dd61093d1d5 100644
--- a/mm/kasan/shadow.c
+++ b/mm/kasan/shadow.c
@@ -648,7 +648,9 @@ void *__kasan_unpoison_vmalloc(const void *start, unsigned long size,
 	    !(flags & KASAN_VMALLOC_PROT_NORMAL))
 		return (void *)start;
 
-	start = set_tag(start, kasan_random_tag());
+	if (!(flags & KASAN_VMALLOC_KEEP_TAG))
+		start = set_tag(start, kasan_random_tag());
+
 	kasan_unpoison(start, size, false);
 	return (void *)start;
 }
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ead22a610b18..c939dc04baa5 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -4180,8 +4180,10 @@ void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align
 	 * We already have the bytes available in the allocation; use them.
 	 */
 	if (size <= alloced_size) {
-		kasan_unpoison_vmalloc(p + old_size, size - old_size,
-				       KASAN_VMALLOC_PROT_NORMAL);
+		kasan_unpoison_vmalloc(p, size,
+				       KASAN_VMALLOC_PROT_NORMAL |
+				       KASAN_VMALLOC_VM_ALLOC |
+				       KASAN_VMALLOC_KEEP_TAG);
 		/*
 		 * No need to zero memory here, as unused memory will have
 		 * already been zeroed at initial allocation time or during

-- 
Kind regards
Maciej Wieczór-Retman



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-12-04 15:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20251128111516.244497-1-jiayuan.chen@linux.dev>
2025-12-02 20:48 ` [PATCH v1] mm/kasan: Fix incorrect unpoisoning in vrealloc for KASAN Maciej Wieczor-Retman
2025-12-03  2:05   ` Jiayuan Chen
2025-12-02 23:23 ` Kees Cook
2025-12-03  1:29   ` Jiayuan Chen
2025-12-03  7:30 Maciej Wieczór-Retman
2025-12-04 13:55 Maciej Wieczor-Retman
2025-12-04 14:38 ` Jiayuan Chen
2025-12-04 15:06   ` Andrey Konovalov
2025-12-04 15:35     ` Jiayuan Chen
2025-12-04 15:19   ` Maciej Wieczor-Retman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox