Re: [PATCH] arm64: ftrace: don't dereference a probably invalid address

On Mon, 7 Jun 2021 11:23:30 +0800
Mark-PK Tsai [off-list ref] wrote:

quoted

Address in __mcount_loc may be invalid if somthing goes wrong.
On our arm64 platform, the bug in recordmcount make kernel
crash in ftrace_init().

How did it crash? The link below doesn't show any crash.

Below is the crash log:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:2008 ftrace_bug+0x9c/0x38c
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Tainted: G        W         5.4.61-350609-gf78fedda5a5e #1
Hardware name: MediaTek MT5896 (DT)
pstate: 60400089 (nZCv daIf +PAN -UAO)
pc : ftrace_bug+0x9c/0x38c
lr : ftrace_process_locs+0x314/0x3b8
sp : ffffffc011743ef0
x29: ffffffc011743f00 x28: 0000000000000001
x27: ffffff818e401b80 x26: 0000000000000000
x25: ffffff818e480008 x24: ffffffc011749000
x23: 0000000000000008 x22: 0000000000000000
x21: ffffffc010084ac0 x20: 0000000000000024
x19: ffffff818e480000 x18: ffffffc011759c20
x17: ffffffc01133dcf8 x16: 0000000000000068
x15: ffffffc01133dcf8 x14: 0000000000000000
x13: 0000000000000000 x12: ffffffc010084ae4
x11: ffffffc011749000 x10: ffffffc011749000
x9 : 0000000000000001 x8 : ffffffc011749000
x7 : 0000000000000000 x6 : 000000000000003f
x5 : 000000000008e93d x4 : 0000000000000000
x3 : 0000000000000101 x2 : ffffffc010084ac0
x1 : ffffff818e480000 x0 : ffffffc01127621c
Call trace:
 ftrace_bug+0x9c/0x38c
 ftrace_process_locs+0x314/0x3b8
 ftrace_init+0x8c/0xbc
 start_kernel+0x180/0x40c
---[ end trace 59db467eb74a6604 ]---
ftrace failed to modify
[<0000000000000024>] 0x24
 actual:
"Unable to handle kernel read from unreadable memory at virtual address 0000000000000024
"Mem abort info:

And the crash is becuase kernel trying to read *rec->ip in print_ip_ins() if
ftrace_bug() get error code -EINVAL.

quoted

https://lore.kernel.org/lkml/20210607023839.26387-1-mark-pk.tsai@mediatek.com/ (local)

Return -EFAULT if we are dealing with out-of-range condition
to prevent dereference the invalid address in ftrace_bug(),
then the kernel can disable ftrace safely for problematic
__mcount_loc.

!mod is not an out-of-range condition. It just happened that the other
bug caused this strange side-effect. A !mod does not mean a fault
happened. Just because it may have been caused by a fault in your use
case does not mean that it's a fault in all use cases. That's like
saying that your dog is a poodle, so all dogs are poodles.

A return of -EINVAL should not cause a crash. If it does, then that
needs to be fixed.

I understand.
Keep -EINVAL here make more sense.
So maybe we should handle this case in ftrace_bug() by checking the rec->ip?

-- Steve

quoted

Signed-off-by: Mark-PK Tsai <mark-pk.tsai@mediatek.com>
---
 arch/arm64/kernel/ftrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index b5d3ddaf69d9..98bec8445a58 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c

@@ -201,7 +201,7 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 			preempt_enable();
 
 			if (WARN_ON(!mod))
-				return -EINVAL;
+				return -EFAULT;
 		}
 
 		/*

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek