| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <151148fe-cd9d-eb81-6aeb-3aae1691b3dd@huawei.com>
Date: Thu, 19 May 2022 18:38:42 +0800
From: Tong Tiangen <tongtiangen@...wei.com>
To: Mark Rutland <mark.rutland@....com>
CC: James Morse <james.morse@....com>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
"Ingo Molnar" <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Robin Murphy <robin.murphy@....com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"Catalin Marinas" <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
"Alexander Viro" <viro@...iv.linux.org.uk>,
Michael Ellerman <mpe@...erman.id.au>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Paul Mackerras <paulus@...ba.org>, <x86@...nel.org>,
"H . Peter Anvin" <hpa@...or.com>, <linuxppc-dev@...ts.ozlabs.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
Kefeng Wang <wangkefeng.wang@...wei.com>,
Xie XiuQi <xiexiuqi@...wei.com>,
Guohanjun <guohanjun@...wei.com>
Subject: Re: [PATCH -next v4 7/7] arm64: add cow to machine check safe
在 2022/5/13 23:44, Mark Rutland 写道:
> On Wed, Apr 20, 2022 at 03:04:18AM +0000, Tong Tiangen wrote:
>> In the cow(copy on write) processing, the data of the user process is
>> copied, when hardware memory error is encountered during copy, only the
>> relevant processes are affected, so killing the user process and isolate
>> the user page with hardware memory errors is a more reasonable choice than
>> kernel panic.
>
> There are plenty of other places we'll access user pages via a kernel
> alias (e.g. when performing IO), so why is this special?
>
> To be clear, I am not entirely averse to this, but it seems like this is
> being done because it's easy to do rather than necessarily being all
> that useful, and I'm not keen on having to duplicate a bunch of logic
> for this.
Yeah, There are lots of cases, COW is selected because it is more
general. In addition, this provides the machine check safe capability of
page copy(copy_highpage_mc), valuable cases can be based on this step by
step[1].
[1]https://lore.kernel.org/all/20220429000947.2172219-1-jiaqiyan@google.com/T/
Thanks,
Tong.
>
>> Add new helper copy_page_mc() which provide a page copy implementation with
>> machine check safe. At present, only used in cow. In future, we can expand
>> more scenes. As long as the consequences of page copy failure are not
>> fatal(eg: only affect user process), we can use this helper.
>>
>> The copy_page_mc() in copy_page_mc.S is largely borrows from copy_page()
>> in copy_page.S and the main difference is copy_page_mc() add extable entry
>> to every load/store insn to support machine check safe. largely to keep the
>> patch simple. If needed those optimizations can be folded in.
>>
>> Add new extable type EX_TYPE_COPY_PAGE_MC which used in copy_page_mc().
>>
>> This type only be processed in fixup_exception_mc(), The reason is that
>> copy_page_mc() is consistent with copy_page() except machine check safe is
>> considered, and copy_page() do not need to consider exception fixup.
>>
>> Signed-off-by: Tong Tiangen <tongtiangen@...wei.com>
>> ---
>> arch/arm64/include/asm/asm-extable.h | 5 ++
>> arch/arm64/include/asm/page.h | 10 ++++
>> arch/arm64/lib/Makefile | 2 +
>> arch/arm64/lib/copy_page_mc.S | 86 ++++++++++++++++++++++++++++
>> arch/arm64/mm/copypage.c | 36 ++++++++++--
>> arch/arm64/mm/extable.c | 2 +
>> include/linux/highmem.h | 8 +++
>> mm/memory.c | 2 +-
>> 8 files changed, 144 insertions(+), 7 deletions(-)
>> create mode 100644 arch/arm64/lib/copy_page_mc.S
>>
>> diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
>> index 80410899a9ad..74c056ddae15 100644
>> --- a/arch/arm64/include/asm/asm-extable.h
>> +++ b/arch/arm64/include/asm/asm-extable.h
>> @@ -14,6 +14,7 @@
>> /* _MC indicates that can fixup from machine check errors */
>> #define EX_TYPE_UACCESS_MC 5
>> #define EX_TYPE_UACCESS_MC_ERR_ZERO 6
>> +#define EX_TYPE_COPY_PAGE_MC 7
>>
>> #ifdef __ASSEMBLY__
>>
>> @@ -42,6 +43,10 @@
>> __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_MC, 0)
>> .endm
>>
>> + .macro _asm_extable_copy_page_mc, insn, fixup
>> + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_COPY_PAGE_MC, 0)
>> + .endm
>> +
>> /*
>> * Create an exception table entry for `insn` if `fixup` is provided. Otherwise
>> * do nothing.
>> diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
>> index 993a27ea6f54..832571a7dddb 100644
>> --- a/arch/arm64/include/asm/page.h
>> +++ b/arch/arm64/include/asm/page.h
>> @@ -29,6 +29,16 @@ void copy_user_highpage(struct page *to, struct page *from,
>> void copy_highpage(struct page *to, struct page *from);
>> #define __HAVE_ARCH_COPY_HIGHPAGE
>>
>> +#ifdef CONFIG_ARCH_HAS_COPY_MC
>> +extern void copy_page_mc(void *to, const void *from);
>> +void copy_highpage_mc(struct page *to, struct page *from);
>> +#define __HAVE_ARCH_COPY_HIGHPAGE_MC
>> +
>> +void copy_user_highpage_mc(struct page *to, struct page *from,
>> + unsigned long vaddr, struct vm_area_struct *vma);
>> +#define __HAVE_ARCH_COPY_USER_HIGHPAGE_MC
>> +#endif
>> +
>> struct page *alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,
>> unsigned long vaddr);
>> #define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE
>> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
>> index 29490be2546b..0d9f292ef68a 100644
>> --- a/arch/arm64/lib/Makefile
>> +++ b/arch/arm64/lib/Makefile
>> @@ -15,6 +15,8 @@ endif
>>
>> lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o
>>
>> +lib-$(CONFIG_ARCH_HAS_COPY_MC) += copy_page_mc.o
>> +
>> obj-$(CONFIG_CRC32) += crc32.o
>>
>> obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
>> diff --git a/arch/arm64/lib/copy_page_mc.S b/arch/arm64/lib/copy_page_mc.S
>> new file mode 100644
>> index 000000000000..655161363dcf
>> --- /dev/null
>> +++ b/arch/arm64/lib/copy_page_mc.S
>> @@ -0,0 +1,86 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2012 ARM Ltd.
>> + */
>> +
>> +#include <linux/linkage.h>
>> +#include <linux/const.h>
>> +#include <asm/assembler.h>
>> +#include <asm/page.h>
>> +#include <asm/cpufeature.h>
>> +#include <asm/alternative.h>
>> +#include <asm/asm-extable.h>
>> +
>> +#define CPY_MC(l, x...) \
>> +9999: x; \
>> + _asm_extable_copy_page_mc 9999b, l
>> +
>> +/*
>> + * Copy a page from src to dest (both are page aligned) with machine check
>> + *
>> + * Parameters:
>> + * x0 - dest
>> + * x1 - src
>> + */
>> +SYM_FUNC_START(__pi_copy_page_mc)
>> +alternative_if ARM64_HAS_NO_HW_PREFETCH
>> + // Prefetch three cache lines ahead.
>> + prfm pldl1strm, [x1, #128]
>> + prfm pldl1strm, [x1, #256]
>> + prfm pldl1strm, [x1, #384]
>> +alternative_else_nop_endif
>> +
>> +CPY_MC(9998f, ldp x2, x3, [x1])
>> +CPY_MC(9998f, ldp x4, x5, [x1, #16])
>> +CPY_MC(9998f, ldp x6, x7, [x1, #32])
>> +CPY_MC(9998f, ldp x8, x9, [x1, #48])
>> +CPY_MC(9998f, ldp x10, x11, [x1, #64])
>> +CPY_MC(9998f, ldp x12, x13, [x1, #80])
>> +CPY_MC(9998f, ldp x14, x15, [x1, #96])
>> +CPY_MC(9998f, ldp x16, x17, [x1, #112])
>> +
>> + add x0, x0, #256
>> + add x1, x1, #128
>> +1:
>> + tst x0, #(PAGE_SIZE - 1)
>> +
>> +alternative_if ARM64_HAS_NO_HW_PREFETCH
>> + prfm pldl1strm, [x1, #384]
>> +alternative_else_nop_endif
>> +
>> +CPY_MC(9998f, stnp x2, x3, [x0, #-256])
>> +CPY_MC(9998f, ldp x2, x3, [x1])
>> +CPY_MC(9998f, stnp x4, x5, [x0, #16 - 256])
>> +CPY_MC(9998f, ldp x4, x5, [x1, #16])
>> +CPY_MC(9998f, stnp x6, x7, [x0, #32 - 256])
>> +CPY_MC(9998f, ldp x6, x7, [x1, #32])
>> +CPY_MC(9998f, stnp x8, x9, [x0, #48 - 256])
>> +CPY_MC(9998f, ldp x8, x9, [x1, #48])
>> +CPY_MC(9998f, stnp x10, x11, [x0, #64 - 256])
>> +CPY_MC(9998f, ldp x10, x11, [x1, #64])
>> +CPY_MC(9998f, stnp x12, x13, [x0, #80 - 256])
>> +CPY_MC(9998f, ldp x12, x13, [x1, #80])
>> +CPY_MC(9998f, stnp x14, x15, [x0, #96 - 256])
>> +CPY_MC(9998f, ldp x14, x15, [x1, #96])
>> +CPY_MC(9998f, stnp x16, x17, [x0, #112 - 256])
>> +CPY_MC(9998f, ldp x16, x17, [x1, #112])
>> +
>> + add x0, x0, #128
>> + add x1, x1, #128
>> +
>> + b.ne 1b
>> +
>> +CPY_MC(9998f, stnp x2, x3, [x0, #-256])
>> +CPY_MC(9998f, stnp x4, x5, [x0, #16 - 256])
>> +CPY_MC(9998f, stnp x6, x7, [x0, #32 - 256])
>> +CPY_MC(9998f, stnp x8, x9, [x0, #48 - 256])
>> +CPY_MC(9998f, stnp x10, x11, [x0, #64 - 256])
>> +CPY_MC(9998f, stnp x12, x13, [x0, #80 - 256])
>> +CPY_MC(9998f, stnp x14, x15, [x0, #96 - 256])
>> +CPY_MC(9998f, stnp x16, x17, [x0, #112 - 256])
>> +
>> +9998: ret
>> +
>> +SYM_FUNC_END(__pi_copy_page_mc)
>> +SYM_FUNC_ALIAS(copy_page_mc, __pi_copy_page_mc)
>> +EXPORT_SYMBOL(copy_page_mc)
>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>> index 0dea80bf6de4..0f28edfcb234 100644
>> --- a/arch/arm64/mm/copypage.c
>> +++ b/arch/arm64/mm/copypage.c
>> @@ -14,13 +14,8 @@
>> #include <asm/cpufeature.h>
>> #include <asm/mte.h>
>>
>> -void copy_highpage(struct page *to, struct page *from)
>> +static void do_mte(struct page *to, struct page *from, void *kto, void *kfrom)
>> {
>> - void *kto = page_address(to);
>> - void *kfrom = page_address(from);
>> -
>> - copy_page(kto, kfrom);
>> -
>> if (system_supports_mte() && test_bit(PG_mte_tagged, &from->flags)) {
>> set_bit(PG_mte_tagged, &to->flags);
>> page_kasan_tag_reset(to);
>> @@ -35,6 +30,15 @@ void copy_highpage(struct page *to, struct page *from)
>> mte_copy_page_tags(kto, kfrom);
>> }
>> }
>> +
>> +void copy_highpage(struct page *to, struct page *from)
>> +{
>> + void *kto = page_address(to);
>> + void *kfrom = page_address(from);
>> +
>> + copy_page(kto, kfrom);
>> + do_mte(to, from, kto, kfrom);
>> +}
>> EXPORT_SYMBOL(copy_highpage);
>>
>> void copy_user_highpage(struct page *to, struct page *from,
>> @@ -44,3 +48,23 @@ void copy_user_highpage(struct page *to, struct page *from,
>> flush_dcache_page(to);
>> }
>> EXPORT_SYMBOL_GPL(copy_user_highpage);
>> +
>> +#ifdef CONFIG_ARCH_HAS_COPY_MC
>> +void copy_highpage_mc(struct page *to, struct page *from)
>> +{
>> + void *kto = page_address(to);
>> + void *kfrom = page_address(from);
>> +
>> + copy_page_mc(kto, kfrom);
>> + do_mte(to, from, kto, kfrom);
>> +}
>> +EXPORT_SYMBOL(copy_highpage_mc);
>
> IIUC the do_mte() portion won't handle mermoy errors, so this isn't
> actually going to recover safely.
>
> Thanks,
> Mark.
OK, Missing that, do_mte needs to be handled.
Thanks,
Tong.
>
>> +
>> +void copy_user_highpage_mc(struct page *to, struct page *from,
>> + unsigned long vaddr, struct vm_area_struct *vma)
>> +{
>> + copy_highpage_mc(to, from);
>> + flush_dcache_page(to);
>> +}
>> +EXPORT_SYMBOL_GPL(copy_user_highpage_mc);
>> +#endif
>> diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
>> index 1023ccdb2f89..4c882d36dd64 100644
>> --- a/arch/arm64/mm/extable.c
>> +++ b/arch/arm64/mm/extable.c
>> @@ -110,6 +110,8 @@ bool fixup_exception_mc(struct pt_regs *regs)
>> return ex_handler_uaccess_type(ex, regs, FIXUP_TYPE_MC);
>> case EX_TYPE_UACCESS_MC_ERR_ZERO:
>> return ex_handler_uaccess_err_zero(ex, regs);
>> + case EX_TYPE_COPY_PAGE_MC:
>> + return ex_handler_fixup(ex, regs);
>>
>> }
>>
>> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
>> index 39bb9b47fa9c..a9dbf331b038 100644
>> --- a/include/linux/highmem.h
>> +++ b/include/linux/highmem.h
>> @@ -283,6 +283,10 @@ static inline void copy_user_highpage(struct page *to, struct page *from,
>>
>> #endif
>>
>> +#ifndef __HAVE_ARCH_COPY_USER_HIGHPAGE_MC
>> +#define copy_user_highpage_mc copy_user_highpage
>> +#endif
>> +
>> #ifndef __HAVE_ARCH_COPY_HIGHPAGE
>>
>> static inline void copy_highpage(struct page *to, struct page *from)
>> @@ -298,6 +302,10 @@ static inline void copy_highpage(struct page *to, struct page *from)
>>
>> #endif
>>
>> +#ifndef __HAVE_ARCH_COPY_HIGHPAGE_MC
>> +#define cop_highpage_mc copy_highpage
>> +#endif
>> +
>> static inline void memcpy_page(struct page *dst_page, size_t dst_off,
>> struct page *src_page, size_t src_off,
>> size_t len)
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 76e3af9639d9..d5f62234152d 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -2767,7 +2767,7 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
>> unsigned long addr = vmf->address;
>>
>> if (likely(src)) {
>> - copy_user_highpage(dst, src, addr, vma);
>> + copy_user_highpage_mc(dst, src, addr, vma);
>> return true;
>> }
>>
>> --
>> 2.25.1
>>
> .
Powered by blists - more mailing lists