Re: [PATCH -next v5 7/8] arm64: add uaccess to machine check safe
From: Tong Tiangen
Date: Sat Jun 18 2022 - 05:27:55 EST
在 2022/6/17 17:06, Mark Rutland 写道:
On Sat, May 28, 2022 at 06:50:55AM +0000, Tong Tiangen wrote:
If user access fail due to hardware memory error, only the relevant
processes are affected, so killing the user process and isolate the
error page with hardware memory errors is a more reasonable choice
than kernel panic.
Signed-off-by: Tong Tiangen <tongtiangen@xxxxxxxxxx>
---
arch/arm64/lib/copy_from_user.S | 8 ++++----
arch/arm64/lib/copy_to_user.S | 8 ++++----
All of these changes are to the *kernel* accesses performed as part of copy
to/from user, and have nothing to do with userspace, so it does not make sense
to mark these as UACCESS.
You have a point. so there is no need to modify copy_from/to_user.S in
this patch set.
Do we *actually* need to recover from failues on these accesses? Looking at
_copy_from_user(), the kernel will immediately follow this up with a memset()
to the same address which will be fatal anyway, so this is only punting the
failure for a few instructions.
If recovery success, The task will be killed and there will be no
subsequent memset().
If we really need to recover from certain accesses to kernel memory we should
add a new EX_TYPE_KACCESS_ERR_ZERO_MC or similar, but we need a strong
rationale as to why that's useful. As things stand I do not beleive it makes
sense for copy to/from user specifically.
arch/arm64/mm/extable.c | 8 ++++----
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
index 34e317907524..402dd48a4f93 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(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(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(9998f, str \reg, [\ptr], \val)
.endm
.macro ldp1 reg1, reg2, ptr, val
@@ -49,7 +49,7 @@
.endm
.macro stp1 reg1, reg2, ptr, val
- stp \reg1, \reg2, [\ptr], \val
+ USER(9998f, stp \reg1, \reg2, [\ptr], \val)
.endm
end .req x5
diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
index 802231772608..4134bdb3a8b0 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(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(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(9998f, ldr \reg, [\ptr], \val)
.endm
.macro str1 reg, ptr, val
@@ -44,7 +44,7 @@
.endm
.macro ldp1 reg1, reg2, ptr, val
- ldp \reg1, \reg2, [\ptr], \val
+ USER(9998f, ldp \reg1, \reg2, [\ptr], \val)
.endm
.macro stp1 reg1, reg2, ptr, val
diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
index c301dcf6335f..8ca8d9639f9f 100644
--- a/arch/arm64/mm/extable.c
+++ b/arch/arm64/mm/extable.c
@@ -86,10 +86,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_ERR_ZERO:
+ return ex_handler_uaccess_err_zero(ex, regs);
+ }
This addition specifically makes sense to me, so can you split this into a separate patch?
According to my understanding of the above, only the modification of
extable.c is retained.
So what do you mean which part is made into a separate patch?
Thanks,
Tong.
Thanks,
Mark.
.