在 2024/1/30 20:01, Mark Rutland 写道:
On Tue, Jan 30, 2024 at 07:14:35PM +0800, Tong Tiangen wrote:
在 2024/1/30 1:43, Mark Rutland 写道:
On Mon, Jan 29, 2024 at 09:46:49PM +0800, Tong Tiangen wrote:
Further, this change will also silently fixup unexpected kernel faults if we
pass bad kernel pointers to copy_{to,from}_user, which will hide real bugs.
I think this is better than the panic kernel, because the real bugs
belongs to the user process. Even if the wrong pointer is
transferred, the page corresponding to the wrong pointer has a memroy
error.
I think you have misunderstood my point; I'm talking about the case of a bad
kernel pointer *without* a memory error.
For example, consider some buggy code such as:
void __user *uptr = some_valid_user_pointer;
void *kptr = NULL; // or any other bad pointer
ret = copy_to_user(uptr, kptr, size);
if (ret)
return -EFAULT;
Before this patch, when copy_to_user() attempted to load from NULL it would
fault, there would be no fixup handler for the LDR, and the kernel would die(),
reporting the bad kernel access.
After this patch (which adds fixup handlers to all the LDR*s in
copy_to_user()), the fault (which is *not* a memory error) would be handled by
the fixup handler, and copy_to_user() would return an error without *any*
indication of the horrible kernel bug.
This will hide kernel bugs, which will make those harder to identify and fix,
and will also potentially make it easier to exploit the kernel: if the user
somehow gains control of the kernel pointer, they can rely on the fixup handler
returning an error, and can scan through memory rather than dying as soon as
they pas a bad pointer.
I should understand what you mean. I'll think about this and reply.
Many thanks.
Tong.
In addition, the panic information contains necessary information
for users to check.
There is no panic() in the case I am describing.
So NAK to this change as-is; likewise for the addition of USER() to other ldr*
macros in copy_from_user.S and the addition of USER() str* macros in
copy_to_user.S.
If we want to handle memory errors on some kaccesses, we need a new EX_TYPE_*
separate from the usual EX_TYPE_KACESS_ERR_ZERO that means "handle memory
errors, but treat other faults as fatal". That should come with a rationale and
explanation of why it's actually useful.
This makes sense. Add kaccess types that can be processed properly.
[...]
diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
index 478e639f8680..28ec35e3d210 100644
--- a/arch/arm64/mm/extable.c
+++ b/arch/arm64/mm/extable.c
@@ -85,10 +85,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);
+ }
Please fold this part into the prior patch, and start ogf with *only* handling
errors on accesses already marked with EX_TYPE_UACCESS_ERR_ZERO. I think that
change would be relatively uncontroversial, and it would be much easier to
build atop that.
OK, the two patches will be merged in the next release.
Thanks.
Mark.
.
