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 8505FC25B10 for ; Mon, 13 May 2024 18:37:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 11D066B00D5; Mon, 13 May 2024 14:37:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F3DC6B00DD; Mon, 13 May 2024 14:37:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ED6426B00E3; Mon, 13 May 2024 14:37:01 -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 CEF1A6B00D5 for ; Mon, 13 May 2024 14:37:01 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 78B931C0EDE for ; Mon, 13 May 2024 18:37:01 +0000 (UTC) X-FDA: 82114229442.14.9241B79 Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) by imf03.hostedemail.com (Postfix) with ESMTP id 905D420010 for ; Mon, 13 May 2024 18:36:59 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=rivosinc-com.20230601.gappssmtp.com header.s=20230601 header.b=y8uj2o7p; spf=pass (imf03.hostedemail.com: domain of charlie@rivosinc.com designates 209.85.210.179 as permitted sender) smtp.mailfrom=charlie@rivosinc.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1715625419; a=rsa-sha256; cv=none; b=bE674nNebJHpu28R+9GvCxFJeeZxpP4NshMKiqOKXl39CeYF4W04ypufRfCXCf1KokAvVI KSo07GIAoTwpYRvKE+kAczRBtEDoORt12ajIpPBddFo6K/4AL5+n+qkoQmmTe654Ac5H4F 3DW8cUOpJjMCrkIeAkJ7mMzDD6yjVSc= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=rivosinc-com.20230601.gappssmtp.com header.s=20230601 header.b=y8uj2o7p; spf=pass (imf03.hostedemail.com: domain of charlie@rivosinc.com designates 209.85.210.179 as permitted sender) smtp.mailfrom=charlie@rivosinc.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1715625419; 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=EJ8xaQQ9gfY7+cG8KujzU2LQ7mZBfQgErlO4DocitkQ=; b=xT+fz7MlGsKyKQtFNrJC9cz0BaJ7rDHrigeLE6sqz1qKatC3occ85Ix4urCjkGka0jb16q Qw+cSMY4x+YJ/njcXt2L/ItOUm6qAV0l75hnBMW0AiToHJYiAGcBlZBAKNxlxxRWQRCjmr CF1PmVIHCAHOfYRO0iARJar6TFRXU3Q= Received: by mail-pf1-f179.google.com with SMTP id d2e1a72fcca58-6f489e64eb3so3364782b3a.1 for ; Mon, 13 May 2024 11:36:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1715625418; x=1716230218; darn=kvack.org; h=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=EJ8xaQQ9gfY7+cG8KujzU2LQ7mZBfQgErlO4DocitkQ=; b=y8uj2o7pM05WKK5ch38lyh3yls5ovjS4VdTBuuLesMl+sUcnOoWgf6Jp7AbeQSILCW GepcCdP+vStnM2BlFf3Ypkb8Xyk+uR+dSPoEOBmU1zEBqzIqUu1LfKHSSEs61MemBQEx UgDTWpxytJShUuYHVcd5oYwOc7vZdXBmaEiTJ6prUTRalZoh7+jdY8QFPDh90XnvBPJB RjWrjhz82+eCe+RQNwm2n5yMCIGHOBcbEVK9nWJ9UyBOZZWx9ZsurXRs/wsmLgayLaWX ONP5xtyoPxGaufqF6cRa/QUIsrzNftX8Kh/KMUSuQvGDxKRG3iwqkxz35f0ZqNotK09N /4VA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715625418; x=1716230218; h=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=EJ8xaQQ9gfY7+cG8KujzU2LQ7mZBfQgErlO4DocitkQ=; b=B4e7+NSbQIovGkZKn1TJx6hqC1KlXVvnFB8SNW2tXfmxy6DrSPKvhKZvbg8KlBeLjo G9McmSWh2Ghn079HmdVXRYL2UwIi5aznNxMocRfjiJk3hNCO33D630Ghpj82RWOb720e pAVnfHdYRw1UWBZJKAeazo2OitmGqacKAtF8i0ibfjpF+qghCkzEgBs1xiOGv/nZ5BlR IACQcTUbSDeDmcXms0w48FPt328eRahNwBn50yJBZHhB3+jALtSg8jvlL0lNjnwVQIy+ wnz6+uyFP8N9ichCiAA1sKw2fxrMAyUSaz5GkAz1aBJft2gYmgSZbSqmBXbo5LFwNoi2 sshg== X-Forwarded-Encrypted: i=1; AJvYcCVEQ1ml5gI17N8Bw+AAkZ+JCLvX6TgL7lJrQ4EEVWPnwSiX3C/HnliqZQ32DfyAKp3g2dfHxtC8PAWu0udXqdu34rs= X-Gm-Message-State: AOJu0YzIi77MrXPrKvWaIiLw3IQXoL3vvU2Pb8g4bk5l5HsbISx+rCiv qLOp/D5zf+Z8tFBCA5ujcEuJIKpFQuB7K67Jibn7R9yUdnZGwiJi7/G+ZRS5ulY= X-Google-Smtp-Source: AGHT+IEtFJqHdWFS93HqHX5hpNlL66iEYhRvuRmrd98dRBICcH+zlHlVBgPVDx05Rv0/gkrNtS6Lwg== X-Received: by 2002:a05:6a00:2406:b0:6f3:86ac:5eae with SMTP id d2e1a72fcca58-6f4e03843d1mr10090101b3a.28.1715625418280; Mon, 13 May 2024 11:36:58 -0700 (PDT) Received: from ghost ([2600:1010:b062:ae34:7efe:e26b:c29e:9a14]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-6f4d2a9d9acsm7881752b3a.90.2024.05.13.11.36.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 May 2024 11:36:57 -0700 (PDT) Date: Mon, 13 May 2024 11:36:49 -0700 From: Charlie Jenkins To: Deepak Gupta Cc: paul.walmsley@sifive.com, rick.p.edgecombe@intel.com, broonie@kernel.org, Szabolcs.Nagy@arm.com, kito.cheng@sifive.com, keescook@chromium.org, ajones@ventanamicro.com, conor.dooley@microchip.com, cleger@rivosinc.com, atishp@atishpatra.org, alex@ghiti.fr, bjorn@rivosinc.com, alexghiti@rivosinc.com, samuel.holland@sifive.com, conor@kernel.org, linux-doc@vger.kernel.org, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-kselftest@vger.kernel.org, corbet@lwn.net, palmer@dabbelt.com, aou@eecs.berkeley.edu, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, oleg@redhat.com, akpm@linux-foundation.org, arnd@arndb.de, ebiederm@xmission.com, Liam.Howlett@oracle.com, vbabka@suse.cz, lstoakes@gmail.com, shuah@kernel.org, brauner@kernel.org, andy.chiu@sifive.com, jerry.shih@sifive.com, hankuan.chen@sifive.com, greentime.hu@sifive.com, evan@rivosinc.com, xiao.w.wang@intel.com, apatel@ventanamicro.com, mchitale@ventanamicro.com, dbarboza@ventanamicro.com, sameo@rivosinc.com, shikemeng@huaweicloud.com, willy@infradead.org, vincent.chen@sifive.com, guoren@kernel.org, samitolvanen@google.com, songshuaishuai@tinylab.org, gerg@kernel.org, heiko@sntech.de, bhe@redhat.com, jeeheng.sia@starfivetech.com, cyy@cyyself.name, maskray@google.com, ancientmodern4@gmail.com, mathis.salmen@matsal.de, cuiyunhui@bytedance.com, bgray@linux.ibm.com, mpe@ellerman.id.au, baruch@tkos.co.il, alx@kernel.org, david@redhat.com, catalin.marinas@arm.com, revest@chromium.org, josh@joshtriplett.org, shr@devkernel.io, deller@gmx.de, omosnace@redhat.com, ojeda@kernel.org, jhubbard@nvidia.com Subject: Re: [PATCH v3 10/29] riscv/mm : ensure PROT_WRITE leads to VM_READ | VM_WRITE Message-ID: References: <20240403234054.2020347-1-debug@rivosinc.com> <20240403234054.2020347-11-debug@rivosinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 905D420010 X-Rspam-User: X-Rspamd-Server: rspam12 X-Stat-Signature: k7nyyzsuc65okrhgu6tdftw1ra63838c X-HE-Tag: 1715625419-672918 X-HE-Meta: U2FsdGVkX188+FqhBnbk3EqXPeO+c8rBMtqBQMqfRStXJLRMd0H5G8dHMF2HhpYpW58qqoqsS9JBUsomqYABrY0preVMQQqGabHjzDL758hBjYTLJhX0FqnUNErilUjAEFsmt8V22OhOqo4lLQOeCUU9r4zGH1M69OQiIaeMU3LCVUvOkfSrfId/fUVPLHUGz2+8+XHSRMX5lMVO3vZl/VPrBTmrVGozoEpxVA2ty2kfmbBOdFF8sUB2z7R8fxjtCE4NlB4a+YQWJKVNkooHBdrmQsf6a+6eZipeKEDfroJj/zdGQup7t7vIB8Tmv3kiBK3W5VN6Cs2a2sP48BMqjEBlE13XBTG2LicUgzf15s6b1nRdd6I4aGxswyTpE94Oc+1OC6GE4JPnCrmyWYegMw5t4MgKxz5ZK7EYPQUVjI1VoDXgTuQufmcYXERj1WmGQ0ByN0FGhZKKH3K2BMKI8wQzi6s2NWOsfZ0nj3e82ulNXKvQjG9pNcxwvULooBA87MWIP+cQpgXCR/hYfEDrdGhcJ/oHdJpols8KG89cuJw4FILxFnIx9w1QEvVJ4Tyn24bPj21maCxV29hKvjodNfHVXPXcr2/KunwIdIF5usJITuowDGr1vzxZ4uAHasqxPpWDh+YUV0+iGXwQz2L51LfmICEd/ZPFBwEJjqJTQnyJTV8+VnivdKVhXqdZLPfadhUOiBkAFvfOa/eSK0OzyuohH6rpVnMeVmvvYxBrbevZvMQ0+ouaq4Oh/Szx3yWHV4fBIyqu6ggPfZcozvAlLfVqqsDNb6TRu00D0us2FypRzG3Q2ydXMqlwL3fbWlnJ+tIdiFyr/4N4/hHAUaLsNwmFg/qITla04gyjNAh1BTGJ7Tkpw+ez37GVgAm/e1DXNu/TBlEbrIjkTtSmiCpIoJ01+F5TlC4D1VxTA6bj0+Rl95bgvIgzRafiVWG9ljlxg/VMpxVuygaIt3tPSvL AiCL1BnP H10231RE5XytSxJbSxQk3CT8LMR6aGcxQqLNE51Nsn2cg8cb72cPODNp3MhBXT8tEesXwMcPHUbD4KP+9uwYRdA72FgidIGCOFqTXOnUgPRxFzOrw1edoQWJAU/qTCC2nmPMl5EGgfNQ2hvMt1HQvYZEVxC9TVjP65tmOjW7f1BS/aLBNKiRnRwSvltvjcZ8lnppBlhlLMp+Bm0jprYkiC2oI3Kzm6RHYsoM/ClEagZbOCvQ8M8hyg5CNXJ5b+vmOmgqWZZx70xcyNDwbcZsdjUKY7zGhBnCByvOT1U4S0lkSLjeJkyt875eHpE1KlDC6bpUHIamlzBftfwj5S2evOpKyQI5QfB3J2PMCBFHmEqOdnJ6XU9vobCylopgMCa6vF5VdIskHlDZYgcvrM7Bn5TR4LEpAkerNk3CRyuLIZqdmeeAmwo2GQZ49kJqfIC1e3zjvB1/8lp/SQ6kSxyNSYLaSXeaUhBqkn/i888307rcOGMbVUF6SmvhYi+1uEdsRqL7hCYL9c/bhiZoaxG5cq4oPotdYK4LgRqt38QHLya1I8d0M9idmIRvZjomC3ryYIxwwlWUDvm7cYwzRIxI8zCjp4gjISh3VmzLplVgYdSGjRbkJiP5NIA2UMw== 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 Mon, May 13, 2024 at 10:47:25AM -0700, Deepak Gupta wrote: > On Fri, May 10, 2024 at 02:02:54PM -0700, Charlie Jenkins wrote: > > On Wed, Apr 03, 2024 at 04:34:58PM -0700, Deepak Gupta wrote: > > > `arch_calc_vm_prot_bits` is implemented on risc-v to return VM_READ | > > > VM_WRITE if PROT_WRITE is specified. Similarly `riscv_sys_mmap` is > > > updated to convert all incoming PROT_WRITE to (PROT_WRITE | PROT_READ). > > > This is to make sure that any existing apps using PROT_WRITE still work. > > > > > > Earlier `protection_map[VM_WRITE]` used to pick read-write PTE encodings. > > > Now `protection_map[VM_WRITE]` will always pick PAGE_SHADOWSTACK PTE > > > encodings for shadow stack. Above changes ensure that existing apps > > > continue to work because underneath kernel will be picking > > > `protection_map[VM_WRITE|VM_READ]` PTE encodings. > > > > > > Signed-off-by: Deepak Gupta > > > --- > > > arch/riscv/include/asm/mman.h | 24 ++++++++++++++++++++++++ > > > arch/riscv/include/asm/pgtable.h | 1 + > > > arch/riscv/kernel/sys_riscv.c | 11 +++++++++++ > > > arch/riscv/mm/init.c | 2 +- > > > mm/mmap.c | 1 + > > > 5 files changed, 38 insertions(+), 1 deletion(-) > > > create mode 100644 arch/riscv/include/asm/mman.h > > > > > > diff --git a/arch/riscv/include/asm/mman.h b/arch/riscv/include/asm/mman.h > > > new file mode 100644 > > > index 000000000000..ef9fedf32546 > > > --- /dev/null > > > +++ b/arch/riscv/include/asm/mman.h > > > @@ -0,0 +1,24 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +#ifndef __ASM_MMAN_H__ > > > +#define __ASM_MMAN_H__ > > > + > > > +#include > > > +#include > > > +#include > > > + > > > +static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot, > > > + unsigned long pkey __always_unused) > > > +{ > > > + unsigned long ret = 0; > > > + > > > + /* > > > + * If PROT_WRITE was specified, force it to VM_READ | VM_WRITE. > > > + * Only VM_WRITE means shadow stack. > > > + */ > > > + if (prot & PROT_WRITE) > > > + ret = (VM_READ | VM_WRITE); > > > + return ret; > > > +} > > > +#define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey) > > > + > > > +#endif /* ! __ASM_MMAN_H__ */ > > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h > > > index 6066822e7396..4d5983bc6766 100644 > > > --- a/arch/riscv/include/asm/pgtable.h > > > +++ b/arch/riscv/include/asm/pgtable.h > > > @@ -184,6 +184,7 @@ extern struct pt_alloc_ops pt_ops __initdata; > > > #define PAGE_READ_EXEC __pgprot(_PAGE_BASE | _PAGE_READ | _PAGE_EXEC) > > > #define PAGE_WRITE_EXEC __pgprot(_PAGE_BASE | _PAGE_READ | \ > > > _PAGE_EXEC | _PAGE_WRITE) > > > +#define PAGE_SHADOWSTACK __pgprot(_PAGE_BASE | _PAGE_WRITE) > > > > > > #define PAGE_COPY PAGE_READ > > > #define PAGE_COPY_EXEC PAGE_READ_EXEC > > > diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c > > > index f1c1416a9f1e..846c36b1b3d5 100644 > > > --- a/arch/riscv/kernel/sys_riscv.c > > > +++ b/arch/riscv/kernel/sys_riscv.c > > > @@ -8,6 +8,8 @@ > > > #include > > > #include > > > #include > > > +#include > > > +#include > > > > > > static long riscv_sys_mmap(unsigned long addr, unsigned long len, > > > unsigned long prot, unsigned long flags, > > > @@ -17,6 +19,15 @@ static long riscv_sys_mmap(unsigned long addr, unsigned long len, > > > if (unlikely(offset & (~PAGE_MASK >> page_shift_offset))) > > > return -EINVAL; > > > > > > + /* > > > + * If only PROT_WRITE is specified then extend that to PROT_READ > > > + * protection_map[VM_WRITE] is now going to select shadow stack encodings. > > > + * So specifying PROT_WRITE actually should select protection_map [VM_WRITE | VM_READ] > > > + * If user wants to create shadow stack then they should use `map_shadow_stack` syscall. > > > + */ > > > + if (unlikely((prot & PROT_WRITE) && !(prot & PROT_READ))) > > > > The comments says that this should extend to PROT_READ if only > > PROT_WRITE is specified. This condition instead is checking if > > PROT_WRITE is selected but PROT_READ is not. If prot is (VM_EXEC | > > VM_WRITE) then it would be extended to (VM_EXEC | VM_WRITE | VM_READ). > > This will not currently cause any issues because these both map to the > > same value in the protection_map PAGE_COPY_EXEC, however this seems to > > be not the intention of this change. > > > > prot == PROT_WRITE better suits the condition explained in the comment. > > If someone specifies this (PROT_EXEC | PROT_WRITE) today, it works because > of the way permissions are setup in `protection_map`. On risc-v there is no > way to have a page which is execute and write only. So expectation is that > if some apps were using `PROT_EXEC | PROT_WRITE` today, they were working > because internally it was translating to read, write and execute on page > permissions level. This patch make sure that, it stays same from page > permissions perspective. > > If someone was using PROT_EXEC, it may translate to execute only and this change > doesn't impact that. > > Patch simply looks for presence of `PROT_WRITE` and absence of `PROT_READ` in > protection flags and if that condition is satisfied, it assumes that caller assumed > page is going to be read allowed as well. The purpose of this change is for compatibility with shadow stack pages but this affects flags for pages that are not shadow stack pages. Adding PROT_READ to the other cases is redundant as protection_map already handles that mapping. Permissions being strictly PROT_WRITE is the only case that needs to be handled, and is the only case that is called out in the commit message and in the comment. - Charlie > > > > > > > + prot |= PROT_READ; > > > + > > > return ksys_mmap_pgoff(addr, len, prot, flags, fd, > > > offset >> (PAGE_SHIFT - page_shift_offset)); > > > } > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > > > index fa34cf55037b..98e5ece4052a 100644 > > > --- a/arch/riscv/mm/init.c > > > +++ b/arch/riscv/mm/init.c > > > @@ -299,7 +299,7 @@ pgd_t early_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE); > > > static const pgprot_t protection_map[16] = { > > > [VM_NONE] = PAGE_NONE, > > > [VM_READ] = PAGE_READ, > > > - [VM_WRITE] = PAGE_COPY, > > > + [VM_WRITE] = PAGE_SHADOWSTACK, > > > [VM_WRITE | VM_READ] = PAGE_COPY, > > > [VM_EXEC] = PAGE_EXEC, > > > [VM_EXEC | VM_READ] = PAGE_READ_EXEC, > > > diff --git a/mm/mmap.c b/mm/mmap.c > > > index d89770eaab6b..57a974f49b00 100644 > > > --- a/mm/mmap.c > > > +++ b/mm/mmap.c > > > @@ -47,6 +47,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > > It doesn't seem like this is necessary for this patch. > > Thanks. Yeah it looks like I forgot to remove this over the churn. > Will fix it. > > > > > - Charlie > > > > > > > > #include > > > #include > > > -- > > > 2.43.2 > > >