From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-186.mta1.migadu.com (out-186.mta1.migadu.com [95.215.58.186]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A23103358B0 for ; Thu, 4 Dec 2025 14:38:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.186 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764859100; cv=none; b=SbvhCljePSANYdZne8e2w5gxY2njz/D1hivV6WHXYodCxuWCiRu0LE7K+dBNV8OKLAvrvmrx/hVLJUV3J4ez6t404Zn0L0dREfmVWqDo/tE4vBqsg5+MJL9ThyY7Q/G+/rpRuUXH+KfdG+3Hh86o2C0wFbguP3cienS5JG1HC2A= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764859100; c=relaxed/simple; bh=bsbmHFN29qzQ/G+k6R7eKChTWGtV41p8gjFriD0jcuc=; h=MIME-Version:Date:Content-Type:From:Message-ID:Subject:To:Cc: In-Reply-To:References; b=aQ5YAmgopeiWk/QGrkufjF1kqDNbXmj4ZSKn5YND4j/P2JGeqmoFGuZoBLGNDnX1xHMgxEnE/N3fFzfdEh5I6TGWlQjLKCPxo/Equ9rVG83VfcfDk4hFObqsxx4yyEx+Jyfkpm+htZdqYoLiKSVrCXF8FfPhviDXDCb77ubOTo4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=JM4wZYbS; arc=none smtp.client-ip=95.215.58.186 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="JM4wZYbS" Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1764859095; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oyv8T3zljC5amsZPaB9Pcte8dRSYUhsdKHtrFYp3ltI=; b=JM4wZYbS6PN4WOkpcTsmyfuEYnqIDtFc8l3AXkH0Qf62PC/gWco3qiHf/uptpNFXlrMpew n/cyWOCkBUDhw8VloiuQNrXbBYec75TdhztnT+UEVVBy4lpnXKawdURO1rU1zqQ030nIG5 RmyNLeEWz7rVwOteYtoF+dwr8ZpVlgw= Date: Thu, 04 Dec 2025 14:38:12 +0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: "Jiayuan Chen" Message-ID: TLS-Required: No Subject: Re: [PATCH v1] mm/kasan: Fix incorrect unpoisoning in vrealloc for KASAN To: "Maciej Wieczor-Retman" Cc: "Maciej Wieczor-Retman" , linux-mm@kvack.org, syzbot+997752115a851cb0cf36@syzkaller.appspotmail.com, "Andrey Ryabinin" , "Alexander Potapenko" , "Andrey Konovalov" , "Dmitry Vyukov" , "Vincenzo Frascino" , "Andrew Morton" , "Uladzislau Rezki" , "Danilo Krummrich" , "Kees Cook" , kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org In-Reply-To: <5o7owlr4ap5fridqlkerrnuvwwlgldr35gvkcf6df4fufatrr6@yn5rmfn54i62> References: <5o7owlr4ap5fridqlkerrnuvwwlgldr35gvkcf6df4fufatrr6@yn5rmfn54i62> X-Migadu-Flow: FLOW_OUT December 4, 2025 at 21:55, "Maciej Wieczor-Retman" wrote: >=20 >=20On 2025-12-03 at 02:05:11 +0000, Jiayuan Chen wrote: >=20 >=20>=20 >=20> December 3, 2025 at 04:48, "Maciej Wieczor-Retman" wrote: > >=20 >=20> >=20 >=20> > Hi, I'm working on [1]. As Andrew pointed out to me the patches a= re quite > > > similar. I was wondering if you mind if the reuse_tag was an actua= l tag value? > > > Instead of just bool toggling the usage of kasan_random_tag()? > > >=20=20 >=20> > I tested the problem I'm seeing, with your patch and the tags en= d 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 ca= lled on each > > > one but with the same tag. > > >=20=20 >=20> > 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 to= morrow. Just > > > wanted to ask if having __kasan_unpoison_vmalloc() set an actual p= redefined tag > > > is a problem from your point of view? > > >=20=20 >=20> > [1] https://lore.kernel.org/all/cover.1764685296.git.m.wieczorre= tman@pm.me/ > > >=20 >=20> Hi Maciej, > >=20 >=20> 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'. > >=20 >=20> 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.=20 >=20>=20 >=20> Thanks. > >=20 >=20After reading Andrey's comments on your patches and mine I tried appl= ying 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? >=20 >=20--- > 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(-) >=20 >=20diff --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)0x04= u) > +#define KASAN_VMALLOC_KEEP_TAG ((__force kasan_vmalloc_flags_t)0x08u) >=20=20 >=20 #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, u= nsigned long size, > return (void *)start; > } >=20=20 >=20- tag =3D kasan_random_tag(); > + tag =3D (flags & KASAN_VMALLOC_KEEP_TAG) ? get_tag(start) : > + kasan_random_tag(); > start =3D set_tag(start, tag); >=20=20 >=20 /* 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, u= nsigned long size, > !(flags & KASAN_VMALLOC_PROT_NORMAL)) > return (void *)start; >=20=20 >=20- start =3D set_tag(start, kasan_random_tag()); > + if (!(flags & KASAN_VMALLOC_KEEP_TAG)) > + start =3D 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 <=3D 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 >=20 >=20--=20 >=20Kind regards > Maciej Wiecz=C3=B3r-Retman > I think I don't need KEEP_TAG flag anymore, following patch works well an= d all kasan tests run successfully with CONFIG_KASAN_SW_TAGS/CONFIG_KASAN_HW_TAGS/CONFIG_KASAN_GENERIC diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c index 1c373cc4b3fa..8b819a9b2a27 100644 --- a/mm/kasan/hw_tags.c +++ b/mm/kasan/hw_tags.c @@ -394,6 +394,11 @@ void __kasan_poison_vmalloc(const void *start, unsig= ned long size) * The physical pages backing the vmalloc() allocation are poisoned * through the usual page_alloc paths. */ + if (!is_vmalloc_or_module_addr(start)) + return; + + size =3D round_up(size, KASAN_GRANULE_SIZE); + kasan_poison(start, size, KASAN_VMALLOC_INVALID, false); } #endif diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c index 2cafca31b092..a5f683c3abde 100644 --- a/mm/kasan/kasan_test_c.c +++ b/mm/kasan/kasan_test_c.c @@ -1840,6 +1840,84 @@ static void vmalloc_helpers_tags(struct kunit *tes= t) vfree(ptr); } + +static void vrealloc_helpers(struct kunit *test, bool tags) +{ + char *ptr; + size_t size =3D PAGE_SIZE / 2 - KASAN_GRANULE_SIZE - 5; + + if (!kasan_vmalloc_enabled()) + kunit_skip(test, "Test requires kasan.vmalloc=3Don"); + + ptr =3D (char *)vmalloc(size); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr); + + OPTIMIZER_HIDE_VAR(ptr); + + size +=3D PAGE_SIZE / 2; + ptr =3D vrealloc(ptr, size, GFP_KERNEL); + /* Check that the returned pointer is tagged. */ + if (tags) { + KUNIT_EXPECT_GE(test, (u8)get_tag(ptr), (u8)KASAN_TAG_MIN); + KUNIT_EXPECT_LT(test, (u8)get_tag(ptr), (u8)KASAN_TAG_KERNEL); + } + /* Make sure in-bounds accesses are valid. */ + ptr[0] =3D 0; + ptr[size - 1] =3D 0; + + /* Make sure exported vmalloc helpers handle tagged pointers. */ + KUNIT_ASSERT_TRUE(test, is_vmalloc_addr(ptr)); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vmalloc_to_page(ptr)); + + size -=3D PAGE_SIZE / 2; + ptr =3D vrealloc(ptr, size, GFP_KERNEL); + + /* Check that the returned pointer is tagged. */ + KUNIT_EXPECT_GE(test, (u8)get_tag(ptr), (u8)KASAN_TAG_MIN); + KUNIT_EXPECT_LT(test, (u8)get_tag(ptr), (u8)KASAN_TAG_KERNEL); + + /* Make sure exported vmalloc helpers handle tagged pointers. */ + KUNIT_ASSERT_TRUE(test, is_vmalloc_addr(ptr)); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vmalloc_to_page(ptr)); + + + /* This access must cause a KASAN report. */ + KUNIT_EXPECT_KASAN_FAIL_READ(test, ((volatile char *)ptr)[size + 5]); + + +#if !IS_MODULE(CONFIG_KASAN_KUNIT_TEST) + { + int rv; + + /* Make sure vrealloc'ed memory permissions can be changed. */ + rv =3D set_memory_ro((unsigned long)ptr, 1); + KUNIT_ASSERT_GE(test, rv, 0); + rv =3D set_memory_rw((unsigned long)ptr, 1); + KUNIT_ASSERT_GE(test, rv, 0); + } +#endif + + vfree(ptr); +} + +static void vrealloc_helpers_tags(struct kunit *test) +{ + /* This test is intended for tag-based modes. */ + KASAN_TEST_NEEDS_CONFIG_OFF(test, CONFIG_KASAN_GENERIC); + + KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_VMALLOC); + vrealloc_helpers(test, true); +} + +static void vrealloc_helpers_generic(struct kunit *test) +{ + /* This test is intended for tag-based modes. */ + KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC); + + KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_VMALLOC); + vrealloc_helpers(test, false); +} + static void vmalloc_oob(struct kunit *test) { char *v_ptr, *p_ptr; @@ -2241,6 +2319,8 @@ static struct kunit_case kasan_kunit_test_cases[] = =3D { KUNIT_CASE_SLOW(kasan_atomics), KUNIT_CASE(vmalloc_helpers_tags), KUNIT_CASE(vmalloc_oob), + KUNIT_CASE(vrealloc_helpers_tags), + KUNIT_CASE(vrealloc_helpers_generic), KUNIT_CASE(vmap_tags), KUNIT_CASE(vm_map_ram_tags), KUNIT_CASE(match_all_not_assigned), diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 798b2ed21e46..9ba2e8a346d6 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -4128,6 +4128,7 @@ EXPORT_SYMBOL(vzalloc_node_noprof); void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned lo= ng align, gfp_t flags, int nid) { + asan_vmalloc_flags_t flags; struct vm_struct *vm =3D NULL; size_t alloced_size =3D 0; size_t old_size =3D 0; @@ -4158,25 +4159,26 @@ void *vrealloc_node_align_noprof(const void *p, s= ize_t size, unsigned long align goto need_realloc; } + flags =3D KASAN_VMALLOC_PROT_NORMAL | KASAN_VMALLOC_VM_ALLOC; /* * TODO: Shrink the vm_area, i.e. unmap and free unused pages. What * would be a good heuristic for when to shrink the vm_area? */ - if (size <=3D old_size) { + if (p && size <=3D old_size) { /* Zero out "freed" memory, potentially for future realloc. */ if (want_init_on_free() || want_init_on_alloc(flags)) memset((void *)p + size, 0, old_size - size); vm->requested_size =3D size; - kasan_poison_vmalloc(p + size, old_size - size); + kasan_poison_vmalloc(p, alloced_size); + p =3D kasan_unpoison_vmalloc(p, size, flags); return (void *)p; } /* * We already have the bytes available in the allocation; use them. */ - if (size <=3D alloced_size) { - kasan_unpoison_vmalloc(p + old_size, size - old_size, - KASAN_VMALLOC_PROT_NORMAL); + if (p && size <=3D alloced_size) { + p =3D kasan_unpoison_vmalloc(p, size, flags); /* * No need to zero memory here, as unused memory will have * already been zeroed at initial allocation time or during