| Message ID | 20220420030418.3189040-5-tongtiangen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: add machine check safe support | expand |
On Wed, Apr 20, 2022 at 03:04:15AM +0000, Tong Tiangen wrote: > Add copy_{to, from}_user() to machine check safe. > > If copy fail due to hardware memory error, 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. Just to make sure I understand - we can only recover if the fault is in a user page. That is, for a copy_from_user(), we can only handle the faults in the source address, not the destination. > diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S > index 34e317907524..480cc5ac0a8d 100644 > --- a/arch/arm64/lib/copy_from_user.S > +++ b/arch/arm64/lib/copy_from_user.S > @@ -25,7 +25,7 @@ > .endm > > .macro strb1 reg, ptr, val > - strb \reg, [\ptr], \val > + USER_MC(9998f, strb \reg, [\ptr], \val) > .endm So if I got the above correctly, why do we need an exception table entry for the store to the kernel address?
在 2022/5/4 18:26, Catalin Marinas 写道: > On Wed, Apr 20, 2022 at 03:04:15AM +0000, Tong Tiangen wrote: >> Add copy_{to, from}_user() to machine check safe. >> >> If copy fail due to hardware memory error, 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. > > Just to make sure I understand - we can only recover if the fault is in > a user page. That is, for a copy_from_user(), we can only handle the > faults in the source address, not the destination. At the beginning, I also thought we can only recover if the fault is in a user page. After discussion with a Mark[1], I think no matter user page or kernel page, as long as it is triggered by the user process, only related processes will be affected. According to this understanding, it seems that all uaccess can be recovered. [1]https://patchwork.kernel.org/project/linux-arm-kernel/patch/20220406091311.3354723-6-tongtiangen@huawei.com/ Thanks, Tong. > >> diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S >> index 34e317907524..480cc5ac0a8d 100644 >> --- a/arch/arm64/lib/copy_from_user.S >> +++ b/arch/arm64/lib/copy_from_user.S >> @@ -25,7 +25,7 @@ >> .endm >> >> .macro strb1 reg, ptr, val >> - strb \reg, [\ptr], \val >> + USER_MC(9998f, strb \reg, [\ptr], \val) >> .endm > > So if I got the above correctly, why do we need an exception table entry > for the store to the kernel address? >
On Thu, May 05, 2022 at 02:39:43PM +0800, Tong Tiangen wrote: > 在 2022/5/4 18:26, Catalin Marinas 写道: > > On Wed, Apr 20, 2022 at 03:04:15AM +0000, Tong Tiangen wrote: > > > Add copy_{to, from}_user() to machine check safe. > > > > > > If copy fail due to hardware memory error, 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. > > > > Just to make sure I understand - we can only recover if the fault is in > > a user page. That is, for a copy_from_user(), we can only handle the > > faults in the source address, not the destination. > > At the beginning, I also thought we can only recover if the fault is in a > user page. > After discussion with a Mark[1], I think no matter user page or kernel page, > as long as it is triggered by the user process, only related processes will > be affected. According to this > understanding, it seems that all uaccess can be recovered. > > [1]https://patchwork.kernel.org/project/linux-arm-kernel/patch/20220406091311.3354723-6-tongtiangen@huawei.com/ We can indeed safely skip this copy and return an error just like pretending there was a user page fault. However, my point was more around the "isolate the user page with hardware memory errors". If the fault is on a kernel address, there's not much you can do about. You'll likely trigger it later when you try to access that address (maybe it was freed and re-allocated). Do we hope we won't get the same error again on that kernel address?
在 2022/5/5 21:41, Catalin Marinas 写道: > On Thu, May 05, 2022 at 02:39:43PM +0800, Tong Tiangen wrote: >> 在 2022/5/4 18:26, Catalin Marinas 写道: >>> On Wed, Apr 20, 2022 at 03:04:15AM +0000, Tong Tiangen wrote: >>>> Add copy_{to, from}_user() to machine check safe. >>>> >>>> If copy fail due to hardware memory error, 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. >>> >>> Just to make sure I understand - we can only recover if the fault is in >>> a user page. That is, for a copy_from_user(), we can only handle the >>> faults in the source address, not the destination. >> >> At the beginning, I also thought we can only recover if the fault is in a >> user page. >> After discussion with a Mark[1], I think no matter user page or kernel page, >> as long as it is triggered by the user process, only related processes will >> be affected. According to this >> understanding, it seems that all uaccess can be recovered. >> >> [1]https://patchwork.kernel.org/project/linux-arm-kernel/patch/20220406091311.3354723-6-tongtiangen@huawei.com/ > > We can indeed safely skip this copy and return an error just like > pretending there was a user page fault. However, my point was more > around the "isolate the user page with hardware memory errors". If the > fault is on a kernel address, there's not much you can do about. You'll > likely trigger it later when you try to access that address (maybe it > was freed and re-allocated). Do we hope we won't get the same error > again on that kernel address? I think the page with memory error will be isolated by memory_failure(), generally, isolation will succeed, if isolate failure(we need to find out why), then maybe the same error will trigger it later. Thanks. >
On Wed, Apr 20, 2022 at 03:04:15AM +0000, Tong Tiangen wrote: > Add copy_{to, from}_user() to machine check safe. > > If copy fail due to hardware memory error, 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. > > Add new extable type EX_TYPE_UACCESS_MC which can be used for uaccess that > can be recovered from hardware memory errors. I don't understand why we need this. If we apply EX_TYPE_UACCESS consistently to *all* user accesses, and *only* to user accesses, that would *always* indicate that we can recover, and that seems much simpler to deal with. Today we use EX_TYPE_UACCESS_ERR_ZERO for kernel accesses in a couple of cases, which we should clean up, and we user EX_TYPE_FIXUP for a couple of user accesses, but those could easily be converted over. > The x16 register is used to save the fixup type in copy_xxx_user which > used extable type EX_TYPE_UACCESS_MC. Why x16? How is this intended to be consumed, and why is that behaviour different from any *other* fault? Mark. > Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> > --- > arch/arm64/include/asm/asm-extable.h | 14 ++++++++++++++ > arch/arm64/include/asm/asm-uaccess.h | 15 ++++++++++----- > arch/arm64/lib/copy_from_user.S | 18 +++++++++++------- > arch/arm64/lib/copy_to_user.S | 18 +++++++++++------- > arch/arm64/mm/extable.c | 18 ++++++++++++++---- > 5 files changed, 60 insertions(+), 23 deletions(-) > > diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h > index c39f2437e08e..75b2c00e9523 100644 > --- a/arch/arm64/include/asm/asm-extable.h > +++ b/arch/arm64/include/asm/asm-extable.h > @@ -2,12 +2,18 @@ > #ifndef __ASM_ASM_EXTABLE_H > #define __ASM_ASM_EXTABLE_H > > +#define FIXUP_TYPE_NORMAL 0 > +#define FIXUP_TYPE_MC 1 > + > #define EX_TYPE_NONE 0 > #define EX_TYPE_FIXUP 1 > #define EX_TYPE_BPF 2 > #define EX_TYPE_UACCESS_ERR_ZERO 3 > #define EX_TYPE_LOAD_UNALIGNED_ZEROPAD 4 > > +/* _MC indicates that can fixup from machine check errors */ > +#define EX_TYPE_UACCESS_MC 5 > + > #ifdef __ASSEMBLY__ > > #define __ASM_EXTABLE_RAW(insn, fixup, type, data) \ > @@ -27,6 +33,14 @@ > __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0) > .endm > > +/* > + * Create an exception table entry for `insn`, which will branch to `fixup` > + * when an unhandled fault(include sea fault) is taken. > + */ > + .macro _asm_extable_uaccess_mc, insn, fixup > + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_MC, 0) > + .endm > + > /* > * Create an exception table entry for `insn` if `fixup` is provided. Otherwise > * do nothing. > diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h > index 0557af834e03..6c23c138e1fc 100644 > --- a/arch/arm64/include/asm/asm-uaccess.h > +++ b/arch/arm64/include/asm/asm-uaccess.h > @@ -63,6 +63,11 @@ alternative_else_nop_endif > 9999: x; \ > _asm_extable 9999b, l > > + > +#define USER_MC(l, x...) \ > +9999: x; \ > + _asm_extable_uaccess_mc 9999b, l > + > /* > * Generate the assembly for LDTR/STTR with exception table entries. > * This is complicated as there is no post-increment or pair versions of the > @@ -73,8 +78,8 @@ alternative_else_nop_endif > 8889: ldtr \reg2, [\addr, #8]; > add \addr, \addr, \post_inc; > > - _asm_extable 8888b,\l; > - _asm_extable 8889b,\l; > + _asm_extable_uaccess_mc 8888b, \l; > + _asm_extable_uaccess_mc 8889b, \l; > .endm > > .macro user_stp l, reg1, reg2, addr, post_inc > @@ -82,14 +87,14 @@ alternative_else_nop_endif > 8889: sttr \reg2, [\addr, #8]; > add \addr, \addr, \post_inc; > > - _asm_extable 8888b,\l; > - _asm_extable 8889b,\l; > + _asm_extable_uaccess_mc 8888b,\l; > + _asm_extable_uaccess_mc 8889b,\l; > .endm > > .macro user_ldst l, inst, reg, addr, post_inc > 8888: \inst \reg, [\addr]; > add \addr, \addr, \post_inc; > > - _asm_extable 8888b,\l; > + _asm_extable_uaccess_mc 8888b, \l; > .endm > #endif > diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S > index 34e317907524..480cc5ac0a8d 100644 > --- a/arch/arm64/lib/copy_from_user.S > +++ b/arch/arm64/lib/copy_from_user.S > @@ -25,7 +25,7 @@ > .endm > > .macro strb1 reg, ptr, val > - strb \reg, [\ptr], \val > + USER_MC(9998f, strb \reg, [\ptr], \val) > .endm > > .macro ldrh1 reg, ptr, val > @@ -33,7 +33,7 @@ > .endm > > .macro strh1 reg, ptr, val > - strh \reg, [\ptr], \val > + USER_MC(9998f, strh \reg, [\ptr], \val) > .endm > > .macro ldr1 reg, ptr, val > @@ -41,7 +41,7 @@ > .endm > > .macro str1 reg, ptr, val > - str \reg, [\ptr], \val > + USER_MC(9998f, str \reg, [\ptr], \val) > .endm > > .macro ldp1 reg1, reg2, ptr, val > @@ -49,11 +49,12 @@ > .endm > > .macro stp1 reg1, reg2, ptr, val > - stp \reg1, \reg2, [\ptr], \val > + USER_MC(9998f, stp \reg1, \reg2, [\ptr], \val) > .endm > > -end .req x5 > -srcin .req x15 > +end .req x5 > +srcin .req x15 > +fixup_type .req x16 > SYM_FUNC_START(__arch_copy_from_user) > add end, x0, x2 > mov srcin, x1 > @@ -62,7 +63,10 @@ SYM_FUNC_START(__arch_copy_from_user) > ret > > // Exception fixups > -9997: cmp dst, dstin > + // x16: fixup type written by ex_handler_uaccess_mc > +9997: cmp fixup_type, #FIXUP_TYPE_MC > + b.eq 9998f > + cmp dst, dstin > b.ne 9998f > // Before being absolutely sure we couldn't copy anything, try harder > USER(9998f, ldtrb tmp1w, [srcin]) > diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S > index 802231772608..021a7d27b3a4 100644 > --- a/arch/arm64/lib/copy_to_user.S > +++ b/arch/arm64/lib/copy_to_user.S > @@ -20,7 +20,7 @@ > * x0 - bytes not copied > */ > .macro ldrb1 reg, ptr, val > - ldrb \reg, [\ptr], \val > + USER_MC(9998f, ldrb \reg, [\ptr], \val) > .endm > > .macro strb1 reg, ptr, val > @@ -28,7 +28,7 @@ > .endm > > .macro ldrh1 reg, ptr, val > - ldrh \reg, [\ptr], \val > + USER_MC(9998f, ldrh \reg, [\ptr], \val) > .endm > > .macro strh1 reg, ptr, val > @@ -36,7 +36,7 @@ > .endm > > .macro ldr1 reg, ptr, val > - ldr \reg, [\ptr], \val > + USER_MC(9998f, ldr \reg, [\ptr], \val) > .endm > > .macro str1 reg, ptr, val > @@ -44,15 +44,16 @@ > .endm > > .macro ldp1 reg1, reg2, ptr, val > - ldp \reg1, \reg2, [\ptr], \val > + USER_MC(9998f, ldp \reg1, \reg2, [\ptr], \val) > .endm > > .macro stp1 reg1, reg2, ptr, val > user_stp 9997f, \reg1, \reg2, \ptr, \val > .endm > > -end .req x5 > -srcin .req x15 > +end .req x5 > +srcin .req x15 > +fixup_type .req x16 > SYM_FUNC_START(__arch_copy_to_user) > add end, x0, x2 > mov srcin, x1 > @@ -61,7 +62,10 @@ SYM_FUNC_START(__arch_copy_to_user) > ret > > // Exception fixups > -9997: cmp dst, dstin > + // x16: fixup type written by ex_handler_uaccess_mc > +9997: cmp fixup_type, #FIXUP_TYPE_MC > + b.eq 9998f > + cmp dst, dstin > b.ne 9998f > // Before being absolutely sure we couldn't copy anything, try harder > ldrb tmp1w, [srcin] > diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c > index 4f0083a550d4..525876c3ebf4 100644 > --- a/arch/arm64/mm/extable.c > +++ b/arch/arm64/mm/extable.c > @@ -24,6 +24,14 @@ static bool ex_handler_fixup(const struct exception_table_entry *ex, > return true; > } > > +static bool ex_handler_uaccess_type(const struct exception_table_entry *ex, > + struct pt_regs *regs, > + unsigned long fixup_type) > +{ > + regs->regs[16] = fixup_type; > + return ex_handler_fixup(ex, regs); > +} > + > static bool ex_handler_uaccess_err_zero(const struct exception_table_entry *ex, > struct pt_regs *regs) > { > @@ -75,6 +83,8 @@ bool fixup_exception(struct pt_regs *regs) > switch (ex->type) { > case EX_TYPE_FIXUP: > return ex_handler_fixup(ex, regs); > + case EX_TYPE_UACCESS_MC: > + return ex_handler_uaccess_type(ex, regs, FIXUP_TYPE_NORMAL); > case EX_TYPE_BPF: > return ex_handler_bpf(ex, regs); > case EX_TYPE_UACCESS_ERR_ZERO: > @@ -94,10 +104,10 @@ bool fixup_exception_mc(struct pt_regs *regs) > if (!ex) > return false; > > - /* > - * This is not complete, More Machine check safe extable type can > - * be processed here. > - */ > + switch (ex->type) { > + case EX_TYPE_UACCESS_MC: > + return ex_handler_uaccess_type(ex, regs, FIXUP_TYPE_MC); > + } > > return false; > } > -- > 2.25.1 >
在 2022/5/13 23:31, Mark Rutland 写道: > On Wed, Apr 20, 2022 at 03:04:15AM +0000, Tong Tiangen wrote: >> Add copy_{to, from}_user() to machine check safe. >> >> If copy fail due to hardware memory error, 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. >> >> Add new extable type EX_TYPE_UACCESS_MC which can be used for uaccess that >> can be recovered from hardware memory errors. > > I don't understand why we need this. > > If we apply EX_TYPE_UACCESS consistently to *all* user accesses, and > *only* to user accesses, that would *always* indicate that we can > recover, and that seems much simpler to deal with. > > Today we use EX_TYPE_UACCESS_ERR_ZERO for kernel accesses in a couple of > cases, which we should clean up, and we user EX_TYPE_FIXUP for a couple > of user accesses, but those could easily be converted over. > >> The x16 register is used to save the fixup type in copy_xxx_user which >> used extable type EX_TYPE_UACCESS_MC. This is dicussed on patch patch 3/7. > > Why x16? > > How is this intended to be consumed, and why is that behaviour different > from any *other* fault? > > Mark. This is to distinguish EX_TYPE_FIXUP, if this exception is triggered, in fixup processing, it is needed to copy by byte, but if exception is triggered by machine check, the data does not need to be copied again. So we need one place to store exception type, Therefore, X16 that is not currently used in copy_from/to_user is selected. Maybe better to use exception_table_entry->data to pass the register that needs to be set? Thanks, Tong. > >> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> >> --- >> arch/arm64/include/asm/asm-extable.h | 14 ++++++++++++++ >> arch/arm64/include/asm/asm-uaccess.h | 15 ++++++++++----- >> arch/arm64/lib/copy_from_user.S | 18 +++++++++++------- >> arch/arm64/lib/copy_to_user.S | 18 +++++++++++------- >> arch/arm64/mm/extable.c | 18 ++++++++++++++---- >> 5 files changed, 60 insertions(+), 23 deletions(-) >> >> diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h >> index c39f2437e08e..75b2c00e9523 100644 >> --- a/arch/arm64/include/asm/asm-extable.h >> +++ b/arch/arm64/include/asm/asm-extable.h >> @@ -2,12 +2,18 @@ >> #ifndef __ASM_ASM_EXTABLE_H >> #define __ASM_ASM_EXTABLE_H >> >> +#define FIXUP_TYPE_NORMAL 0 >> +#define FIXUP_TYPE_MC 1 >> + >> #define EX_TYPE_NONE 0 >> #define EX_TYPE_FIXUP 1 >> #define EX_TYPE_BPF 2 >> #define EX_TYPE_UACCESS_ERR_ZERO 3 >> #define EX_TYPE_LOAD_UNALIGNED_ZEROPAD 4 >> >> +/* _MC indicates that can fixup from machine check errors */ >> +#define EX_TYPE_UACCESS_MC 5 >> + >> #ifdef __ASSEMBLY__ >> >> #define __ASM_EXTABLE_RAW(insn, fixup, type, data) \ >> @@ -27,6 +33,14 @@ >> __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0) >> .endm >> >> +/* >> + * Create an exception table entry for `insn`, which will branch to `fixup` >> + * when an unhandled fault(include sea fault) is taken. >> + */ >> + .macro _asm_extable_uaccess_mc, insn, fixup >> + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_MC, 0) >> + .endm >> + >> /* >> * Create an exception table entry for `insn` if `fixup` is provided. Otherwise >> * do nothing. >> diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h >> index 0557af834e03..6c23c138e1fc 100644 >> --- a/arch/arm64/include/asm/asm-uaccess.h >> +++ b/arch/arm64/include/asm/asm-uaccess.h >> @@ -63,6 +63,11 @@ alternative_else_nop_endif >> 9999: x; \ >> _asm_extable 9999b, l >> >> + >> +#define USER_MC(l, x...) \ >> +9999: x; \ >> + _asm_extable_uaccess_mc 9999b, l >> + >> /* >> * Generate the assembly for LDTR/STTR with exception table entries. >> * This is complicated as there is no post-increment or pair versions of the >> @@ -73,8 +78,8 @@ alternative_else_nop_endif >> 8889: ldtr \reg2, [\addr, #8]; >> add \addr, \addr, \post_inc; >> >> - _asm_extable 8888b,\l; >> - _asm_extable 8889b,\l; >> + _asm_extable_uaccess_mc 8888b, \l; >> + _asm_extable_uaccess_mc 8889b, \l; >> .endm >> >> .macro user_stp l, reg1, reg2, addr, post_inc >> @@ -82,14 +87,14 @@ alternative_else_nop_endif >> 8889: sttr \reg2, [\addr, #8]; >> add \addr, \addr, \post_inc; >> >> - _asm_extable 8888b,\l; >> - _asm_extable 8889b,\l; >> + _asm_extable_uaccess_mc 8888b,\l; >> + _asm_extable_uaccess_mc 8889b,\l; >> .endm >> >> .macro user_ldst l, inst, reg, addr, post_inc >> 8888: \inst \reg, [\addr]; >> add \addr, \addr, \post_inc; >> >> - _asm_extable 8888b,\l; >> + _asm_extable_uaccess_mc 8888b, \l; >> .endm >> #endif >> diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S >> index 34e317907524..480cc5ac0a8d 100644 >> --- a/arch/arm64/lib/copy_from_user.S >> +++ b/arch/arm64/lib/copy_from_user.S >> @@ -25,7 +25,7 @@ >> .endm >> >> .macro strb1 reg, ptr, val >> - strb \reg, [\ptr], \val >> + USER_MC(9998f, strb \reg, [\ptr], \val) >> .endm >> >> .macro ldrh1 reg, ptr, val >> @@ -33,7 +33,7 @@ >> .endm >> >> .macro strh1 reg, ptr, val >> - strh \reg, [\ptr], \val >> + USER_MC(9998f, strh \reg, [\ptr], \val) >> .endm >> >> .macro ldr1 reg, ptr, val >> @@ -41,7 +41,7 @@ >> .endm >> >> .macro str1 reg, ptr, val >> - str \reg, [\ptr], \val >> + USER_MC(9998f, str \reg, [\ptr], \val) >> .endm >> >> .macro ldp1 reg1, reg2, ptr, val >> @@ -49,11 +49,12 @@ >> .endm >> >> .macro stp1 reg1, reg2, ptr, val >> - stp \reg1, \reg2, [\ptr], \val >> + USER_MC(9998f, stp \reg1, \reg2, [\ptr], \val) >> .endm >> >> -end .req x5 >> -srcin .req x15 >> +end .req x5 >> +srcin .req x15 >> +fixup_type .req x16 >> SYM_FUNC_START(__arch_copy_from_user) >> add end, x0, x2 >> mov srcin, x1 >> @@ -62,7 +63,10 @@ SYM_FUNC_START(__arch_copy_from_user) >> ret >> >> // Exception fixups >> -9997: cmp dst, dstin >> + // x16: fixup type written by ex_handler_uaccess_mc >> +9997: cmp fixup_type, #FIXUP_TYPE_MC >> + b.eq 9998f >> + cmp dst, dstin >> b.ne 9998f >> // Before being absolutely sure we couldn't copy anything, try harder >> USER(9998f, ldtrb tmp1w, [srcin]) >> diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S >> index 802231772608..021a7d27b3a4 100644 >> --- a/arch/arm64/lib/copy_to_user.S >> +++ b/arch/arm64/lib/copy_to_user.S >> @@ -20,7 +20,7 @@ >> * x0 - bytes not copied >> */ >> .macro ldrb1 reg, ptr, val >> - ldrb \reg, [\ptr], \val >> + USER_MC(9998f, ldrb \reg, [\ptr], \val) >> .endm >> >> .macro strb1 reg, ptr, val >> @@ -28,7 +28,7 @@ >> .endm >> >> .macro ldrh1 reg, ptr, val >> - ldrh \reg, [\ptr], \val >> + USER_MC(9998f, ldrh \reg, [\ptr], \val) >> .endm >> >> .macro strh1 reg, ptr, val >> @@ -36,7 +36,7 @@ >> .endm >> >> .macro ldr1 reg, ptr, val >> - ldr \reg, [\ptr], \val >> + USER_MC(9998f, ldr \reg, [\ptr], \val) >> .endm >> >> .macro str1 reg, ptr, val >> @@ -44,15 +44,16 @@ >> .endm >> >> .macro ldp1 reg1, reg2, ptr, val >> - ldp \reg1, \reg2, [\ptr], \val >> + USER_MC(9998f, ldp \reg1, \reg2, [\ptr], \val) >> .endm >> >> .macro stp1 reg1, reg2, ptr, val >> user_stp 9997f, \reg1, \reg2, \ptr, \val >> .endm >> >> -end .req x5 >> -srcin .req x15 >> +end .req x5 >> +srcin .req x15 >> +fixup_type .req x16 >> SYM_FUNC_START(__arch_copy_to_user) >> add end, x0, x2 >> mov srcin, x1 >> @@ -61,7 +62,10 @@ SYM_FUNC_START(__arch_copy_to_user) >> ret >> >> // Exception fixups >> -9997: cmp dst, dstin >> + // x16: fixup type written by ex_handler_uaccess_mc >> +9997: cmp fixup_type, #FIXUP_TYPE_MC >> + b.eq 9998f >> + cmp dst, dstin >> b.ne 9998f >> // Before being absolutely sure we couldn't copy anything, try harder >> ldrb tmp1w, [srcin] >> diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c >> index 4f0083a550d4..525876c3ebf4 100644 >> --- a/arch/arm64/mm/extable.c >> +++ b/arch/arm64/mm/extable.c >> @@ -24,6 +24,14 @@ static bool ex_handler_fixup(const struct exception_table_entry *ex, >> return true; >> } >> >> +static bool ex_handler_uaccess_type(const struct exception_table_entry *ex, >> + struct pt_regs *regs, >> + unsigned long fixup_type) >> +{ >> + regs->regs[16] = fixup_type; >> + return ex_handler_fixup(ex, regs); >> +} >> + >> static bool ex_handler_uaccess_err_zero(const struct exception_table_entry *ex, >> struct pt_regs *regs) >> { >> @@ -75,6 +83,8 @@ bool fixup_exception(struct pt_regs *regs) >> switch (ex->type) { >> case EX_TYPE_FIXUP: >> return ex_handler_fixup(ex, regs); >> + case EX_TYPE_UACCESS_MC: >> + return ex_handler_uaccess_type(ex, regs, FIXUP_TYPE_NORMAL); >> case EX_TYPE_BPF: >> return ex_handler_bpf(ex, regs); >> case EX_TYPE_UACCESS_ERR_ZERO: >> @@ -94,10 +104,10 @@ bool fixup_exception_mc(struct pt_regs *regs) >> if (!ex) >> return false; >> >> - /* >> - * This is not complete, More Machine check safe extable type can >> - * be processed here. >> - */ >> + switch (ex->type) { >> + case EX_TYPE_UACCESS_MC: >> + return ex_handler_uaccess_type(ex, regs, FIXUP_TYPE_MC); >> + } >> >> return false; >> } >> -- >> 2.25.1 >> > .
diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h index c39f2437e08e..75b2c00e9523 100644 --- a/arch/arm64/include/asm/asm-extable.h +++ b/arch/arm64/include/asm/asm-extable.h @@ -2,12 +2,18 @@ #ifndef __ASM_ASM_EXTABLE_H #define __ASM_ASM_EXTABLE_H +#define FIXUP_TYPE_NORMAL 0 +#define FIXUP_TYPE_MC 1 + #define EX_TYPE_NONE 0 #define EX_TYPE_FIXUP 1 #define EX_TYPE_BPF 2 #define EX_TYPE_UACCESS_ERR_ZERO 3 #define EX_TYPE_LOAD_UNALIGNED_ZEROPAD 4 +/* _MC indicates that can fixup from machine check errors */ +#define EX_TYPE_UACCESS_MC 5 + #ifdef __ASSEMBLY__ #define __ASM_EXTABLE_RAW(insn, fixup, type, data) \ @@ -27,6 +33,14 @@ __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0) .endm +/* + * Create an exception table entry for `insn`, which will branch to `fixup` + * when an unhandled fault(include sea fault) is taken. + */ + .macro _asm_extable_uaccess_mc, insn, fixup + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_MC, 0) + .endm + /* * Create an exception table entry for `insn` if `fixup` is provided. Otherwise * do nothing. diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h index 0557af834e03..6c23c138e1fc 100644 --- a/arch/arm64/include/asm/asm-uaccess.h +++ b/arch/arm64/include/asm/asm-uaccess.h @@ -63,6 +63,11 @@ alternative_else_nop_endif 9999: x; \ _asm_extable 9999b, l + +#define USER_MC(l, x...) \ +9999: x; \ + _asm_extable_uaccess_mc 9999b, l + /* * Generate the assembly for LDTR/STTR with exception table entries. * This is complicated as there is no post-increment or pair versions of the @@ -73,8 +78,8 @@ alternative_else_nop_endif 8889: ldtr \reg2, [\addr, #8]; add \addr, \addr, \post_inc; - _asm_extable 8888b,\l; - _asm_extable 8889b,\l; + _asm_extable_uaccess_mc 8888b, \l; + _asm_extable_uaccess_mc 8889b, \l; .endm .macro user_stp l, reg1, reg2, addr, post_inc @@ -82,14 +87,14 @@ alternative_else_nop_endif 8889: sttr \reg2, [\addr, #8]; add \addr, \addr, \post_inc; - _asm_extable 8888b,\l; - _asm_extable 8889b,\l; + _asm_extable_uaccess_mc 8888b,\l; + _asm_extable_uaccess_mc 8889b,\l; .endm .macro user_ldst l, inst, reg, addr, post_inc 8888: \inst \reg, [\addr]; add \addr, \addr, \post_inc; - _asm_extable 8888b,\l; + _asm_extable_uaccess_mc 8888b, \l; .endm #endif diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S index 34e317907524..480cc5ac0a8d 100644 --- a/arch/arm64/lib/copy_from_user.S +++ b/arch/arm64/lib/copy_from_user.S @@ -25,7 +25,7 @@ .endm .macro strb1 reg, ptr, val - strb \reg, [\ptr], \val + USER_MC(9998f, strb \reg, [\ptr], \val) .endm .macro ldrh1 reg, ptr, val @@ -33,7 +33,7 @@ .endm .macro strh1 reg, ptr, val - strh \reg, [\ptr], \val + USER_MC(9998f, strh \reg, [\ptr], \val) .endm .macro ldr1 reg, ptr, val @@ -41,7 +41,7 @@ .endm .macro str1 reg, ptr, val - str \reg, [\ptr], \val + USER_MC(9998f, str \reg, [\ptr], \val) .endm .macro ldp1 reg1, reg2, ptr, val @@ -49,11 +49,12 @@ .endm .macro stp1 reg1, reg2, ptr, val - stp \reg1, \reg2, [\ptr], \val + USER_MC(9998f, stp \reg1, \reg2, [\ptr], \val) .endm -end .req x5 -srcin .req x15 +end .req x5 +srcin .req x15 +fixup_type .req x16 SYM_FUNC_START(__arch_copy_from_user) add end, x0, x2 mov srcin, x1 @@ -62,7 +63,10 @@ SYM_FUNC_START(__arch_copy_from_user) ret // Exception fixups -9997: cmp dst, dstin + // x16: fixup type written by ex_handler_uaccess_mc +9997: cmp fixup_type, #FIXUP_TYPE_MC + b.eq 9998f + cmp dst, dstin b.ne 9998f // Before being absolutely sure we couldn't copy anything, try harder USER(9998f, ldtrb tmp1w, [srcin]) diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S index 802231772608..021a7d27b3a4 100644 --- a/arch/arm64/lib/copy_to_user.S +++ b/arch/arm64/lib/copy_to_user.S @@ -20,7 +20,7 @@ * x0 - bytes not copied */ .macro ldrb1 reg, ptr, val - ldrb \reg, [\ptr], \val + USER_MC(9998f, ldrb \reg, [\ptr], \val) .endm .macro strb1 reg, ptr, val @@ -28,7 +28,7 @@ .endm .macro ldrh1 reg, ptr, val - ldrh \reg, [\ptr], \val + USER_MC(9998f, ldrh \reg, [\ptr], \val) .endm .macro strh1 reg, ptr, val @@ -36,7 +36,7 @@ .endm .macro ldr1 reg, ptr, val - ldr \reg, [\ptr], \val + USER_MC(9998f, ldr \reg, [\ptr], \val) .endm .macro str1 reg, ptr, val @@ -44,15 +44,16 @@ .endm .macro ldp1 reg1, reg2, ptr, val - ldp \reg1, \reg2, [\ptr], \val + USER_MC(9998f, ldp \reg1, \reg2, [\ptr], \val) .endm .macro stp1 reg1, reg2, ptr, val user_stp 9997f, \reg1, \reg2, \ptr, \val .endm -end .req x5 -srcin .req x15 +end .req x5 +srcin .req x15 +fixup_type .req x16 SYM_FUNC_START(__arch_copy_to_user) add end, x0, x2 mov srcin, x1 @@ -61,7 +62,10 @@ SYM_FUNC_START(__arch_copy_to_user) ret // Exception fixups -9997: cmp dst, dstin + // x16: fixup type written by ex_handler_uaccess_mc +9997: cmp fixup_type, #FIXUP_TYPE_MC + b.eq 9998f + cmp dst, dstin b.ne 9998f // Before being absolutely sure we couldn't copy anything, try harder ldrb tmp1w, [srcin] diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c index 4f0083a550d4..525876c3ebf4 100644 --- a/arch/arm64/mm/extable.c +++ b/arch/arm64/mm/extable.c @@ -24,6 +24,14 @@ static bool ex_handler_fixup(const struct exception_table_entry *ex, return true; } +static bool ex_handler_uaccess_type(const struct exception_table_entry *ex, + struct pt_regs *regs, + unsigned long fixup_type) +{ + regs->regs[16] = fixup_type; + return ex_handler_fixup(ex, regs); +} + static bool ex_handler_uaccess_err_zero(const struct exception_table_entry *ex, struct pt_regs *regs) { @@ -75,6 +83,8 @@ bool fixup_exception(struct pt_regs *regs) switch (ex->type) { case EX_TYPE_FIXUP: return ex_handler_fixup(ex, regs); + case EX_TYPE_UACCESS_MC: + return ex_handler_uaccess_type(ex, regs, FIXUP_TYPE_NORMAL); case EX_TYPE_BPF: return ex_handler_bpf(ex, regs); case EX_TYPE_UACCESS_ERR_ZERO: @@ -94,10 +104,10 @@ bool fixup_exception_mc(struct pt_regs *regs) if (!ex) return false; - /* - * This is not complete, More Machine check safe extable type can - * be processed here. - */ + switch (ex->type) { + case EX_TYPE_UACCESS_MC: + return ex_handler_uaccess_type(ex, regs, FIXUP_TYPE_MC); + } return false; }
Add copy_{to, from}_user() to machine check safe. If copy fail due to hardware memory error, 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. Add new extable type EX_TYPE_UACCESS_MC which can be used for uaccess that can be recovered from hardware memory errors. The x16 register is used to save the fixup type in copy_xxx_user which used extable type EX_TYPE_UACCESS_MC. Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> --- arch/arm64/include/asm/asm-extable.h | 14 ++++++++++++++ arch/arm64/include/asm/asm-uaccess.h | 15 ++++++++++----- arch/arm64/lib/copy_from_user.S | 18 +++++++++++------- arch/arm64/lib/copy_to_user.S | 18 +++++++++++------- arch/arm64/mm/extable.c | 18 ++++++++++++++---- 5 files changed, 60 insertions(+), 23 deletions(-)