From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 47D5C33F5BE for ; Tue, 19 May 2026 08:53:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779180840; cv=none; b=Kyay0nj++HLDvQ64AkSBg96mGErh0TamSJ7hhEmkJccxQqyEA3+bARBkT82RmSZVRZM/SQQ8RmerHia22Jt3J6QszUXXTI2r57EfbMvRN5eqkvBTLAWaQx/C+tvNvMdu1PqirTy+3pVTLJLN+XYbeuyW6C6TWzxuXFM8eAjsyAk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779180840; c=relaxed/simple; bh=9m3g68iZohH7nlU/DGiQB3qTcGS3h2EgujSLKlci3tc=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=X8za6/R9m2MSFaUUODBaM4lip4/i8/ByGBgleDvoFcTss6gPuTtVOo7mdDJ3sUzLjU5Kb5lERkgwJ8x9VNiyBnf04AN1uPTpUrg5HoSL2cXkoLMPvnpwarhC7MniXoxnEK5DY0Rx1DbFrY31b0nMdHXlzr8U+ETIp4M3fzcV0No= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=fSzoZtlt; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="fSzoZtlt" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 773C034FC; Tue, 19 May 2026 01:53:53 -0700 (PDT) Received: from [10.57.90.238] (unknown [10.57.90.238]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 16F373F85F; Tue, 19 May 2026 01:53:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1779180838; bh=9m3g68iZohH7nlU/DGiQB3qTcGS3h2EgujSLKlci3tc=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=fSzoZtltMXzCsNvgy6KTo/Lt4o+ML+Au+mkLPARFmb4EqGGuLhfsiohF66+1n0k8D IAMrNNehPpApn+MLNU+l8BTnDKR9kk0dcJyLCJEXl6Wv6V4JGI4KGX6YoAAUpECu1Q 8u1M8JmgOZo+o7nmeV/cYxGYG4oUrj6vcPtWaYoE= Message-ID: <2609d691-5e70-4baa-8e58-99f5ea708c6a@arm.com> Date: Tue, 19 May 2026 09:53:53 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/3] kasan: avoid re-poisoning tag-based kmalloc redzones Content-Language: en-GB To: Dev Jain , akpm@linux-foundation.org, vbabka@kernel.org, harry@kernel.org, ryabinin.a.a@gmail.com Cc: surenb@google.com, mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org, ziy@nvidia.com, hao.li@linux.dev, cl@gentwo.org, rientjes@google.com, roman.gushchin@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, glider@google.com, andreyknvl@gmail.com, dvyukov@google.com, vincenzo.frascino@arm.com, kasan-dev@googlegroups.com, anshuman.khandual@arm.com, catalin.marinas@arm.com References: <20260513105734.3380544-1-dev.jain@arm.com> <20260513105734.3380544-3-dev.jain@arm.com> <14da7ffe-1332-45d0-86d6-e9d9ae0a4fe0@arm.com> From: Ryan Roberts In-Reply-To: <14da7ffe-1332-45d0-86d6-e9d9ae0a4fe0@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 19/05/2026 09:15, Dev Jain wrote: > > > On 13/05/26 4:27 pm, Dev Jain wrote: >> When we allocate object from slab, kasan will unpoison the entire object. >> In case of allocation from kmalloc caches, the actual allocation size >> request can be less than the size of the kmalloc cache. kasan poisons >> the bytes following allocation size up till object size to catch OOB. >> >> We can do this operation in one shot: while unpoisoning the object upon >> allocation, only unpoison up till allocation size bytes, so that the >> bytes following that up till object size remain poisoned. >> >> Currently when we free an object into the slab, we use KASAN_SLAB_FREE for >> poisoning, and use KASAN_SLAB_REDZONE for poisoning the tail end. For >> tag-based kasan, these two are equal, as opposed to generic kasan. So we >> make this optimization only for tag-based kasan. >> >> Signed-off-by: Dev Jain >> --- > > Sashiko: https://sashiko.dev/#/patchset/20260513105734.3380544-1-dev.jain@arm.com > > So I'll explain what Sashiko is saying with examples. If I am mis-interpreting > then please correct me! > > Problem 1: > > p = kzalloc(96, GFP_KERNEL); // kmalloc-128 > p = krealloc(p, 120, GFP_KERNEL | __GFP_ZERO); > > Before my patch, kzalloc zeroed bytes 0..127, krealloc exposes bytes 96..119 > which are already zero from kzalloc. > > After my patch, kzalloc zeroes only 0..95, krealloc may think old > size is 128, so it does not zero 96..119, 96..119 may contain stale data. > > Why is this a problem? Intuitively, I would say it's a problem if you're leaking when you previously were not. > Surely caller of krealloc shouldn't be > relying on a previous zeroing. krealloc documentation explicitly says that > GFP_ZERO is ignored. What would happen if you did: p = kzalloc(96, GFP_KERNEL); // kmalloc-128 p = krealloc(p, 129, GFP_KERNEL | __GFP_ZERO); Presumably the krealloc() would have to move the whole allocation to the next biggest bin? Would p[128] be zeroed? If so, then I'd say your change is problematic. > > > Problem 2 for SW tags: > > p = kzalloc(97, GFP_KERNEL); // kmalloc-128, granule = 16 > > Before patch: > > KASAN unpoisons 0..111 > memset zeroes 0..127 > p[100] will not be caught by KASAN, but reads zero > > After patch: > > KASAN unpoisons 0..111 > memset zeroes only 0..96 > p[100] will not be caught by KASAN and reads stale data > > Again why is this a problem? Because you have defeated KASAN and are leaking data? I think you'd want to zero at least to the end of the KASAN granule? Thanks, Ryan > >> include/linux/kasan.h | 17 +++++++++---- >> mm/kasan/common.c | 55 +++++++++++++++++++++++++++++++++---------- >> mm/slub.c | 11 +++++---- >> 3 files changed, 61 insertions(+), 22 deletions(-) >> >> diff --git a/include/linux/kasan.h b/include/linux/kasan.h >> index bf233bde68c7..fd7c1f5f9fd6 100644 >> --- a/include/linux/kasan.h >> +++ b/include/linux/kasan.h >> @@ -102,6 +102,12 @@ static inline bool kasan_has_integrated_init(void) >> return kasan_hw_tags_enabled(); >> } >> >> +static inline bool kasan_has_tag_based_kmalloc_redzones(void) >> +{ >> + return kasan_enabled() && >> + (IS_ENABLED(CONFIG_KASAN_SW_TAGS) || kasan_hw_tags_enabled()); >> +} >> + >> #ifdef CONFIG_KASAN >> void __kasan_unpoison_range(const void *addr, size_t size); >> static __always_inline void kasan_unpoison_range(const void *addr, size_t size) >> @@ -244,13 +250,14 @@ static __always_inline void kasan_kfree_large(void *ptr) >> __kasan_kfree_large(ptr, _RET_IP_); >> } >> >> -void * __must_check __kasan_slab_alloc(struct kmem_cache *s, >> - void *object, gfp_t flags, bool init); >> +void * __must_check __kasan_slab_alloc(struct kmem_cache *s, void *object, >> + size_t size, gfp_t flags, bool init); >> static __always_inline void * __must_check kasan_slab_alloc( >> - struct kmem_cache *s, void *object, gfp_t flags, bool init) >> + struct kmem_cache *s, void *object, size_t size, >> + gfp_t flags, bool init) >> { >> if (kasan_enabled()) >> - return __kasan_slab_alloc(s, object, flags, init); >> + return __kasan_slab_alloc(s, object, size, flags, init); >> return object; >> } >> >> @@ -437,7 +444,7 @@ static inline bool kasan_slab_free(struct kmem_cache *s, void *object, >> } >> static inline void kasan_kfree_large(void *ptr) {} >> static inline void *kasan_slab_alloc(struct kmem_cache *s, void *object, >> - gfp_t flags, bool init) >> + size_t size, gfp_t flags, bool init) >> { >> return object; >> } >> diff --git a/mm/kasan/common.c b/mm/kasan/common.c >> index b7d05c2a6d93..9a4db9c21aaf 100644 >> --- a/mm/kasan/common.c >> +++ b/mm/kasan/common.c >> @@ -326,14 +326,25 @@ void __kasan_kfree_large(void *ptr, unsigned long ip) >> /* The object will be poisoned by kasan_poison_pages(). */ >> } >> >> +static inline size_t slab_unpoison_size(struct kmem_cache *cache, size_t size) >> +{ >> + if (kasan_has_tag_based_kmalloc_redzones() && is_kmalloc_cache(cache)) >> + return min_t(size_t, size, cache->object_size); >> + >> + return cache->object_size; >> +} >> + >> static inline void unpoison_slab_object(struct kmem_cache *cache, void *object, >> - gfp_t flags, bool init) >> + size_t size, gfp_t flags, bool init) >> { >> /* >> - * Unpoison the whole object. For kmalloc() allocations, >> - * poison_kmalloc_redzone() will do precise poisoning. >> + * For tag-based modes, kmalloc redzones all use the same invalid tag. >> + * Keep the tail poisoned and only unpoison the requested allocation >> + * size. Generic KASAN keeps distinct shadow values for free objects and >> + * redzones, so it still unpoisons the whole object and later poisons >> + * the precise redzone. >> */ >> - kasan_unpoison(object, cache->object_size, init); >> + kasan_unpoison(object, slab_unpoison_size(cache, size), init); >> >> /* Save alloc info (if possible) for non-kmalloc() allocations. */ >> if (kasan_stack_collection_enabled() && !is_kmalloc_cache(cache)) >> @@ -341,7 +352,8 @@ static inline void unpoison_slab_object(struct kmem_cache *cache, void *object, >> } >> >> void * __must_check __kasan_slab_alloc(struct kmem_cache *cache, >> - void *object, gfp_t flags, bool init) >> + void *object, size_t size, >> + gfp_t flags, bool init) >> { >> u8 tag; >> void *tagged_object; >> @@ -363,11 +375,18 @@ void * __must_check __kasan_slab_alloc(struct kmem_cache *cache, >> tagged_object = set_tag(object, tag); >> >> /* Unpoison the object and save alloc info for non-kmalloc() allocations. */ >> - unpoison_slab_object(cache, tagged_object, flags, init); >> + unpoison_slab_object(cache, tagged_object, size, flags, init); >> >> return tagged_object; >> } >> >> +static inline void save_kmalloc_alloc_info(struct kmem_cache *cache, >> + void *object, gfp_t flags) >> +{ >> + if (kasan_stack_collection_enabled() && is_kmalloc_cache(cache)) >> + kasan_save_alloc_info(cache, object, flags); >> +} >> + >> static inline void poison_kmalloc_redzone(struct kmem_cache *cache, >> const void *object, size_t size, gfp_t flags) >> { >> @@ -394,8 +413,7 @@ static inline void poison_kmalloc_redzone(struct kmem_cache *cache, >> * Save alloc info (if possible) for kmalloc() allocations. >> * This also rewrites the alloc info when called from kasan_krealloc(). >> */ >> - if (kasan_stack_collection_enabled() && is_kmalloc_cache(cache)) >> - kasan_save_alloc_info(cache, (void *)object, flags); >> + save_kmalloc_alloc_info(cache, (void *)object, flags); >> >> } >> >> @@ -411,8 +429,14 @@ void * __must_check __kasan_kmalloc(struct kmem_cache *cache, const void *object >> if (is_kfence_address(object)) >> return (void *)object; >> >> - /* The object has already been unpoisoned by kasan_slab_alloc(). */ >> - poison_kmalloc_redzone(cache, object, size, flags); >> + /* >> + * For tag-based modes, the object has already been precisely >> + * unpoisoned by kasan_slab_alloc(). The tail remains poisoned. >> + */ >> + if (kasan_has_tag_based_kmalloc_redzones()) >> + save_kmalloc_alloc_info(cache, (void *)object, flags); >> + else >> + poison_kmalloc_redzone(cache, object, size, flags); >> >> /* Keep the tag that was set by kasan_slab_alloc(). */ >> return (void *)object; >> @@ -561,11 +585,16 @@ void __kasan_mempool_unpoison_object(void *ptr, size_t size, unsigned long ip) >> return; >> >> /* Unpoison the object and save alloc info for non-kmalloc() allocations. */ >> - unpoison_slab_object(slab->slab_cache, ptr, flags, false); >> + unpoison_slab_object(slab->slab_cache, ptr, size, flags, false); >> >> /* Poison the redzone and save alloc info for kmalloc() allocations. */ >> - if (is_kmalloc_cache(slab->slab_cache)) >> - poison_kmalloc_redzone(slab->slab_cache, ptr, size, flags); >> + if (is_kmalloc_cache(slab->slab_cache)) { >> + if (kasan_has_tag_based_kmalloc_redzones()) >> + save_kmalloc_alloc_info(slab->slab_cache, ptr, flags); >> + else >> + poison_kmalloc_redzone(slab->slab_cache, ptr, size, >> + flags); >> + } >> } >> >> bool __kasan_check_byte(const void *address, unsigned long ip) >> diff --git a/mm/slub.c b/mm/slub.c >> index da3520769d1f..15144b2e078c 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -4550,8 +4550,9 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, >> * replacement of current poisoning under certain debug option, and >> * won't break other sanity checks. >> */ >> - if (kmem_cache_debug_flags(s, SLAB_STORE_USER | SLAB_RED_ZONE) && >> - (s->flags & SLAB_KMALLOC)) >> + if ((s->flags & SLAB_KMALLOC) && >> + (kmem_cache_debug_flags(s, SLAB_STORE_USER | SLAB_RED_ZONE) || >> + kasan_has_tag_based_kmalloc_redzones())) >> zero_size = orig_size; >> >> /* >> @@ -4573,7 +4574,8 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru, >> * As p[i] might get tagged, memset and kmemleak hook come after KASAN. >> */ >> for (i = 0; i < size; i++) { >> - p[i] = kasan_slab_alloc(s, p[i], init_flags, kasan_init); >> + p[i] = kasan_slab_alloc(s, p[i], orig_size, init_flags, >> + kasan_init); >> if (p[i] && init && (!kasan_init || >> !kasan_has_integrated_init())) >> memset(p[i], 0, zero_size); >> @@ -7615,7 +7617,8 @@ static void early_kmem_cache_node_alloc(int node) >> #ifdef CONFIG_SLUB_DEBUG >> init_object(kmem_cache_node, n, SLUB_RED_ACTIVE); >> #endif >> - n = kasan_slab_alloc(kmem_cache_node, n, GFP_KERNEL, false); >> + n = kasan_slab_alloc(kmem_cache_node, n, kmem_cache_node->object_size, >> + GFP_KERNEL, false); >> slab->freelist = get_freepointer(kmem_cache_node, n); >> slab->inuse = 1; >> kmem_cache_node->per_node[node].node = n; >