From patchwork Tue Dec 26 22:51:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: andrey.konovalov@linux.dev X-Patchwork-Id: 13505067 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 19F55C46CD3 for ; Tue, 26 Dec 2023 22:51:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 72E9B6B0074; Tue, 26 Dec 2023 17:51:35 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6DF6A6B0075; Tue, 26 Dec 2023 17:51:35 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5A76C6B007B; Tue, 26 Dec 2023 17:51:35 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 478B06B0074 for ; Tue, 26 Dec 2023 17:51:35 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 26D58A0696 for ; Tue, 26 Dec 2023 22:51:35 +0000 (UTC) X-FDA: 81610467750.27.94748E2 Received: from out-175.mta1.migadu.com (out-175.mta1.migadu.com [95.215.58.175]) by imf24.hostedemail.com (Postfix) with ESMTP id 363DA180005 for ; Tue, 26 Dec 2023 22:51:32 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=KsKHNPqD; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf24.hostedemail.com: domain of andrey.konovalov@linux.dev designates 95.215.58.175 as permitted sender) smtp.mailfrom=andrey.konovalov@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1703631093; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-transfer-encoding:content-transfer-encoding: in-reply-to:references:dkim-signature; bh=KAcZVWUdwxMgEguWO/ipgfFw4d2BOub4xvB7dxmq+gU=; b=47aE3jr15psbvb0o5C2eA0zYpJsAh6qFm8gPsIZFveZqeAuEJMtrz0NSnec613cYVwevSH m7+zO4uO8JaXXcE4FXHZSfVulCBzfmLBTxkUGGxcCrMeQd9OlQrRkaPcl4RGvH+Nnx2lTp vx657ao0c+/2miGqBQequTssu+O558g= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=KsKHNPqD; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf24.hostedemail.com: domain of andrey.konovalov@linux.dev designates 95.215.58.175 as permitted sender) smtp.mailfrom=andrey.konovalov@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1703631093; a=rsa-sha256; cv=none; b=H/1YSt2q8vUIvzXPISC7sMMqCORaax/wWiyAp8wMphKI5PRUgsDU+ViQ5Ul5yUGUbbmotZ 0dxOyFYqSuv2wRd95TM1m77xVUKeFoW6Yt0QPYyUOQFdeNDoPt85E4T5PcwnA0mCU2uNSB +U7OBoUqXvvMCOK09q5Ok+J+BdF48wA= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1703631091; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=KAcZVWUdwxMgEguWO/ipgfFw4d2BOub4xvB7dxmq+gU=; b=KsKHNPqDeqoEeAJoRpQZG4yWXxJWmQq2+AcxQm+Busl0tPEWJWU4/fqxWpQGGxTvf7k1vq Szj8Tq6B+qN48kKlRbItk5DzB2dvWd++Mi863otmoP2s1LGT3V5Ep+WP0vnfQSNHeDW92e DRyIKyGgCs+z/VAHI+ViGjP8HSLLhhw= From: andrey.konovalov@linux.dev To: Marco Elver , Andrew Morton Cc: Andrey Konovalov , Alexander Potapenko , Dmitry Vyukov , Andrey Ryabinin , kasan-dev@googlegroups.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrey Konovalov Subject: [PATCH mm] kasan: stop leaking stack trace handles Date: Tue, 26 Dec 2023 23:51:21 +0100 Message-Id: <20231226225121.235865-1-andrey.konovalov@linux.dev> MIME-Version: 1.0 X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 363DA180005 X-Stat-Signature: 8wf3oa3aaa83fsm9q6trm1p7ci5e7o19 X-HE-Tag: 1703631092-723254 X-HE-Meta: U2FsdGVkX18urQ9Lg+D8J6nCH20pZsyP96sNCf21aCwdNfhxfj6GId4+hWL8KtC1wgCK8hzMbkwG3wo00E5KA69ipgjsEls8UTx2TmYOG22sBlByOVjNigYY0xUahFqfQXro0dQNBM7wJPpySCvSYAYOYXPNW+/q7LSSXTpbHBjgSdc/DN8kUOUmU2XDasKvQ9CSXvs8J9kbR6aE/7tihjf4PIDighqSwEqNypmvbG0YmNOstaw8n3plVDwQ3SHzz8Mz2YmET8Kw9KeIyvjINkEfuQaZa3ZSQR/ZP5XCXTKfhbgsrHPKHRI1Wud0ei0bSc2JtkZQc+3UuysqJYTz4z5fqLjzgQ/8Sl++aUjLSXTKrpdTDur6uxrgJW97vMceH3yT+gEnDw3l3XEQFDhrMcrBbz0fUMdD0HRdsJGIy2yFDJMhkA+4Y+PmlSuIJssalObBn9vREHmG43olKcs18ni8OHiilDRX7PQUA74b2LVi3OaiCmxIWgRTh8rqB1CtESRH2FE3FWSJdt1qeYDkHBR1xcZDNhcyOlRc7fAQNQBnkzqPw/bQL4qKFuoJ72uhK1RE1vrmjLgnu2gwxawV0p2IdP+JjIxP8YyKQPinW1kSLxu0x2qH4Lyf5hqXzGR++DdlX+kpUYlzfnrYddsNPNNvqqUWNqx4FnIk6pHwC3ZhREAjJt59O1EEjbVFIpnu89iaYLVNv3IDYfRbVRMPFheNC9q9zRW51FTmlyPAAgtwjdeXiJLi6pnIuniCF+Sa0lHroPsmur16RZTiiBh6D9ceSDAjH5gVMtzsf7Kp+11GTrCdhovalIS9FWucSILfUy5alzaiHwO+zcwVRCte+ElhEg4bTtij9eMRpb6SJ6nYGN8ASwvtpYnSsLHYQLKu8d0LMkr66LXN5fdJ/I4tdT1uK4UtpqpfN2XG+JM0OZyJr+AI13v9vpLVZd54lx8pgSUs+xFHa3YHw+op3O1 SgOfw66K lilDhuO6i2w/hVYnW263IUU18EU+aXRe0DNj6HYtuSs0Nwz1M3O20hKSRaKcaFiYqzfUUN3492kV0YepG4/EYxoUYd2v4GzE9wsd9ffmvK1fXiw4ejkReuhj/7Fj+cBAARSMBxkkEXCBkYVErenTDN6VZSY4Uz8fEEyzlw4+3UH8mcYRa9ZDctAAtu2EfcNK1ij9/gvasuNOgIts= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: From: Andrey Konovalov Commit 773688a6cb24 ("kasan: use stack_depot_put for Generic mode") added support for stack trace eviction for Generic KASAN. However, that commit didn't evict stack traces when the object is not put into quarantine. As a result, some stack traces are never evicted from the stack depot. In addition, with the "kasan: save mempool stack traces" series, the free stack traces for mempool objects are also not properly evicted from the stack depot. Fix both issues by: 1. Evicting all stack traces when an object if freed if it was not put into quarantine; 2. Always evicting an existing free stack trace when a new one is saved. Also do a few related clean-ups: - Do not zero out free track when initializing/invalidating free meta: set a value in shadow memory instead; - Rename KASAN_SLAB_FREETRACK to KASAN_SLAB_FREE_META; - Drop the kasan_init_cache_meta function as it's not used by KASAN; - Add comments for the kasan_alloc_meta and kasan_free_meta structs. Fixes: 773688a6cb24 ("kasan: use stack_depot_put for Generic mode") Signed-off-by: Andrey Konovalov --- Andrew, please put this as a separate patch on top of all KASAN patches in mm. --- mm/kasan/common.c | 27 +++++++++++++++--- mm/kasan/generic.c | 60 +++++++++++++++++++++++++++++++++------ mm/kasan/kasan.h | 25 ++++++++++++---- mm/kasan/quarantine.c | 20 +------------ mm/kasan/report_generic.c | 6 ++-- 5 files changed, 97 insertions(+), 41 deletions(-) diff --git a/mm/kasan/common.c b/mm/kasan/common.c index a486e9b1ac68..223af53d4338 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -255,14 +255,33 @@ static inline bool poison_slab_object(struct kmem_cache *cache, void *object, bool __kasan_slab_free(struct kmem_cache *cache, void *object, unsigned long ip, bool init) { - bool buggy_object; - if (is_kfence_address(object)) return false; - buggy_object = poison_slab_object(cache, object, ip, init); + /* + * If the object is buggy, do not let slab put the object onto the + * freelist. The object will thus never be allocated again and its + * metadata will never get released. + */ + if (poison_slab_object(cache, object, ip, init)) + return true; + + /* + * If the object is put into quarantine, do not let slab put the object + * onto the freelist for now. The object's metadata is kept until the + * object gets evicted from quarantine. + */ + if (kasan_quarantine_put(cache, object)) + return true; + + /* + * If the object is not put into quarantine, it will likely be quickly + * reallocated. Thus, release its metadata now. + */ + kasan_release_object_meta(cache, object); - return buggy_object ? true : kasan_quarantine_put(cache, object); + /* Let slab put the object onto the freelist. */ + return false; } static inline bool check_page_allocation(void *ptr, unsigned long ip) diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c index 0e77c43c559e..fc22ea1af775 100644 --- a/mm/kasan/generic.c +++ b/mm/kasan/generic.c @@ -480,10 +480,10 @@ struct kasan_free_meta *kasan_get_free_meta(struct kmem_cache *cache, void kasan_init_object_meta(struct kmem_cache *cache, const void *object) { struct kasan_alloc_meta *alloc_meta; - struct kasan_free_meta *free_meta; alloc_meta = kasan_get_alloc_meta(cache, object); if (alloc_meta) { + /* Zero out alloc meta to mark it as invalid. */ __memset(alloc_meta, 0, sizeof(*alloc_meta)); /* @@ -495,9 +495,50 @@ void kasan_init_object_meta(struct kmem_cache *cache, const void *object) raw_spin_lock_init(&alloc_meta->aux_lock); kasan_enable_current(); } + + /* + * Explicitly marking free meta as invalid is not required: the shadow + * value for the first 8 bytes of a newly allocated object is not + * KASAN_SLAB_FREE_META. + */ +} + +void release_alloc_meta(struct kasan_alloc_meta *meta) +{ + /* Evict the stack traces from stack depot. */ + stack_depot_put(meta->alloc_track.stack); + stack_depot_put(meta->aux_stack[0]); + stack_depot_put(meta->aux_stack[1]); + + /* Zero out alloc meta to mark it as invalid. */ + __memset(meta, 0, sizeof(*meta)); +} + +void release_free_meta(const void *object, struct kasan_free_meta *meta) +{ + /* Check if free meta is valid. */ + if (*(u8 *)kasan_mem_to_shadow(object) != KASAN_SLAB_FREE_META) + return; + + /* Evict the stack trace from the stack depot. */ + stack_depot_put(meta->free_track.stack); + + /* Mark free meta as invalid. */ + *(u8 *)kasan_mem_to_shadow(object) = KASAN_SLAB_FREE; +} + +void kasan_release_object_meta(struct kmem_cache *cache, const void *object) +{ + struct kasan_alloc_meta *alloc_meta; + struct kasan_free_meta *free_meta; + + alloc_meta = kasan_get_alloc_meta(cache, object); + if (alloc_meta) + release_alloc_meta(alloc_meta); + free_meta = kasan_get_free_meta(cache, object); if (free_meta) - __memset(free_meta, 0, sizeof(*free_meta)); + release_free_meta(object, free_meta); } size_t kasan_metadata_size(struct kmem_cache *cache, bool in_object) @@ -573,11 +614,8 @@ void kasan_save_alloc_info(struct kmem_cache *cache, void *object, gfp_t flags) if (!alloc_meta) return; - /* Evict previous stack traces (might exist for krealloc). */ - stack_depot_put(alloc_meta->alloc_track.stack); - stack_depot_put(alloc_meta->aux_stack[0]); - stack_depot_put(alloc_meta->aux_stack[1]); - __memset(alloc_meta, 0, sizeof(*alloc_meta)); + /* Evict previous stack traces (might exist for krealloc or mempool). */ + release_alloc_meta(alloc_meta); kasan_save_track(&alloc_meta->alloc_track, flags); } @@ -590,7 +628,11 @@ void kasan_save_free_info(struct kmem_cache *cache, void *object) if (!free_meta) return; + /* Evict previous stack trace (might exist for mempool). */ + release_free_meta(object, free_meta); + kasan_save_track(&free_meta->free_track, 0); - /* The object was freed and has free track set. */ - *(u8 *)kasan_mem_to_shadow(object) = KASAN_SLAB_FREETRACK; + + /* Mark free meta as valid. */ + *(u8 *)kasan_mem_to_shadow(object) = KASAN_SLAB_FREE_META; } diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h index 814e89523c64..645ae04539c9 100644 --- a/mm/kasan/kasan.h +++ b/mm/kasan/kasan.h @@ -156,7 +156,7 @@ static inline bool kasan_requires_meta(void) #ifdef CONFIG_KASAN_GENERIC -#define KASAN_SLAB_FREETRACK 0xFA /* freed slab object with free track */ +#define KASAN_SLAB_FREE_META 0xFA /* freed slab object with free meta */ #define KASAN_GLOBAL_REDZONE 0xF9 /* redzone for global variable */ /* Stack redzone shadow values. Compiler ABI, do not change. */ @@ -253,6 +253,15 @@ struct kasan_global { #ifdef CONFIG_KASAN_GENERIC +/* + * Alloc meta contains the allocation-related information about a slab object. + * Alloc meta is saved when an object is allocated and is kept until either the + * object returns to the slab freelist (leaves quarantine for quarantined + * objects or gets freed for the non-quarantined ones) or reallocated via + * krealloc or through a mempool. + * Alloc meta is stored inside of the object's redzone. + * Alloc meta is considered valid whenever it contains non-zero data. + */ struct kasan_alloc_meta { struct kasan_track alloc_track; /* Free track is stored in kasan_free_meta. */ @@ -278,8 +287,12 @@ struct qlist_node { #define KASAN_NO_FREE_META INT_MAX /* - * Free meta is only used by Generic mode while the object is in quarantine. - * After that, slab allocator stores the freelist pointer in the object. + * Free meta contains the freeing-related information about a slab object. + * Free meta is only kept for quarantined objects and for mempool objects until + * the object gets allocated again. + * Free meta is stored within the object's memory. + * Free meta is considered valid whenever the value of the shadow byte that + * corresponds to the first 8 bytes of the object is KASAN_SLAB_FREE_META. */ struct kasan_free_meta { struct qlist_node quarantine_link; @@ -380,15 +393,15 @@ void kasan_report_invalid_free(void *object, unsigned long ip, enum kasan_report struct slab *kasan_addr_to_slab(const void *addr); #ifdef CONFIG_KASAN_GENERIC -void kasan_init_cache_meta(struct kmem_cache *cache, unsigned int *size); -void kasan_init_object_meta(struct kmem_cache *cache, const void *object); struct kasan_alloc_meta *kasan_get_alloc_meta(struct kmem_cache *cache, const void *object); struct kasan_free_meta *kasan_get_free_meta(struct kmem_cache *cache, const void *object); +void kasan_init_object_meta(struct kmem_cache *cache, const void *object); +void kasan_release_object_meta(struct kmem_cache *cache, const void *object); #else -static inline void kasan_init_cache_meta(struct kmem_cache *cache, unsigned int *size) { } static inline void kasan_init_object_meta(struct kmem_cache *cache, const void *object) { } +static inline void kasan_release_object_meta(struct kmem_cache *cache, const void *object) { } #endif depot_stack_handle_t kasan_save_stack(gfp_t flags, depot_flags_t depot_flags); diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c index 782e045da911..8afa77bc5d3b 100644 --- a/mm/kasan/quarantine.c +++ b/mm/kasan/quarantine.c @@ -143,22 +143,10 @@ static void *qlink_to_object(struct qlist_node *qlink, struct kmem_cache *cache) static void qlink_free(struct qlist_node *qlink, struct kmem_cache *cache) { void *object = qlink_to_object(qlink, cache); - struct kasan_alloc_meta *alloc_meta = kasan_get_alloc_meta(cache, object); struct kasan_free_meta *free_meta = kasan_get_free_meta(cache, object); unsigned long flags; - if (alloc_meta) { - stack_depot_put(alloc_meta->alloc_track.stack); - stack_depot_put(alloc_meta->aux_stack[0]); - stack_depot_put(alloc_meta->aux_stack[1]); - __memset(alloc_meta, 0, sizeof(*alloc_meta)); - } - - if (free_meta && - *(u8 *)kasan_mem_to_shadow(object) == KASAN_SLAB_FREETRACK) { - stack_depot_put(free_meta->free_track.stack); - __memset(&free_meta->free_track, 0, sizeof(free_meta->free_track)); - } + kasan_release_object_meta(cache, object); /* * If init_on_free is enabled and KASAN's free metadata is stored in @@ -170,12 +158,6 @@ static void qlink_free(struct qlist_node *qlink, struct kmem_cache *cache) cache->kasan_info.free_meta_offset == 0) memzero_explicit(free_meta, sizeof(*free_meta)); - /* - * As the object now gets freed from the quarantine, - * take note that its free track is no longer exists. - */ - *(u8 *)kasan_mem_to_shadow(object) = KASAN_SLAB_FREE; - if (IS_ENABLED(CONFIG_SLAB)) local_irq_save(flags); diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c index 99cbcd73cff7..f5b8e37b3805 100644 --- a/mm/kasan/report_generic.c +++ b/mm/kasan/report_generic.c @@ -110,7 +110,7 @@ static const char *get_shadow_bug_type(struct kasan_report_info *info) bug_type = "use-after-free"; break; case KASAN_SLAB_FREE: - case KASAN_SLAB_FREETRACK: + case KASAN_SLAB_FREE_META: bug_type = "slab-use-after-free"; break; case KASAN_ALLOCA_LEFT: @@ -173,8 +173,8 @@ void kasan_complete_mode_report_info(struct kasan_report_info *info) memcpy(&info->alloc_track, &alloc_meta->alloc_track, sizeof(info->alloc_track)); - if (*(u8 *)kasan_mem_to_shadow(info->object) == KASAN_SLAB_FREETRACK) { - /* Free meta must be present with KASAN_SLAB_FREETRACK. */ + if (*(u8 *)kasan_mem_to_shadow(info->object) == KASAN_SLAB_FREE_META) { + /* Free meta must be present with KASAN_SLAB_FREE_META. */ free_meta = kasan_get_free_meta(info->cache, info->object); memcpy(&info->free_track, &free_meta->free_track, sizeof(info->free_track));