Message-ID: <5dce1baa4672646c54beaa94355e32fab856fa2b@linux.dev>
Date: Thu, 04 Dec 2025 15:35:23 +0000
From: "Jiayuan Chen" <jiayuan.chen@...ux.dev>
To: "Andrey Konovalov" <andreyknvl@...il.com>
Cc: "Maciej Wieczor-Retman" <m.wieczorretman@...me>, "Maciej Wieczor-Retman"
 <maciej.wieczor-retman@...el.com>, linux-mm@...ck.org,
 syzbot+997752115a851cb0cf36@...kaller.appspotmail.com, "Andrey Ryabinin"
 <ryabinin.a.a@...il.com>, "Alexander Potapenko" <glider@...gle.com>,
 "Dmitry Vyukov" <dvyukov@...gle.com>, "Vincenzo Frascino"
 <vincenzo.frascino@....com>, "Andrew Morton" <akpm@...ux-foundation.org>,
 "Uladzislau Rezki" <urezki@...il.com>, "Danilo Krummrich"
 <dakr@...nel.org>, "Kees Cook" <kees@...nel.org>,
 kasan-dev@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] mm/kasan: Fix incorrect unpoisoning in vrealloc for
 KASAN

December 4, 2025 at 23:06, "Andrey Konovalov" <andreyknvl@...il.com mailto:andreyknvl@...il.com?to=%22Andrey%20Konovalov%22%20%3Candreyknvl%40gmail.com%3E > wrote:


> 
> On Thu, Dec 4, 2025 at 3:38 PM Jiayuan Chen <jiayuan.chen@...ux.dev> wrote:
> 
> > 
> > I think I don't need KEEP_TAG flag anymore, following patch works well and all kasan tests run successfully
> >  with CONFIG_KASAN_SW_TAGS/CONFIG_KASAN_HW_TAGS/CONFIG_KASAN_GENERIC
> > 
> Thanks for working on improving the vrealloc annotations!
> 
> But I think we need to first fix the vrealloc issue you discovered in
> a separate patch (so that it can be backported), and then we can apply
> your other vrealloc changes on top later.
> 
> So please implement a version of your fix with KEEP_TAG -- this would
> also allow Maciej to build on top.

Thanks, i will just use KEEP_TAG and make the patch as simple as possible.

CC Maciej

Thanks.

> > 
> > 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, unsigned 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 = round_up(size, KASAN_GRANULE_SIZE);
> >  + kasan_poison(start, size, KASAN_VMALLOC_INVALID, false);
> > 
> This does not look good - we will end up poisoning the same memory
> twice, once here and once it's freed to page_alloc.
> 
> Is this change required?
> 
> > 
> > }
> > 
> >  #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 *test)
> >  vfree(ptr);
> >  }
> > 
> >  +
> >  +static void vrealloc_helpers(struct kunit *test, bool tags)
> >  +{
> >  + char *ptr;
> >  + size_t size = PAGE_SIZE / 2 - KASAN_GRANULE_SIZE - 5;
> >  +
> >  + if (!kasan_vmalloc_enabled())
> >  + kunit_skip(test, "Test requires kasan.vmalloc=on");
> >  +
> >  + ptr = (char *)vmalloc(size);
> >  + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
> >  +
> >  + OPTIMIZER_HIDE_VAR(ptr);
> >  +
> >  + size += PAGE_SIZE / 2;
> >  + ptr = 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] = 0;
> >  + ptr[size - 1] = 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 -= PAGE_SIZE / 2;
> >  + ptr = 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 = set_memory_ro((unsigned long)ptr, 1);
> >  + KUNIT_ASSERT_GE(test, rv, 0);
> >  + rv = 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[] = {
> >  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 long align,
> >  gfp_t flags, int nid)
> >  {
> >  + asan_vmalloc_flags_t flags;
> >  struct vm_struct *vm = NULL;
> >  size_t alloced_size = 0;
> >  size_t old_size = 0;
> >  @@ -4158,25 +4159,26 @@ void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align
> >  goto need_realloc;
> >  }
> > 
> >  + flags = 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 <= old_size) {
> >  + if (p && size <= 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 = size;
> >  - kasan_poison_vmalloc(p + size, old_size - size);
> >  + kasan_poison_vmalloc(p, alloced_size);
> >  + p = kasan_unpoison_vmalloc(p, size, flags);
> >  return (void *)p;
> >  }
> > 
> >  /*
> >  * 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);
> >  + if (p && size <= alloced_size) {
> >  + p = 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
> >
>

Powered by blists - more mailing lists