From mboxrd@z Thu Jan 1 00:00:00 1970 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 74681C47422 for ; Fri, 26 Jan 2024 14:44:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E9A096B0089; Fri, 26 Jan 2024 09:44:43 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E4A376B008C; Fri, 26 Jan 2024 09:44:43 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D38066B0093; Fri, 26 Jan 2024 09:44:43 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id C3B766B0089 for ; Fri, 26 Jan 2024 09:44:43 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 29FB61C1A72 for ; Fri, 26 Jan 2024 14:44:43 +0000 (UTC) X-FDA: 81721733646.22.F28CAD5 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by imf23.hostedemail.com (Postfix) with ESMTP id 1C97E140018 for ; Fri, 26 Jan 2024 14:44:40 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ieFlDvx5; spf=pass (imf23.hostedemail.com: domain of elver@google.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706280281; 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-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=i5B+fHMvjnJUN3quASTzUIwtKvGYeHenqNIe0ucdsRE=; b=CmjQ54D5gNFVNx+Ui66c//mWYKvNSU61jaqKoCyTmrVBJ9BoINbGFx73OOkLzP73w2t5Dw tUHlh7GD2xWRVCnnz66VJbh9hvFr5os9YqkAvnJvYOWFmk+vGeW/+sAJZqXexxnSE6A3oo xpHM8B/mgUWNzq3SxdxMTAlVMpbpPZs= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ieFlDvx5; spf=pass (imf23.hostedemail.com: domain of elver@google.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706280281; a=rsa-sha256; cv=none; b=gcQkIe3DjGir/GLuFFexOql1aRYMSDgkuBqy2GO1eT5lTQ+1pLPrsLob8j3HqraCzXyoZA VB3cqNpGgmD8eo7p61yfxamjdFfhnEqU6CsqxP+5ycuBAByYOQWazAQ+H+u2aP41146V7N QbxlD39nJsbErsErTZ3ryCmqkOwBS9w= Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-40e775695c6so3608505e9.3 for ; Fri, 26 Jan 2024 06:44:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1706280280; x=1706885080; darn=kvack.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=i5B+fHMvjnJUN3quASTzUIwtKvGYeHenqNIe0ucdsRE=; b=ieFlDvx5/KNXm0dJGOSlx9iiq++mAKnatPFXZYENWcit+/wBkNr9E0tgo+Yq6jcDuO AML0voxd7WVIoVVSPJEOJkJdxfAlLDSH4/GrlVjsJGB3z3Jzw0rViMjhP5U8U26jaujF zo93ThS1cX2iOAyLFHZsFzqWuQ8Wf88R/wMUInnpPd2HxqnnM2ZSNvlyVoMztdsDp//Y 7Lfo2KLtPsleLUUF50TykB/x+LImfPPtyj+Jyw1PWPWEXnUeR6EnMXFY8uKOXYMINLyv CpGKaj6Ymz3Vw60amDcqrx2AB3WLBi/KWmPmP5sgvTeEVXzTjkX2ktq1E0ys3TqDGeCY VpCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706280280; x=1706885080; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=i5B+fHMvjnJUN3quASTzUIwtKvGYeHenqNIe0ucdsRE=; b=LGbTU81rMziAjKzC2FxvXPEhMqm7klDA7tOvtjc+I9WUP1gIDLxpmIPjIUZ0uYmNt5 mZPuyO7guOhkyGwXOiEyIfH++Z6re83/tDu15FI64nj8lgRq36jEexEubzk3LAP17hRt 355GVf0ig/c5KomwHFpjHLTdeOFHuk/XjLJzo5HKXVFP8B3oD7XKUmICypIczVtpZ3MC Cyd1qeBSV8kcd1a19ccOoi+J4sBt96K7HwAU5g3Iqj0YobQ7S+oU6lmzCFz1pQNVeivb sDGtMRivhN+dJVcJUDHLNRqPHCAe6P0BWRcbbIpNFR7UCDfbm3wwoQRGUrCOp1XUa41q mk9Q== X-Gm-Message-State: AOJu0Yy5dJjFu186gvcDK4qPnTr43qUPJXd1c7HbIG9DmfVWNm1pGYsN wY7WT2AWyDjTag6ddaQjhjvuENWBwjDl8VdRpu+zxcvYTR0FcmOIgcTV8rrXrQ== X-Google-Smtp-Source: AGHT+IERvL8VWBU0pIT6Hmg6/b0+n3elHgZZdlfzUMyAnnic5sMEMijkPnCj31BtoZQBu1KA6TJrWA== X-Received: by 2002:adf:f10e:0:b0:337:9d3b:c180 with SMTP id r14-20020adff10e000000b003379d3bc180mr1855231wro.4.1706280279513; Fri, 26 Jan 2024 06:44:39 -0800 (PST) Received: from elver.google.com ([2a00:79e0:9c:201:3837:e733:e624:7fe2]) by smtp.gmail.com with ESMTPSA id f15-20020a056000036f00b00337d84efaf7sm1415718wrf.74.2024.01.26.06.44.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 Jan 2024 06:44:38 -0800 (PST) Date: Fri, 26 Jan 2024 15:44:33 +0100 From: Marco Elver To: Andrey Konovalov Cc: Andrew Morton , Alexander Potapenko , Dmitry Vyukov , Vlastimil Babka , Andrey Ryabinin , Vincenzo Frascino , linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, linux-mm@kvack.org Subject: Re: [PATCH 2/2] kasan: revert eviction of stack traces in generic mode Message-ID: References: <20240125094815.2041933-1-elver@google.com> <20240125094815.2041933-2-elver@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.2.12 (2023-09-09) X-Rspamd-Queue-Id: 1C97E140018 X-Rspam-User: X-Stat-Signature: zdmnhm1uynhfhgqm9zxd8aursm57jnf6 X-Rspamd-Server: rspam01 X-HE-Tag: 1706280280-850450 X-HE-Meta: U2FsdGVkX18MIRPq80CGZMFOf3LYZjMmhwHreuyHmr15lUNvY18QVwj6vnd3OEajILDlB5xjw7V6fRzXkbX5yPc8zHzi+spxNYlS9z9wMFxt7iVHPNLr/dYOLpGnD/jutSpE2qKChFxjh+hZYoGg8K7cZofOFSh1dCkt11tsWN8a0Qv5IjNhSM3w6d+dxOzI5kDknyMXX5tTjNtC8oRd6RIb4+8dcbRZBP8YYrxK6pggv9UDwPqmHQcBwWTV9nwSJBeIX/mW/R958qfOZcDDoMIq5uqHrEaCDjthSGbbaf43KgR4zyJum7lKkk+r9ACjUxstGoOGFygXna5jpCt1VI0Eaq3kW7eSTJMGjPrRPHfd0czfFQAjn7kqigHTZl7OC2ryWoQx+jLolq/V/rtzuxf0A5/f5v5I2APvAaz6UXGC+hD44TZjyaGYL9NjdXHTO3v0p1MuTd2GfIun5eGBhSVQjKTbhbEo7/vCIaX0MJ5/kkPT3KWt9v9Z4X8fG+mmGFTm8HaLM/NQiu8jVkBpXIUcBT4r4p1+qc3Iii4D6iOE/nyYTszdQBf1igdQRRWkLyoJqsozcUO/tIXmheoffYSpqATNmmaD+42xuNI7qA4OHokkX8Cb95UsZu5goID2Ckt5RpO2M8I0sDQTBwQDzKevTyduelLy2aoL0GrvW/gxZ//D2+J0eMBir8zS1JRpB1enuNplg0W977udv3mIf4MffLkAorGuWGiVpqGOXJfido/1BMMYQT1pw8uLovp3UnvABJybMPCImtqJY0ojw7QmNWTFlQ+nm3iPEPjBqedTQnBI4b/Jraw85xRJ21wkoHopUAZwb/OM6NQPNlYdFyMVOOg3SInUjPJaS4AyJtpu0wpIahxzVIeJ8qXW2Hen26wNOJvUnKje5x9Gnzb/4Xy+CkIPFZNxgOCyMbUTHIpS66N2eu9Jt9RJgIdnLR5r05q4peq2dVduxmy/zaw sL5ezmTL ocpn5oUp7X5IJTSqdExIP/KEp5TYyc3j64FrySZXES342qgxlB4CbfJFCc9CtVj7IJf+2x/8JA0o9AZKlC6buyz7B20TH6lMn0S16YZO31aKjV/WTZAgb3rbEdHN3eP6jAbk2pzlxefuAy9TMQq3aGXoLcxxlzuT9OM+1k6hJGHHe88tDcieCMHiV4oUQVvG7tts+YQzO9pjRGKZWTneBF8WYYdvT8AsIiUdJxZ3lP6Mqb2/DjOo0qd6tJSc3hTaydih0OJfP6oo16/3UlS4EEOXLpNRQfx8WPl3Mme7DSt5J9+OCEMZe/z5pFxpQVl88kmfoZCiKAHvS1HRiNp1wap8Qs05KOq594IITokZzFYT4Yxyr6R+tHBER0LhfWs+9OfFoXH3BIS1S9mtK1coffaWnqyBU9I66ujh87hByVZSTtLjBY9NC26KskDfQgG2B/6NeQRm/UbIpCcPWsMY2iF/+G/vdBlBJYHLZM/3eNQPwtbVdMJiT1tf6pA== 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: On Thu, Jan 25, 2024 at 11:36PM +0100, Andrey Konovalov wrote: [...] > > Reviewed-by: Andrey Konovalov > > But I'm wondering if we should also stop resetting metadata when the > object is fully freed (from quarantine or bypassing quarantine). > > With stack_depot_put, I had to put the stack handles on free, as > otherwise we would leak the stack depot references. And I also chose > to memset meta at that point, as its gets invalid anyway. But without > stack_depot_put, this is not required. > > Before the stack depot-related changes, the code was inconsistent in > this regard AFAICS: for quarantine, free meta was marked as invalid > via KASAN_SLAB_FREE but alloc meta was kept; for no quarantine, both > alloc and free meta were kept. > > So perhaps we can just keep both metas on full free. I.e. drop both > kasan_release_object_meta calls. This will go back to the old behavior > + keeping free meta for the quarantine case (I think there's no harm > in that). This will give better reporting for uaf-before-realloc bugs. > > WDYT? Yes, that makes sense. You mean this on top? diff --git a/mm/kasan/common.c b/mm/kasan/common.c index ad32803e34e9..0577db1d2c62 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -264,12 +264,6 @@ bool __kasan_slab_free(struct kmem_cache *cache, void *object, 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); - /* Let slab put the object onto the freelist. */ return false; } diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c index 8bfb52b28c22..fc9cf1860efb 100644 --- a/mm/kasan/generic.c +++ b/mm/kasan/generic.c @@ -510,20 +510,6 @@ static void release_free_meta(const void *object, struct kasan_free_meta *meta) *(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) - release_free_meta(object, free_meta); -} - size_t kasan_metadata_size(struct kmem_cache *cache, bool in_object) { struct kasan_cache *info = &cache->kasan_info; diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h index 216ae0ef1e4b..fb2b9ac0659a 100644 --- a/mm/kasan/kasan.h +++ b/mm/kasan/kasan.h @@ -390,10 +390,8 @@ struct kasan_alloc_meta *kasan_get_alloc_meta(struct kmem_cache *cache, 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_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 3ba02efb952a..a758c2e10703 100644 --- a/mm/kasan/quarantine.c +++ b/mm/kasan/quarantine.c @@ -145,8 +145,6 @@ static void qlink_free(struct qlist_node *qlink, struct kmem_cache *cache) void *object = qlink_to_object(qlink, cache); struct kasan_free_meta *free_meta = kasan_get_free_meta(cache, object); - kasan_release_object_meta(cache, object); - /* * If init_on_free is enabled and KASAN's free metadata is stored in * the object, zero the metadata. Otherwise, the object's memory will