| 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: <88914aef-b73e-b8c1-1c06-5a424d8a8b57@huawei.com>
Date: Wed, 12 Apr 2023 10:31:52 +0800
From: Tong Tiangen <tongtiangen@...wei.com>
To: Catalin Marinas <catalin.marinas@....com>
CC: Mark Rutland <mark.rutland@....com>,
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>,
Will Deacon <will@...nel.org>,
Alexander Viro <viro@...iv.linux.org.uk>, <x86@...nel.org>,
"H . Peter Anvin" <hpa@...or.com>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
Kefeng Wang <wangkefeng.wang@...wei.com>,
Guohanjun <guohanjun@...wei.com>, Xie XiuQi <xiexiuqi@...wei.com>
Subject: Re: [PATCH -next v8 4/4] arm64: add cow to machine check safe
在 2023/4/12 0:45, Catalin Marinas 写道:
> On Mon, Dec 19, 2022 at 12:00:08PM +0000, Tong Tiangen wrote:
>> At present, Recover from poison consumption from copy-on-write has been
>> supported[1], arm64 should also support this mechanism.
>>
>> Add new helper copy_mc_page() which provide a page copy implementation with
>> machine check safe. At present, only used in cow. In the 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_mc_page() in copy_page_mc.S is largely borrows from copy_page()
>> in copy_page.S and the main difference is copy_mc_page() 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_MC_PAGE which used in copy_mc_page().
>>
>> [1]https://lore.kernel.org/lkml/20221031201029.102123-1-tony.luck@intel.com/
>>
>> Signed-off-by: Tong Tiangen <tongtiangen@...wei.com>
>
> This series needs rebasing onto a newer kernel. Some random comments
> below.
OK, very willing to do it :)
>
>> diff --git a/arch/arm64/lib/copy_mc_page.S b/arch/arm64/lib/copy_mc_page.S
>> new file mode 100644
>> index 000000000000..03d657a182f6
>> --- /dev/null
>> +++ b/arch/arm64/lib/copy_mc_page.S
>> @@ -0,0 +1,82 @@
> [...]
>> +SYM_FUNC_START(__pi_copy_mc_page)
>> +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])
> [...]
> [...]
>> +9998: ret
>
> What I don't understand, is there any error returned here or the bytes
> not copied? I can see its return value is never used in this series.
>
> Also, do we need to distinguish between fault on the source or the
> destination?
Oh, missing it, This should rerun bytes not copied.
will be fixed next version.
>
>> diff --git a/arch/arm64/lib/mte.S b/arch/arm64/lib/mte.S
>> index 5018ac03b6bf..bf4dd861c41c 100644
>> --- a/arch/arm64/lib/mte.S
>> +++ b/arch/arm64/lib/mte.S
>> @@ -80,6 +80,25 @@ SYM_FUNC_START(mte_copy_page_tags)
>> ret
>> SYM_FUNC_END(mte_copy_page_tags)
>>
>> +/*
>> + * Copy the tags from the source page to the destination one wiht machine check safe
>> + * x0 - address of the destination page
>> + * x1 - address of the source page
>> + */
>> +SYM_FUNC_START(mte_copy_mc_page_tags)
>> + mov x2, x0
>> + mov x3, x1
>> + multitag_transfer_size x5, x6
>> +1:
>> +CPY_MC(2f, ldgm x4, [x3])
>> + stgm x4, [x2]
>> + add x2, x2, x5
>> + add x3, x3, x5
>> + tst x2, #(PAGE_SIZE - 1)
>> + b.ne 1b
>> +2: ret
>> +SYM_FUNC_END(mte_copy_mc_page_tags)
>
> While the data copy above handles errors on both source and destination,
> here you skip the destination. Any reason?
Oh, my fault, miss the destination.
>
>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>> index 8dd5a8fe64b4..005ee2a3cb4e 100644
>> --- a/arch/arm64/mm/copypage.c
>> +++ b/arch/arm64/mm/copypage.c
> [...]
>> +#ifdef CONFIG_ARCH_HAS_COPY_MC
>> +void copy_mc_highpage(struct page *to, struct page *from)
>> +{
>> + void *kto = page_address(to);
>> + void *kfrom = page_address(from);
>> +
>> + copy_mc_page(kto, kfrom);
>> + do_mte(to, from, kto, kfrom, true);
>> +}
>> +EXPORT_SYMBOL(copy_mc_highpage);
>> +
>> +int copy_mc_user_highpage(struct page *to, struct page *from,
>> + unsigned long vaddr, struct vm_area_struct *vma)
>> +{
>> + copy_mc_highpage(to, from);
>> + flush_dcache_page(to);
>> + return 0;
>> +}
>
> This one always returns 0. Does it actually catch any memory failures?
Yes, will be fixed next version.
>
>> +EXPORT_SYMBOL_GPL(copy_mc_user_highpage);
>> +#endif
>> diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
>> index 28ec35e3d210..0fdab18f2f07 100644
>> --- a/arch/arm64/mm/extable.c
>> +++ b/arch/arm64/mm/extable.c
>> @@ -16,6 +16,13 @@ get_ex_fixup(const struct exception_table_entry *ex)
>> return ((unsigned long)&ex->fixup + ex->fixup);
>> }
>>
>> +static bool ex_handler_fixup(const struct exception_table_entry *ex,
>> + struct pt_regs *regs)
>> +{
>> + regs->pc = get_ex_fixup(ex);
>> + return true;
>> +}
>
> Should we prepare some error here like -EFAULT? That's done in
> ex_handler_uaccess_err_zero().
Yes, it should be done here and will be fixed next version.
Thank you for these great suggestions.
Tong.
>
Powered by blists - more mailing lists