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 9F712C433EF for ; Tue, 31 May 2022 17:16:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 27BE06B0073; Tue, 31 May 2022 13:16:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 227D76B0074; Tue, 31 May 2022 13:16:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 07A9C6B0075; Tue, 31 May 2022 13:16:16 -0400 (EDT) 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 E97F46B0073 for ; Tue, 31 May 2022 13:16:15 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay12.hostedemail.com (Postfix) with ESMTP id A64CF12127A for ; Tue, 31 May 2022 17:16:15 +0000 (UTC) X-FDA: 79526691510.15.CBFBFF6 Received: from mail-il1-f169.google.com (mail-il1-f169.google.com [209.85.166.169]) by imf01.hostedemail.com (Postfix) with ESMTP id 866EB40069 for ; Tue, 31 May 2022 17:16:08 +0000 (UTC) Received: by mail-il1-f169.google.com with SMTP id a15so10037692ilq.12 for ; Tue, 31 May 2022 10:16:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=VRR/7w506vBh7WOgx3P+Z/7F/uNHjpGKlcsdV9A5CEQ=; b=EWBdzD/WyerGAoYXKFB14Zvssq1g24JkseG4cjjm8AnkrTHOCqSI7yJBJoCJqCjjI/ YQ+N59VEEonXWGIBJByAkKsYlfGFiMQak9QqHNzd0FnNEGLScAGgM5+EH3imkS0Gq4xE cRSYAJoFUOCdpORc+addGgxL9dewP8y64jLw0IzhAhTSpFmnnbRxD3StOBME47xoJ9aF 0hcZlauCOEW5MtbjNNg0/U/YgSoa42Aqw6MKkZw+a0ANjRLZMmO9xRkc2dUp7zwWyPS8 lQ7uRdzu8bJOyMB1xnIMAxrg2h6vHhIcG2M0t6TPIJMLl7z2OQX2WpPQJqQKyWCpVcWT 0lLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VRR/7w506vBh7WOgx3P+Z/7F/uNHjpGKlcsdV9A5CEQ=; b=58kge4cTmoR4wBPXXRZfDtPdCQDT+nkjPiWxMBuUx7/Fn9O+kBiySketvd0YcVrjL6 IwSpcTsP8evHy52yjo+7cjcKKORFEDRW9Jl5gXFZzI1pAidayntA1OrO7DDpqxFwHXQU YCkaqT+WmvKs2LQFYVJlIedWWs/mXCvfqfJrnhcSR73LHJ1sK8h2ORlixO6BLAoVL6xv zbFreMWpQzFCp3yOMdbivRLeSUvz0nDlLYHT6cKxANLdccEP2veAKggYtXIif+EgxRnJ rXuna/MLmAtK18IuXqqS7KB1NRKkjeGu6a1joaSC5VVwk7FhBNE46eVyedSTkqykMEbq AmOw== X-Gm-Message-State: AOAM532kzulO65Mcdiktsorw0e7wGvQ7i5z65anVVsi3AIsFKX8z5Veb JHcFMEEZxBAGEAkTaV9bUCdCuHSYg7+pje/phMA= X-Google-Smtp-Source: ABdhPJzV8JO6pZTC/X1L0QHgT0OU/7cODoenV8Hf1cf4HnezfbpXJGCjTbF7WDq/gWciScYmi+55VFtQqLw9PFzzcX8= X-Received: by 2002:a05:6e02:1c2a:b0:2d1:9e4c:203d with SMTP id m10-20020a056e021c2a00b002d19e4c203dmr23852118ilh.235.1654017374514; Tue, 31 May 2022 10:16:14 -0700 (PDT) MIME-Version: 1.0 References: <20220517180945.756303-1-catalin.marinas@arm.com> In-Reply-To: From: Andrey Konovalov Date: Tue, 31 May 2022 19:16:03 +0200 Message-ID: Subject: Re: [PATCH 0/3] kasan: Fix ordering between MTE tag colouring and page->flags To: Catalin Marinas Cc: Andrey Ryabinin , Will Deacon , Vincenzo Frascino , Peter Collingbourne , kasan-dev , Linux Memory Management List , Linux ARM Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 866EB40069 X-Stat-Signature: uzauc8e64qf6gprr6n9h8qqcoqdsdxw9 X-Rspam-User: Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="EWBdzD/W"; spf=pass (imf01.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.166.169 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspamd-Server: rspam08 X-HE-Tag: 1654017368-530238 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: On Thu, May 26, 2022 at 2:24 PM Catalin Marinas wrote: > > On Wed, May 25, 2022 at 07:41:08PM +0200, Andrey Konovalov wrote: > > On Wed, May 25, 2022 at 5:45 PM Catalin Marinas wrote: > > > > Adding __GFP_SKIP_KASAN_UNPOISON makes sense, but we still need to > > > > reset the tag in page->flags. > > > > > > My thought was to reset the tag in page->flags based on 'unpoison' > > > alone without any extra flags. We use this flag for vmalloc() pages but > > > it seems we don't reset the page tags (as we do via > > > kasan_poison_slab()). > > > > I just realized that we already have __GFP_ZEROTAGS that initializes > > both in-memory and page->flags tags. > > IIUC it only zeroes the tags and skips the unpoisoning but > page_kasan_tag() remains unchanged. No, it does page_kasan_tag_reset() via tag_clear_highpage(). At least, currently. > > Currently only used for user > > pages allocated via alloc_zeroed_user_highpage_movable(). Perhaps we > > can add this flag to GFP_HIGHUSER_MOVABLE? > > I wouldn't add __GFP_ZEROTAGS to GFP_HIGHUSER_MOVABLE as we only need it > if the page is mapped with PROT_MTE. Clearing a page without tags may be > marginally faster. Ah, right. We need a dedicated flag for PROT_MTE allocations. > > We'll also need to change the behavior of __GFP_ZEROTAGS to work even > > when GFP_ZERO is not set, but this doesn't seem to be a problem. > > Why? We'd get unnecessary tag zeroing. We have these cases for > anonymous, private pages: > > 1. Zeroed page allocation without PROT_MTE: we need GFP_ZERO and > page_kasan_tag_reset() in case of later mprotect(PROT_MTE). > > 2. Zeroed page allocation with PROT_MTE: we need GFP_ZERO, > __GFP_ZEROTAGS and page_kasan_tag_reset(). > > 3. CoW page allocation without PROT_MTE: copy data and we only need > page_kasan_tag_reset() in case of later mprotect(PROT_MTE). > > 4. CoW page allocation with PROT_MTE: copy data and tags together with > page_kasan_tag_reset(). > > So basically we always need page_kasan_tag_reset() for pages mapped in > user space even if they are not PROT_MTE, in case of a later > mprotect(PROT_MTE). For (1), (3) and (4) we don't need to zero the tags. > For (1) maybe we could do it as part of data zeroing (subject to some > benchmarks) but for (3) and (4) they'd be overridden by the copy anyway. Ack. > > And, at this point, we can probably combine __GFP_ZEROTAGS with > > __GFP_SKIP_KASAN_POISON, as they both would target user pages. > > For user pages, I think we should skip unpoisoning as well. We can keep > unpoisoning around but if we end up calling page_kasan_tag_reset(), > there's not much value, at least in page_address() accesses since the > pointer would match all tags. That's unless you want to detect other > stray pointers to such pages but we already skip the poisoning on free, > so it doesn't seem to be a use-case. Skipping unpoisoning makes sense. > If we skip unpoisoning (not just poisoning as we already do) for user > pages, we should reset the tags in page->flags. Whether __GFP_ZEROTAGS > is passed is complementary, depending on the reason for allocation. [...] > Currently if __GFP_ZEROTAGS is passed, the unpoisoning is skipped but I > think we should have just added __GFP_SKIP_KASAN_UNPOISON instead and > not add a new argument to should_skip_kasan_unpoison(). If we decide to > always skip unpoisoning, something like below on top of the vanilla > kernel: [...] > With the above, we can wire up page_kasan_tag_reset() to the > __GFP_SKIP_KASAN_UNPOISON check without any additional flags. This would make __GFP_SKIP_KASAN_UNPOISON do two logically unrelated things: skip setting memory tags and reset page tags. This seems weird. I think it makes more sense to split __GFP_ZEROTAGS into __GFP_ZERO_MEMORY_TAGS and __GFP_ZERO_PAGE_TAGS: the first one does tag_clear_highpage() without page_kasan_tag_reset() and the second one does page_kasan_tag_reset() in post_alloc_hook(). Then, add __GFP_ZERO_PAGE_TAGS to GFP_HIGHUSER_MOVABLE along with __GFP_SKIP_KASAN_UNPOISON and __GFP_SKIP_KASAN_POISON. And replace __GFP_ZEROTAGS with __GFP_ZERO_MEMORY_TAGS in alloc_zeroed_user_highpage_movable(). An a alternative approach that would reduce the number of GFP flags, we could extend your suggestion and pre-combining all standalone MTE-related GFP flags based on their use cases: __GFP_KASAN_VMALLOC == essentially __GFP_SKIP_KASAN_UNPOISON | __GFP_SKIP_ZERO __GFP_MTE_USER == essentially __GFP_ZERO_PAGE_TAGS | __GFP_SKIP_KASAN_UNPOISON | __GFP_SKIP_KASAN_POISON __GFP_MTE_USER_ZERO == essentially __GFP_ZERO_MEMORY_TAGS Then we would only need 3 flags instead of 5. However, this seems to be unaligned with the idea that __GFP flags should enable/disable a single piece of functionality. So I like the first approach better. What do you think? Thanks!