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 AC3DCC47422 for ; Sat, 27 Jan 2024 01:53:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 28E026B0081; Fri, 26 Jan 2024 20:53:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 23E326B0082; Fri, 26 Jan 2024 20:53:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1066B6B0083; Fri, 26 Jan 2024 20:53:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 00B5D6B0081 for ; Fri, 26 Jan 2024 20:53:37 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id A0585161023 for ; Sat, 27 Jan 2024 01:53:37 +0000 (UTC) X-FDA: 81723419274.22.5149FC7 Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) by imf13.hostedemail.com (Postfix) with ESMTP id DF2E62000B for ; Sat, 27 Jan 2024 01:53:35 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=SkF4Bvlz; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.221.46 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706320416; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=WXrbWEt/1qQdJy8DYI2a1Mv/37w6xBSoBjlES19/LuY=; b=NhC4+qWzbQ19NRrze79xgieXqgbwFx1IDrQ8dFcelJs1OBG8PlusBokBheji45l491MOkG Bue+CNZ3f+T+tXu9DyvErDDgeXwbZQpm/dhw/1lBJVKt1mGUuyeyk4QpiuETX5E9XvYu+D 4N7+8jpOGcBvb7F+XDfv6xvLo8EB3Uc= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=SkF4Bvlz; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.221.46 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706320416; a=rsa-sha256; cv=none; b=FOhNU9HBEgKuozC4qQYoCbG+rV0Vps713O5R09qYrCEIH/km1/Fv64Pwle/8G82stj3mF0 mVLOjKlU+NzmwvGy4QYV96NSB6GusdeS+yDObxRskXVo3IbDxZVYqkbwuvwnZPAkMmbhzW bmuVOW9oysEtTaIi692og9qn/OxpKYo= Received: by mail-wr1-f46.google.com with SMTP id ffacd0b85a97d-3392b045e0aso1232286f8f.2 for ; Fri, 26 Jan 2024 17:53:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1706320414; x=1706925214; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=WXrbWEt/1qQdJy8DYI2a1Mv/37w6xBSoBjlES19/LuY=; b=SkF4BvlzKP+EiFizh6a3o7W7sspKUrCpwQlWRfsKus2hxuweOYVqn7GA5hQItu9Hc5 9ISOFQq6HiNZ4sTEsKio60X7cofxLL7Uh/DBkf7/fTdk8jrUjjPA/xu4T2laivqT8INJ OZtPEyNnw+MZByi8MVYBcjMTeg5oKWI8UqehQPx7dmn3rLimuotj717Fgu/BdtN2ZhAl ccybeJxItv3NvLZAanZtydc5EOfuATNBLqgei9MZ6zYPHTy+VNtQWoZCM3ffXLD1Rd4Y bMxkePHBOQ6f2jGmyjjYSD71/P/hEhEh1PW8eaUg50UdATyDLiJ4IRBPKDkYpax4QXzQ koJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706320414; x=1706925214; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=WXrbWEt/1qQdJy8DYI2a1Mv/37w6xBSoBjlES19/LuY=; b=AUGnXw8Y7yjxXTZP7mEUtOvupDQQQk0+tiRdPHFK6wpJmkguYTIu2QL4XslWIUNPo+ IjyZflTNGSJaxjvpOppPGJh992SKQ8RyzQkJBnbzfNTwPd0F4y9vU0oz9w4T7CB+WxsJ l0A6PCK20P+OBIwjgPnSnTOO7l1GkGuTA2lZAJ7YlVhDNkTbW9NoO2a3k3bluF29nj9K xCcwk/f+iJfmYemdHuxU4e3N07DE4VoDCXH7DVRCmm1i644VlNlBn99Wg0ISGT6n+UpK gfcfBUGVACrVMCBWgWbxqv9OSVDaj6TeFFUcWa27N+J6Q3I8MZYU34hbEynWN66KJLh0 GIgA== X-Gm-Message-State: AOJu0YyiI97PGe8+a3w8tSqXqjG4VEB9r1FF6r8Tx4CBHP9sjel+ejbz JA6Cs2hKpR2ccIa+sN/DMQ+ubBvUFbnlbmq9913lXcc/EFR1U7mejNYj3ZuG7yyW6uoWANiyURv hy625ou3Z8V6noxrRJhtlG2Y/PSI= X-Google-Smtp-Source: AGHT+IFIaRsewSFaU5O34oAPpEWxPXr10elT1/kf7v442sw5K56ZXlfin0AAl4T0yDZGZxTbrRZKh67LZJ46L4lOcwA= X-Received: by 2002:a05:600c:3515:b0:40d:5b0c:736c with SMTP id h21-20020a05600c351500b0040d5b0c736cmr447423wmq.127.1706320414219; Fri, 26 Jan 2024 17:53:34 -0800 (PST) MIME-Version: 1.0 References: <20240125094815.2041933-1-elver@google.com> <20240125094815.2041933-2-elver@google.com> In-Reply-To: From: Andrey Konovalov Date: Sat, 27 Jan 2024 02:53:23 +0100 Message-ID: Subject: Re: [PATCH 2/2] kasan: revert eviction of stack traces in generic mode To: Marco Elver 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: DF2E62000B X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: q1yw11auttmey8djdnhr43wwetcao1bh X-HE-Tag: 1706320415-741623 X-HE-Meta: U2FsdGVkX1/dItAcKSbA3FQPkstxPwUWWEpJPwGv9vT6YfAmngaWf79VConZ7tugwcRDNF6vLjp/U1Uo4lnLVBVbavBLmdaPytpfmHjLJPMUr4PDn9uDAwWxzM6HMAIZvcOkvtOEsMkapO24bsznEUf0YrUfPb/a42vNbcYOj4XNDg7tS6kAbCr9p/uvXhvkp66hzZhHAtF5/BXcOGud9pu9WZIfT65GWm3mfrWKUWQkyO31HiPWv0o/rNybIQAYmQLy9ISLtTZS/71VFglV0Ymp0XHSnTz9QnyQlp2QaT+mBAGsCx8mpEbjFqYeszztGI70tnh8ZuZ7MPwMKBY0oCWerbqPeW8CZgQDUKCOYsCW3E1OkUiG3fAmyg2fdzVd1Ebq/IMHEUS+QUGliA2uuBLaS5ncH/F0Vk/nbzrQEb6iZdUg+JOWPbOrPw4fR4CoMgXSKuoJVVYlU08TV1Xs46L6r0oI2ybfwpCJWlLaLjy5eG6HU8rTyE3qQ5/ckCWvhZiQtvisDZubkC/SvAsaiEbBjvMCnz6fzwaaIWJwVIPzCKVU/YiSaM6Xm90UWsS4u9fjJASwEioQymYIUYMtQ9hQ3JaHGOdBRZZoCwILROrJIfN2DSS04bwlwbd1MvUcWV/2QK5Nrr6jTnGn1Yqdn3OfMNJDmRq3N+YOUsSik8lMY3xMN0oKrvbAleO2I/r1243Ds6+gdTFxMSZwPsEC2HGPuoELWoYDaA7UJe8zDG6aUAcjAqyu0a9K7auvoLM4Cxp3wvdgJNWr9JTokhOVKs4O9WaP9gLcKFv7wxHT0BCeGh7ACuhOMUu6hoouzMc6cL5hXZ7+uiLWjpyXWmrAQzzm9f4jqftfy31rnXp9mxja/grwjrCF/9n7xhzr1uv/FSL65cv5GKIaFS0XOnsPpys1T2QscRARYnDGPXu1V/WP3aERgcubXQvfM+zd7+mxNj7YghHOAo4R/hhCqFG ZUFDEvgr 6Qh9u7Wx8iY0GDze2216BT940ZLkOP375VDC4kv/WmFWHhzkWcIwWDTil5T3L2f16KMUc5vCpn8bBshQjWIi2k2NzPDwHYJqZZtoRgLUFpfjnbj00iwUjA+7m4pZgOpbyvqx8oGd70eGHMQsE6KDZrsWIVLTZB2803fZQx/d/txqJjTLJsmL7wFX0tptawJzUELQ/LZNFvWDVKupWRQ65UDPfELjUUx92v/rzJJlmbjxZN4kbGQRNThfr/Gks5ftRJPcOj/tSoPXOWH727YzQzRls3BvaTWTDPucSvsl3NzwHoykj6KByK9TCIs716v55MB6s96YFu1M6RWIV2mPwd9xbhocSGRB6J50gGjHZkt55N63jD9r1ca02AcTEQsIsv/hY5acr72Q1JuVF/0sd8JEOuEvNJXaJysgPw9G6cAiwwxv2KEZ7ggiM24EVYFNjDU01A18vvn5SN5u9vX8Si/aoXw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000007, 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 Fri, Jan 26, 2024 at 3:44=E2=80=AFPM Marco Elver wrot= e: > > 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, voi= d *object, > if (kasan_quarantine_put(cache, object)) > return true; > > - /* > - * If the object is not put into quarantine, it will likely be qu= ickly > - * 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, st= ruct kasan_free_meta *meta) > *(u8 *)kasan_mem_to_shadow(object) =3D KASAN_SLAB_FREE; > } > > -void kasan_release_object_meta(struct kmem_cache *cache, const void *obj= ect) > -{ > - struct kasan_alloc_meta *alloc_meta; > - struct kasan_free_meta *free_meta; > - > - alloc_meta =3D kasan_get_alloc_meta(cache, object); > - if (alloc_meta) > - release_alloc_meta(alloc_meta); > - > - free_meta =3D 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 =3D &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 *obj= ect); > #else > static inline void kasan_init_object_meta(struct kmem_cache *cache, cons= t void *object) { } > -static inline void kasan_release_object_meta(struct kmem_cache *cache, c= onst void *object) { } > #endif > > depot_stack_handle_t kasan_save_stack(gfp_t flags, depot_flags_t depot_f= lags); > 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, stru= ct kmem_cache *cache) > void *object =3D qlink_to_object(qlink, cache); > struct kasan_free_meta *free_meta =3D 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 Please also add a comment saying something like "Keep per-object metadata to allow KASAN print stack traces for use-after-free-before-realloc bugs." to the places where you removed kasan_release_object_meta. Otherwise, looks good to me.