From: Borislav Petkov <bp@alien8.de>
To: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-mm@kvack.org, linux-arch@vger.kernel.org,
linux-api@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
Andy Lutomirski <luto@kernel.org>,
Balbir Singh <bsingharora@gmail.com>,
Cyrill Gorcunov <gorcunov@gmail.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
Eugene Syromiatnikov <esyr@redhat.com>,
Florian Weimer <fweimer@redhat.com>,
"H.J. Lu" <hjl.tools@gmail.com>, Jann Horn <jannh@google.com>,
Jonathan Corbet <corbet@lwn.net>,
Kees Cook <keescook@chromium.org>,
Mike Kravetz <mike.kravetz@oracle.com>,
Nadav Amit <nadav.amit@gmail.com>,
Oleg Nesterov <oleg@redhat.com>, Pavel Machek <pavel@ucw.cz>,
Peter Zijlstra <peterz@infradead.org>,
Randy Dunlap <rdunlap@infradead.org>,
"Ravi V. Shankar" <ravi.v.shankar@intel.com>,
Vedvyas Shanbhogue <vedvyas.shanbhogue@intel.com>,
Dave Martin <Dave.Martin@arm.com>,
Weijiang Yang <weijiang.yang@intel.com>,
Pengfei Xu <pengfei.xu@intel.com>
Subject: Re: [PATCH v17 11/26] x86/mm: Update ptep_set_wrprotect() and pmdp_set_wrprotect() for transition from _PAGE_DIRTY to _PAGE_COW
Date: Mon, 25 Jan 2021 19:27:09 +0100 [thread overview]
Message-ID: <20210125182709.GC23290@zn.tnic> (raw)
In-Reply-To: <20201229213053.16395-12-yu-cheng.yu@intel.com>
On Tue, Dec 29, 2020 at 01:30:38PM -0800, Yu-cheng Yu wrote:
> When Shadow Stack is introduced, [R/O + _PAGE_DIRTY] PTE is reserved for
> shadow stack. Copy-on-write PTEs have [R/O + _PAGE_COW].
>
> When a PTE goes from [R/W + _PAGE_DIRTY] to [R/O + _PAGE_COW], it could
> become a transient shadow stack PTE in two cases:
>
> The first case is that some processors can start a write but end up seeing
> a read-only PTE by the time they get to the Dirty bit, creating a transient
> shadow stack PTE. However, this will not occur on processors supporting
> Shadow Stack, therefore we don't need a TLB flush here.
Who's "we"?
> The second case is that when the software, without atomic, tests & replaces
"... when _PAGE_DIRTY is replaced with _PAGE_COW non-atomically, a transient
shadow stack PTE can be created, as a result."
> _PAGE_DIRTY with _PAGE_COW, a transient shadow stack PTE can exist.
> This is prevented with cmpxchg.
>
> Dave Hansen, Jann Horn, Andy Lutomirski, and Peter Zijlstra provided many
> insights to the issue. Jann Horn provided the cmpxchg solution.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> ---
> arch/x86/include/asm/pgtable.h | 52 ++++++++++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
>
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 666c25ab9564..1c84f1ba32b9 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -1226,6 +1226,32 @@ static inline pte_t ptep_get_and_clear_full(struct mm_struct *mm,
> static inline void ptep_set_wrprotect(struct mm_struct *mm,
> unsigned long addr, pte_t *ptep)
> {
> + /*
> + * Some processors can start a write, but end up seeing a read-only
> + * PTE by the time they get to the Dirty bit. In this case, they
> + * will set the Dirty bit, leaving a read-only, Dirty PTE which
> + * looks like a shadow stack PTE.
> + *
> + * However, this behavior has been improved
Improved how?
> and will not occur on
> + * processors supporting Shadow Stack. Without this guarantee, a
Which guarantee? That it won't happen on CPUs which support SHSTK?
> + * transition to a non-present PTE and flush the TLB would be
s/flush the TLB/TLB flush/
> + * needed.
> + *
> + * When changing a writable PTE to read-only and if the PTE has
> + * _PAGE_DIRTY set, move that bit to _PAGE_COW so that the PTE is
> + * not a shadow stack PTE.
> + */
This sentence doesn't belong here as it refers to what pte_wrprotect()
does. You could expand the comment in pte_wrprotect() with this here as
it is better.
> + if (cpu_feature_enabled(X86_FEATURE_SHSTK)) {
> + pte_t old_pte, new_pte;
> +
> + do {
> + old_pte = READ_ONCE(*ptep);
> + new_pte = pte_wrprotect(old_pte);
Maybe I'm missing something but those two can happen outside of the
loop, no? Or is *ptep somehow changing concurrently while the loop is
doing the CMPXCHG and you need to recreate it each time?
IOW, you can generate upfront and do the empty loop...
> +
> + } while (!try_cmpxchg(&ptep->pte, &old_pte.pte, new_pte.pte));
> +
> + return;
> + }
> clear_bit(_PAGE_BIT_RW, (unsigned long *)&ptep->pte);
> }
>
> @@ -1282,6 +1308,32 @@ static inline pud_t pudp_huge_get_and_clear(struct mm_struct *mm,
> static inline void pmdp_set_wrprotect(struct mm_struct *mm,
> unsigned long addr, pmd_t *pmdp)
> {
> + /*
> + * Some processors can start a write, but end up seeing a read-only
> + * PMD by the time they get to the Dirty bit. In this case, they
> + * will set the Dirty bit, leaving a read-only, Dirty PMD which
> + * looks like a Shadow Stack PMD.
> + *
> + * However, this behavior has been improved and will not occur on
> + * processors supporting Shadow Stack. Without this guarantee, a
> + * transition to a non-present PMD and flush the TLB would be
> + * needed.
> + *
> + * When changing a writable PMD to read-only and if the PMD has
> + * _PAGE_DIRTY set, move that bit to _PAGE_COW so that the PMD is
> + * not a shadow stack PMD.
> + */
Same comments as above.
> + if (cpu_feature_enabled(X86_FEATURE_SHSTK)) {
> + pmd_t old_pmd, new_pmd;
> +
> + do {
> + old_pmd = READ_ONCE(*pmdp);
> + new_pmd = pmd_wrprotect(old_pmd);
> +
> + } while (!try_cmpxchg((pmdval_t *)pmdp, (pmdval_t *)&old_pmd, pmd_val(new_pmd)));
> +
> + return;
> + }
> clear_bit(_PAGE_BIT_RW, (unsigned long *)pmdp);
> }
>
> --
> 2.21.0
>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
next prev parent reply other threads:[~2021-01-25 18:28 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-29 21:30 [PATCH v17 00/26] Control-flow Enforcement: Shadow Stack Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 01/26] Documentation/x86: Add CET description Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 02/26] x86/cet/shstk: Add Kconfig option for user-mode control-flow protection Yu-cheng Yu
2021-01-19 11:06 ` Borislav Petkov
2021-01-19 18:10 ` Yu, Yu-cheng
2020-12-29 21:30 ` [PATCH v17 03/26] x86/cpufeatures: Add CET CPU feature flags for Control-flow Enforcement Technology (CET) Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 04/26] x86/cpufeatures: Introduce X86_FEATURE_CET and setup functions Yu-cheng Yu
2021-01-11 17:56 ` Borislav Petkov
2021-01-11 20:25 ` Yu, Yu-cheng
2021-01-11 23:09 ` Yu-cheng Yu
2021-01-12 0:09 ` Yu, Yu-cheng
2021-01-12 12:38 ` Borislav Petkov
2021-01-12 23:02 ` Yu, Yu-cheng
2021-01-13 10:04 ` Borislav Petkov
2020-12-29 21:30 ` [PATCH v17 05/26] x86/fpu/xstate: Introduce CET MSR and XSAVES supervisor states Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 06/26] x86/cet: Add control-protection fault handler Yu-cheng Yu
2021-01-19 12:04 ` Borislav Petkov
2021-01-19 19:36 ` Yu, Yu-cheng
2020-12-29 21:30 ` [PATCH v17 07/26] x86/mm: Remove _PAGE_DIRTY from kernel RO pages Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 08/26] x86/mm: Introduce _PAGE_COW Yu-cheng Yu
2021-01-21 18:44 ` Borislav Petkov
2021-01-21 20:16 ` Yu, Yu-cheng
2021-01-21 20:20 ` Dave Hansen
2021-01-21 20:26 ` Dave Hansen
2021-01-21 20:44 ` Yu, Yu-cheng
2021-01-21 20:41 ` Borislav Petkov
2021-01-21 21:40 ` Yu, Yu-cheng
2021-01-21 22:16 ` David Laight
2021-01-21 22:19 ` Randy Dunlap
2021-01-21 22:32 ` David Laight
2021-01-22 21:54 ` Yu, Yu-cheng
2020-12-29 21:30 ` [PATCH v17 09/26] drm/i915/gvt: Change _PAGE_DIRTY to _PAGE_DIRTY_BITS Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 10/26] x86/mm: Update pte_modify for _PAGE_COW Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 11/26] x86/mm: Update ptep_set_wrprotect() and pmdp_set_wrprotect() for transition from _PAGE_DIRTY to _PAGE_COW Yu-cheng Yu
2021-01-25 18:27 ` Borislav Petkov [this message]
2021-01-25 21:27 ` Yu, Yu-cheng
2021-01-25 21:55 ` Borislav Petkov
2021-01-25 22:18 ` Yu, Yu-cheng
2021-01-26 10:24 ` Borislav Petkov
2021-01-26 16:45 ` Yu, Yu-cheng
2021-01-26 8:46 ` Peter Zijlstra
2021-01-26 9:40 ` Peter Zijlstra
2021-01-26 16:43 ` Yu, Yu-cheng
2020-12-29 21:30 ` [PATCH v17 12/26] mm: Introduce VM_SHSTK for shadow stack memory Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 13/26] x86/mm: Shadow Stack page fault error checking Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 14/26] x86/mm: Update maybe_mkwrite() for shadow stack Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 15/26] mm: Fixup places that call pte_mkwrite() directly Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 16/26] mm: Add guard pages around a shadow stack Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 17/26] mm/mmap: Add shadow stack pages to memory accounting Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 18/26] mm: Update can_follow_write_pte() for shadow stack Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 19/26] mm: Re-introduce vm_flags to do_mmap() Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 20/26] x86/cet/shstk: User-mode shadow stack support Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 21/26] x86/cet/shstk: Handle signals for shadow stack Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 22/26] binfmt_elf: Define GNU_PROPERTY_X86_FEATURE_1_AND properties Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 23/26] ELF: Introduce arch_setup_elf_property() Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 24/26] x86/cet/shstk: Handle thread shadow stack Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 25/26] x86/cet/shstk: Add arch_prctl functions for " Yu-cheng Yu
2020-12-29 21:30 ` [PATCH v17 26/26] mm: Introduce PROT_SHSTK " Yu-cheng Yu
2021-01-04 20:08 ` [PATCH v17 00/26] Control-flow Enforcement: Shadow Stack Yu, Yu-cheng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210125182709.GC23290@zn.tnic \
--to=bp@alien8.de \
--cc=Dave.Martin@arm.com \
--cc=arnd@arndb.de \
--cc=bsingharora@gmail.com \
--cc=corbet@lwn.net \
--cc=dave.hansen@linux.intel.com \
--cc=esyr@redhat.com \
--cc=fweimer@redhat.com \
--cc=gorcunov@gmail.com \
--cc=hjl.tools@gmail.com \
--cc=hpa@zytor.com \
--cc=jannh@google.com \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@kernel.org \
--cc=mike.kravetz@oracle.com \
--cc=mingo@redhat.com \
--cc=nadav.amit@gmail.com \
--cc=oleg@redhat.com \
--cc=pavel@ucw.cz \
--cc=pengfei.xu@intel.com \
--cc=peterz@infradead.org \
--cc=ravi.v.shankar@intel.com \
--cc=rdunlap@infradead.org \
--cc=tglx@linutronix.de \
--cc=vedvyas.shanbhogue@intel.com \
--cc=weijiang.yang@intel.com \
--cc=x86@kernel.org \
--cc=yu-cheng.yu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).