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 B1FB3C48BEB for ; Wed, 14 Feb 2024 22:18:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1ACDA6B009F; Wed, 14 Feb 2024 17:18:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 136F16B00A0; Wed, 14 Feb 2024 17:18:27 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EA3136B00A1; Wed, 14 Feb 2024 17:18:26 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id D14BD6B009F for ; Wed, 14 Feb 2024 17:18:26 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 96ECF140E42 for ; Wed, 14 Feb 2024 22:18:26 +0000 (UTC) X-FDA: 81791824212.13.E17F7C6 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by imf07.hostedemail.com (Postfix) with ESMTP id B316540012 for ; Wed, 14 Feb 2024 22:18:23 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=bsFNFpfX; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf07.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.221.51 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=1707949103; 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=b89wrtq0WG5HUy7Xs4940xVOlE3ZB98xRJ+p4Pz65kk=; b=tFIp/F5yjeprjepFpx0TD73x05phglbj0PDWwUKmcuf20/sV6Lroj87ajZtoCSD09B94rS xU6YiW4tdh/0qxoaNGLtpHbH61zGV2qBP624gwWF0cY4AqXaVCFh8DK1UV/8l372skKfyt i/YlE0LfiUAsQggj470pgnV69lwKuHs= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=bsFNFpfX; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf07.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.221.51 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707949103; a=rsa-sha256; cv=none; b=aHlaDgSaWurnkHSgP/ziaYIqfmlChZrq7UaOiaCHX4Axkvv7HLXbAIC1DwsTXhGCPcIwBk +cHHIQJUVS5KBYwTP5KrhiTh5767YAg5Vt2zO/fYpdYJJUbPDTP2fjuJPcAtPrlvLjbbKG wMRzSneH1BYoWpWK4S0dol/KFu4971E= Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-33cda3dfa06so128555f8f.3 for ; Wed, 14 Feb 2024 14:18:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1707949102; x=1708553902; 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=b89wrtq0WG5HUy7Xs4940xVOlE3ZB98xRJ+p4Pz65kk=; b=bsFNFpfXw3Isfv78XWSRmWrOgqjlOhoU8KDNxxtu7zotxSACjcPzfvw8tTyqUsD5in sSzg0cc/LjQhHSQ41e5uUDif8HvYVVT286OGqc6Iif4oB7XYdGv5pNAb6lCHnzglObsX ICjnjx4jldlEZAe/iVohH+GPHnT/lEYSd1Nc2ls1gknFKULrHVyKWzSLF8Hnnpn/gxKy QfmvGqtoZNJ7561uJC274eY493w2mhiIsjFeIPmqtVyTwOHpxdaNN86pilAju/GRWqyu Qdgnf2fTUuYwt0WuS+0bNB0FMTUb4gSLtGUIGNYLGG1tyVblFko0QQjKKqdo8KO+HE1f rNRQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707949102; x=1708553902; 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=b89wrtq0WG5HUy7Xs4940xVOlE3ZB98xRJ+p4Pz65kk=; b=rdMtoVCUCRVsAv5uOMWgSrWo0jH0/CqTp4kpqTkb/PFgiCZbFPZZEbbZrsL4TubnNQ DzTeQ7FQeb2CNsVeD9xXUFLO8SWuOp+l+5Mde0kxcdZqaodMhlNiSDh5koCPJlBWJbNY ALx7mdmBmFoKKEXKr3DqI4UmRiK21Qz8Mp2zwWriuKdFlDz/UDkyLtEUzLWVwSCW0Prq pxPk02wOBcd9FZyF99hOPTjR6mWvopJg+gfrDp4tNR4sROuiOlRTvHu+0r/vsfYBWo73 Vw/d527fs955gnDNSejTR8DFjXppDg6BV+i7Btxf7aLvBH+32w6vToDB1c4jUBXcZYq8 tNgA== X-Forwarded-Encrypted: i=1; AJvYcCVRMuHyrdrYtmnwqUXCaDC6H5eqiIRRbGj1w26XeIWHAIDSl0MVCJ91yf+pROwbwaoGcRO1xGYupMHEDpImFaA3Oh0= X-Gm-Message-State: AOJu0YxE2D3I0V/g+guxIzv9FL0BrkL7fBiJRHj/p+yHOtv5/tXs8o6P bEK6gC2t2RhVnGqhLoykLpgOhxZWX1FMTiqRVoCoE6qgjVWiIzRiLICD9BoETNPOoEFle+xb9uU /5YjPv/eIdnvqVakhfkJ7NJCrbaE= X-Google-Smtp-Source: AGHT+IEtx6ciCCenkTh9gHKDD8zYafZ2ZTQ4e0POVY4gaaRLQawihQLMbXEaRvLvAhjdMPAR0qMlxz2+Dx+/4MjEtH8= X-Received: by 2002:a05:6000:1247:b0:33b:4d13:a1b2 with SMTP id j7-20020a056000124700b0033b4d13a1b2mr2634603wrx.30.1707949102194; Wed, 14 Feb 2024 14:18:22 -0800 (PST) MIME-Version: 1.0 References: <20240213033958.139383-1-bgray@linux.ibm.com> In-Reply-To: <20240213033958.139383-1-bgray@linux.ibm.com> From: Andrey Konovalov Date: Wed, 14 Feb 2024 23:18:11 +0100 Message-ID: Subject: Re: [PATCH] kasan: guard release_free_meta() shadow access with kasan_arch_is_ready() To: Benjamin Gray Cc: kasan-dev@googlegroups.com, mpe@ellerman.id.au, ryabinin.a.a@gmail.com, glider@google.com, dvyukov@google.com, vincenzo.frascino@arm.com, akpm@linux-foundation.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: B316540012 X-Stat-Signature: b4fhqpw5y9a8e1sk7ccj9wgswb7kkbz5 X-HE-Tag: 1707949103-862437 X-HE-Meta: U2FsdGVkX19FliCiKgqyulfaqVHBycUWPyCHtHxiV/hqnyKhOVAvxP5UsKloYlPYAIXEUoZhlEgKJLCIshP3fgGHMmr9AppHvPuDHRJlSZUEITUemzmnjYpGW+q8R0VMrO8KSbXdw1//2QSWb4LwX7HiXDKini09znGdjMHXLAT+CRWd3ggjAuMMjYs6rfNe/9ozjSrHFH8cn3gSuLlyNyeqoIWtnibAi240UjtThsRUJFVPyT7ndD1t+gpag4At8GgwmH1nhLoLPbeFWI0vlXxTpAQ5GaGVmWqxdeWGPdpIoFgMJ68Sm2Em0soZY/EP9TLb02f/IScVl7si+cabjgMJFV2QW9LSTKflEzVVXHpIMNPhlhg/BwJl5BTdZb15CmPezMEAa1A2pMfhvP52kdG+89SnQGUr7hFxrzjPnNpBhcvtYE+DHMmP5YiN4zN98fr85fk9soHZDH3uYj995l2Bq3bP7lwwbni9/3e8O+ybZBgxZjhAeVPZhaKHhxgoPuW6Rn4FFCgwddUaCHXCYw4K5m38ZGINEC1v2BIBdXQzA6ENeMMHnOxlakwoqi010FeIbNArWE3OeXEpAaCfTcWnrorcewrf2wTX0D33wTPUhZhTHEoNWBD4G/VHf4wTmuJVp9+YBfPLlgQ4OSauI9hQUDCT7Nfw3k+gZ/iRS4CboplBwolm+GyIhJ74peEWL9884ZMFQhuUWc9Q0JFIivdOJnMx1QQVDj+RaGmeUvyKcvt02jsZDJ19HlhAVfj+N+oEleEmBens4byjdsD4HXsTQNvv3JYCDnSAMgeABP/yzAwAwCOeptDEEvUwfQHz9dOPHvXra9fml0TkR7HdKz+FsA+roLswmPix1N313VJUhisU1BbdQZV+mcsZPldmmFPOiB+me4L/RPE/t5rzWqAovyE4qnJ3Vy5nnc1i9L4hUul3e9f53K54A+4AU7ASqmAZuhEWWpSqvwbLzHK 7qQMJVZW 9HE2gIQzS5plU6phL/KW3IHZ/UTujgKD/qerqiUSZLrRoYzae9+yy9YDPSBOpBvEmyP9AnwCUctPMsX6wie1o7dlVMVJ5Zp6MAEZxtGUxjkKJDA9UTZf2FCZKzNtuBgyOp6S9WK3TY6krsXBzFKGwyHhjcJRRjCa28NN0OyFTmF+veZZP9KmhwiLhyRwFacx18zZEYvRVv31ElWyPe/DXqq8Land0o5ltpgzVoUmvkI5Lt0euO6zUH64EBKPngxdsq9UA9WFmTK8+dwd0s4b2S0grdxfYaRVXYa0xWtytIcETg8qW1RfWi14xKBL/ncXbR6akTMARfw3aB4V84zvB51kt8AiWAOBw4y1cXnvkotrebDcAY2FvIwHEIw1hhXXoVp56IvNF8wfwScfuZqwuyqOwQ0ucY9GZGIQ0jrCORZD2ee4LdSjU+3PYawX700sUFAdSFW2KgRAE/BdKAlIDEGeH2/hNCjBeVgklCjddcvcQ4Ilp45bgh5/A92D3rFcw5MoAX5s0nEpxXKMnPSkEW7OXB+9Lt2kE0+cc4fAGo2qMR1B0prShjTYpVF9elJnaakVH 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 Tue, Feb 13, 2024 at 4:40=E2=80=AFAM Benjamin Gray = wrote: > > release_free_meta() accesses the shadow directly through the path > > kasan_slab_free > __kasan_slab_free > kasan_release_object_meta > release_free_meta > kasan_mem_to_shadow > > There are no kasan_arch_is_ready() guards here, allowing an oops when > the shadow is not initialized. The oops can be seen on a Power8 KVM > guest. > > This patch adds the guard to release_free_meta(), as it's the first > level that specifically requires the shadow. > > It is safe to put the guard at the start of this function, before the > stack put: only kasan_save_free_info() can initialize the saved stack, > which itself is guarded with kasan_arch_is_ready() by its caller > poison_slab_object(). If the arch becomes ready before > release_free_meta() then we will not observe KASAN_SLAB_FREE_META in the > object's shadow, so we will not put an uninitialized stack either. > > Signed-off-by: Benjamin Gray > > --- > > I am interested in removing the need for kasan_arch_is_ready() entirely, > as it mostly acts like a separate check of kasan_enabled(). Dropping kasan_arch_is_ready() calls from KASAN internals and instead relying on kasan_enabled() checks in include/linux/kasan.h would be great! I filed a bug about this a while ago: https://bugzilla.kernel.org/show_bug.cgi?id=3D217049 > Currently > both are necessary, but I think adding a kasan_enabled() guard to > check_region_inline() makes kasan_enabled() a superset of > kasan_arch_is_ready(). Sounds good to me. I would also go through the list of other exported KASAN functions to check whether any of them also need a kasan_enabled() check. At least kasan_unpoison_task_stack() seems to be one of them. > Allowing an arch to override kasan_enabled() can then let us replace it > with a static branch that we enable somewhere in boot (for PowerPC, > after we use a bunch of generic code to parse the device tree to > determine how we want to configure the MMU). This should generally work > OK I think, as HW tags already does this, We can also add something like CONFIG_ARCH_HAS_KASAN_FLAG_ENABLE and only use a static branch only on those architectures where it's required. > but I did have to add another > patch for an uninitialised data access it introduces. What was this data access? Is this something we need to fix in the mainline= ? > On the other hand, KASAN does more than shadow based sanitisation, so > we'd be disabling that in early boot too. I think the things that we need to handle before KASAN is enabled is kasan_cache_create() and kasan_metadata_size() (if these can even called before KASAN is enabled). Otherwise, KASAN just collects metadata, which is useless without shadow memory-based reporting anyway. > --- > mm/kasan/generic.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c > index df6627f62402..032bf3e98c24 100644 > --- a/mm/kasan/generic.c > +++ b/mm/kasan/generic.c > @@ -522,6 +522,9 @@ static void release_alloc_meta(struct kasan_alloc_met= a *meta) > > static void release_free_meta(const void *object, struct kasan_free_meta= *meta) > { > + if (!kasan_arch_is_ready()) > + return; > + > /* Check if free meta is valid. */ > if (*(u8 *)kasan_mem_to_shadow(object) !=3D KASAN_SLAB_FREE_META) > return; > -- > 2.43.0 > For the patch itself as a fix: Reviewed-by: Andrey Konovalov Thanks!