* [PATCH 1/2] arm64/process.c: fix Wmissing-prototypes build warnings [not found] <CGME20210324065511epcas5p1cb74119660e2d98a381ae69d01b29275@epcas5p1.samsung.com> @ 2021-03-24 6:54 ` Maninder Singh 0 siblings, 0 replies; 10+ messages in thread From: Maninder Singh @ 2021-03-24 6:54 UTC (permalink / raw) To: catalin.marinas, will, pcc, mark.rutland, akpm, 0x7f454c46, amit.kachhap, james.morse, vincenzo.frascino, nsaenzjulienne, keescook, samitolvanen Cc: linux-arm-kernel, linux-kernel, a.sahrawat, Maninder Singh function protypes are missed before defination, which leads to compilation warning with "-Wmissing-prototypes" flag. https://lkml.org/lkml/2021/3/19/840 arch/arm64/kernel/process.c:261:6: warning: no previous prototype for '__show_regs' [-Wmissing-prototypes] 261 | void __show_regs(struct pt_regs *regs) | ^~~~~~~~~~~ arch/arm64/kernel/process.c:307:6: warning: no previous prototype for '__show_regs_alloc_free' [-Wmissing-prototypes] 307 | void __show_regs_alloc_free(struct pt_regs *regs) | ^~~~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/process.c:365:5: warning: no previous prototype for 'arch_dup_task_struct' [-Wmissing-prototypes] 365 | int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) | ^~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/process.c:546:41: warning: no previous prototype for '__switch_to' [-Wmissing-prototypes] 546 | __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev, | ^~~~~~~~~~~ arch/arm64/kernel/process.c:710:25: warning: no previous prototype for 'arm64_preempt_schedule_irq' [-Wmissing-prototypes] 710 | asmlinkage void __sched arm64_preempt_schedule_irq(void) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Maninder Singh <maninder1.s@samsung.com> --- arch/arm64/include/asm/processor.h | 2 ++ arch/arm64/include/asm/thread_info.h | 2 ++ arch/arm64/kernel/process.c | 2 ++ 3 files changed, 6 insertions(+) diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index ca2cd75d3286..efc10e9041a0 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h @@ -251,6 +251,8 @@ unsigned long get_wchan(struct task_struct *p); extern struct task_struct *cpu_switch_to(struct task_struct *prev, struct task_struct *next); +asmlinkage void arm64_preempt_schedule_irq(void); + #define task_pt_regs(p) \ ((struct pt_regs *)(THREAD_SIZE + task_stack_page(p)) - 1) diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h index 9f4e3b266f21..6623c99f0984 100644 --- a/arch/arm64/include/asm/thread_info.h +++ b/arch/arm64/include/asm/thread_info.h @@ -55,6 +55,8 @@ void arch_setup_new_exec(void); #define arch_setup_new_exec arch_setup_new_exec void arch_release_task_struct(struct task_struct *tsk); +int arch_dup_task_struct(struct task_struct *dst, + struct task_struct *src); #endif diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 325c83b1a24d..6e60aa3b5ea9 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -57,6 +57,8 @@ #include <asm/processor.h> #include <asm/pointer_auth.h> #include <asm/stacktrace.h> +#include <asm/switch_to.h> +#include <asm/system_misc.h> #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK) #include <linux/stackprotector.h> -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/2] arm64/process.c: fix Wmissing-prototypes build warnings @ 2021-03-24 6:54 ` Maninder Singh 0 siblings, 0 replies; 10+ messages in thread From: Maninder Singh @ 2021-03-24 6:54 UTC (permalink / raw) To: catalin.marinas, will, pcc, mark.rutland, akpm, 0x7f454c46, amit.kachhap, james.morse, vincenzo.frascino, nsaenzjulienne, keescook, samitolvanen Cc: linux-arm-kernel, linux-kernel, a.sahrawat, Maninder Singh function protypes are missed before defination, which leads to compilation warning with "-Wmissing-prototypes" flag. https://lkml.org/lkml/2021/3/19/840 arch/arm64/kernel/process.c:261:6: warning: no previous prototype for '__show_regs' [-Wmissing-prototypes] 261 | void __show_regs(struct pt_regs *regs) | ^~~~~~~~~~~ arch/arm64/kernel/process.c:307:6: warning: no previous prototype for '__show_regs_alloc_free' [-Wmissing-prototypes] 307 | void __show_regs_alloc_free(struct pt_regs *regs) | ^~~~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/process.c:365:5: warning: no previous prototype for 'arch_dup_task_struct' [-Wmissing-prototypes] 365 | int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) | ^~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/process.c:546:41: warning: no previous prototype for '__switch_to' [-Wmissing-prototypes] 546 | __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev, | ^~~~~~~~~~~ arch/arm64/kernel/process.c:710:25: warning: no previous prototype for 'arm64_preempt_schedule_irq' [-Wmissing-prototypes] 710 | asmlinkage void __sched arm64_preempt_schedule_irq(void) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Maninder Singh <maninder1.s@samsung.com> --- arch/arm64/include/asm/processor.h | 2 ++ arch/arm64/include/asm/thread_info.h | 2 ++ arch/arm64/kernel/process.c | 2 ++ 3 files changed, 6 insertions(+) diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index ca2cd75d3286..efc10e9041a0 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h @@ -251,6 +251,8 @@ unsigned long get_wchan(struct task_struct *p); extern struct task_struct *cpu_switch_to(struct task_struct *prev, struct task_struct *next); +asmlinkage void arm64_preempt_schedule_irq(void); + #define task_pt_regs(p) \ ((struct pt_regs *)(THREAD_SIZE + task_stack_page(p)) - 1) diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h index 9f4e3b266f21..6623c99f0984 100644 --- a/arch/arm64/include/asm/thread_info.h +++ b/arch/arm64/include/asm/thread_info.h @@ -55,6 +55,8 @@ void arch_setup_new_exec(void); #define arch_setup_new_exec arch_setup_new_exec void arch_release_task_struct(struct task_struct *tsk); +int arch_dup_task_struct(struct task_struct *dst, + struct task_struct *src); #endif diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 325c83b1a24d..6e60aa3b5ea9 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -57,6 +57,8 @@ #include <asm/processor.h> #include <asm/pointer_auth.h> #include <asm/stacktrace.h> +#include <asm/switch_to.h> +#include <asm/system_misc.h> #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK) #include <linux/stackprotector.h> -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <CGME20210324065516epcas5p450958011b69f5941e8f2bc993b82b904@epcas5p4.samsung.com>]
* [PATCH 2/2] arm64: print alloc free paths for address in registers [not found] ` <CGME20210324065516epcas5p450958011b69f5941e8f2bc993b82b904@epcas5p4.samsung.com> @ 2021-03-24 6:54 ` Maninder Singh 0 siblings, 0 replies; 10+ messages in thread From: Maninder Singh @ 2021-03-24 6:54 UTC (permalink / raw) To: catalin.marinas, will, pcc, mark.rutland, akpm, 0x7f454c46, amit.kachhap, james.morse, vincenzo.frascino, nsaenzjulienne, keescook, samitolvanen Cc: linux-arm-kernel, linux-kernel, a.sahrawat, Maninder Singh, Vaneet Narang In case of a use after free kernel OOPs, freed path of the object is required to debug futher. In most of cases the object address is present in one of the registers. Thus check the register's address and if it belongs to slab, print its alloc and free path. commit a02a25709155 ("mm/slub: add support for free path information of an object") provides free path along with alloc path of object in mem_dump_obj(). Thus call it with register values same as in ARM with commit 14c0508adcdb ("arm: print alloc free paths for address in registers") e.g. in the below issue register x20 belongs to slab, and a use after free issue occurred on one of its dereferenced values: [ 19.516507] Unable to handle kernel paging request at virtual address 006b6b6b6b6b6b73 .. .. [ 19.528784] Register x10 information: 0-page vmalloc region starting at 0xffff800011bb0000 allocated at paging_init+0x1d8/0x544 [ 19.529143] Register x11 information: 0-page vmalloc region starting at 0xffff800011bb0000 allocated at paging_init+0x1d8/0x544 [ 19.529513] Register x12 information: non-paged memory .. [ 19.544953] Register x20 information: slab kmalloc-128 start ffff0000c3a34280 data offset 128 pointer offset 0 size 128 allocated at meminfo_proc_show+0x44/0x588 [ 19.545432] ___slab_alloc+0x638/0x658 [ 19.545576] __slab_alloc.isra.0+0x2c/0x58 [ 19.545728] kmem_cache_alloc+0x584/0x598 [ 19.545877] meminfo_proc_show+0x44/0x588 [ 19.546022] seq_read_iter+0x258/0x460 [ 19.546160] proc_reg_read_iter+0x90/0xd0 [ 19.546308] generic_file_splice_read+0xd0/0x188 [ 19.546474] do_splice_to+0x90/0xe0 [ 19.546609] splice_direct_to_actor+0xbc/0x240 [ 19.546768] do_splice_direct+0x8c/0xe8 [ 19.546911] do_sendfile+0x2c4/0x500 [ 19.547048] __arm64_sys_sendfile64+0x160/0x168 [ 19.547205] el0_svc_common.constprop.0+0x60/0x120 [ 19.547377] do_el0_svc_compat+0x1c/0x40 [ 19.547524] el0_svc_compat+0x24/0x38 [ 19.547660] el0_sync_compat_handler+0x90/0x158 [ 19.547821] Free path: [ 19.547906] __slab_free+0x3dc/0x538 [ 19.548051] kfree+0x2d8/0x310 [ 19.548176] meminfo_proc_show+0x60/0x588 [ 19.548322] seq_read_iter+0x258/0x460 [ 19.548459] proc_reg_read_iter+0x90/0xd0 [ 19.548602] generic_file_splice_read+0xd0/0x188 [ 19.548761] do_splice_to+0x90/0xe0 [ 19.548889] splice_direct_to_actor+0xbc/0x240 [ 19.549040] do_splice_direct+0x8c/0xe8 [ 19.549183] do_sendfile+0x2c4/0x500 [ 19.549319] __arm64_sys_sendfile64+0x160/0x168 [ 19.549477] el0_svc_common.constprop.0+0x60/0x120 [ 19.549646] do_el0_svc_compat+0x1c/0x40 [ 19.549782] el0_svc_compat+0x24/0x38 [ 19.549913] el0_sync_compat_handler+0x90/0x158 [ 19.550067] el0_sync_compat+0x174/0x180 .. Signed-off-by: Vaneet Narang <v.narang@samsung.com> Signed-off-by: Maninder Singh <maninder1.s@samsung.com> --- arch/arm64/include/asm/system_misc.h | 1 + arch/arm64/kernel/process.c | 11 +++++++++++ arch/arm64/kernel/traps.c | 1 + 3 files changed, 13 insertions(+) diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h index 673be2d1263c..84d5204cdb80 100644 --- a/arch/arm64/include/asm/system_misc.h +++ b/arch/arm64/include/asm/system_misc.h @@ -31,6 +31,7 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned int, struct mm_struct; extern void __show_regs(struct pt_regs *); +extern void __show_regs_alloc_free(struct pt_regs *regs); extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 6e60aa3b5ea9..d0d0ada332c3 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -306,6 +306,17 @@ void __show_regs(struct pt_regs *regs) } } +void __show_regs_alloc_free(struct pt_regs *regs) +{ + int i; + + /* check for x0 - x29 only */ + for (i = 0; i <= 29; i++) { + pr_alert("Register x%d information:", i); + mem_dump_obj((void *)regs->regs[i]); + } +} + void show_regs(struct pt_regs *regs) { __show_regs(regs); diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index a05d34f0e82a..cb4858c6e57b 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -104,6 +104,7 @@ static int __die(const char *str, int err, struct pt_regs *regs) print_modules(); show_regs(regs); + __show_regs_alloc_free(regs); dump_kernel_instr(KERN_EMERG, regs); -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] arm64: print alloc free paths for address in registers @ 2021-03-24 6:54 ` Maninder Singh 0 siblings, 0 replies; 10+ messages in thread From: Maninder Singh @ 2021-03-24 6:54 UTC (permalink / raw) To: catalin.marinas, will, pcc, mark.rutland, akpm, 0x7f454c46, amit.kachhap, james.morse, vincenzo.frascino, nsaenzjulienne, keescook, samitolvanen Cc: linux-arm-kernel, linux-kernel, a.sahrawat, Maninder Singh, Vaneet Narang In case of a use after free kernel OOPs, freed path of the object is required to debug futher. In most of cases the object address is present in one of the registers. Thus check the register's address and if it belongs to slab, print its alloc and free path. commit a02a25709155 ("mm/slub: add support for free path information of an object") provides free path along with alloc path of object in mem_dump_obj(). Thus call it with register values same as in ARM with commit 14c0508adcdb ("arm: print alloc free paths for address in registers") e.g. in the below issue register x20 belongs to slab, and a use after free issue occurred on one of its dereferenced values: [ 19.516507] Unable to handle kernel paging request at virtual address 006b6b6b6b6b6b73 .. .. [ 19.528784] Register x10 information: 0-page vmalloc region starting at 0xffff800011bb0000 allocated at paging_init+0x1d8/0x544 [ 19.529143] Register x11 information: 0-page vmalloc region starting at 0xffff800011bb0000 allocated at paging_init+0x1d8/0x544 [ 19.529513] Register x12 information: non-paged memory .. [ 19.544953] Register x20 information: slab kmalloc-128 start ffff0000c3a34280 data offset 128 pointer offset 0 size 128 allocated at meminfo_proc_show+0x44/0x588 [ 19.545432] ___slab_alloc+0x638/0x658 [ 19.545576] __slab_alloc.isra.0+0x2c/0x58 [ 19.545728] kmem_cache_alloc+0x584/0x598 [ 19.545877] meminfo_proc_show+0x44/0x588 [ 19.546022] seq_read_iter+0x258/0x460 [ 19.546160] proc_reg_read_iter+0x90/0xd0 [ 19.546308] generic_file_splice_read+0xd0/0x188 [ 19.546474] do_splice_to+0x90/0xe0 [ 19.546609] splice_direct_to_actor+0xbc/0x240 [ 19.546768] do_splice_direct+0x8c/0xe8 [ 19.546911] do_sendfile+0x2c4/0x500 [ 19.547048] __arm64_sys_sendfile64+0x160/0x168 [ 19.547205] el0_svc_common.constprop.0+0x60/0x120 [ 19.547377] do_el0_svc_compat+0x1c/0x40 [ 19.547524] el0_svc_compat+0x24/0x38 [ 19.547660] el0_sync_compat_handler+0x90/0x158 [ 19.547821] Free path: [ 19.547906] __slab_free+0x3dc/0x538 [ 19.548051] kfree+0x2d8/0x310 [ 19.548176] meminfo_proc_show+0x60/0x588 [ 19.548322] seq_read_iter+0x258/0x460 [ 19.548459] proc_reg_read_iter+0x90/0xd0 [ 19.548602] generic_file_splice_read+0xd0/0x188 [ 19.548761] do_splice_to+0x90/0xe0 [ 19.548889] splice_direct_to_actor+0xbc/0x240 [ 19.549040] do_splice_direct+0x8c/0xe8 [ 19.549183] do_sendfile+0x2c4/0x500 [ 19.549319] __arm64_sys_sendfile64+0x160/0x168 [ 19.549477] el0_svc_common.constprop.0+0x60/0x120 [ 19.549646] do_el0_svc_compat+0x1c/0x40 [ 19.549782] el0_svc_compat+0x24/0x38 [ 19.549913] el0_sync_compat_handler+0x90/0x158 [ 19.550067] el0_sync_compat+0x174/0x180 .. Signed-off-by: Vaneet Narang <v.narang@samsung.com> Signed-off-by: Maninder Singh <maninder1.s@samsung.com> --- arch/arm64/include/asm/system_misc.h | 1 + arch/arm64/kernel/process.c | 11 +++++++++++ arch/arm64/kernel/traps.c | 1 + 3 files changed, 13 insertions(+) diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h index 673be2d1263c..84d5204cdb80 100644 --- a/arch/arm64/include/asm/system_misc.h +++ b/arch/arm64/include/asm/system_misc.h @@ -31,6 +31,7 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned int, struct mm_struct; extern void __show_regs(struct pt_regs *); +extern void __show_regs_alloc_free(struct pt_regs *regs); extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 6e60aa3b5ea9..d0d0ada332c3 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -306,6 +306,17 @@ void __show_regs(struct pt_regs *regs) } } +void __show_regs_alloc_free(struct pt_regs *regs) +{ + int i; + + /* check for x0 - x29 only */ + for (i = 0; i <= 29; i++) { + pr_alert("Register x%d information:", i); + mem_dump_obj((void *)regs->regs[i]); + } +} + void show_regs(struct pt_regs *regs) { __show_regs(regs); diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index a05d34f0e82a..cb4858c6e57b 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -104,6 +104,7 @@ static int __die(const char *str, int err, struct pt_regs *regs) print_modules(); show_regs(regs); + __show_regs_alloc_free(regs); dump_kernel_instr(KERN_EMERG, regs); -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] arm64: print alloc free paths for address in registers 2021-03-24 6:54 ` Maninder Singh @ 2021-03-24 11:51 ` Mark Rutland -1 siblings, 0 replies; 10+ messages in thread From: Mark Rutland @ 2021-03-24 11:51 UTC (permalink / raw) To: Maninder Singh Cc: catalin.marinas, will, pcc, akpm, 0x7f454c46, amit.kachhap, james.morse, vincenzo.frascino, nsaenzjulienne, keescook, samitolvanen, linux-arm-kernel, linux-kernel, a.sahrawat, Vaneet Narang Hi, On Wed, Mar 24, 2021 at 12:24:59PM +0530, Maninder Singh wrote: > In case of a use after free kernel OOPs, freed path of the object is > required to debug futher. In most of cases the object address is present > in one of the registers. > > Thus check the register's address and if it belongs to slab, print its > alloc and free path. This path is used for a number of failures that might have nothing to do with a use-after-free, and from the trimmed example below it looks like this could significantly bloat the panic and potentially cause important information to be lost from the log, especially given the large number of GPRs arm64 has. Given that, I suspect this is not something we want enabled by default. When is this logging enabled? I assume the kernel doesn't always record the alloc/free paths. Is there a boot-time option to control this? How many lines does this produce on average? > commit a02a25709155 ("mm/slub: add support for free path information of an object") > provides free path along with alloc path of object in mem_dump_obj(). > > Thus call it with register values same as in ARM with > commit 14c0508adcdb ("arm: print alloc free paths for address in registers") > > e.g. in the below issue register x20 belongs to slab, and a use after free > issue occurred on one of its dereferenced values: > > [ 19.516507] Unable to handle kernel paging request at virtual address 006b6b6b6b6b6b73 > .. > .. > [ 19.528784] Register x10 information: 0-page vmalloc region starting at 0xffff800011bb0000 allocated at paging_init+0x1d8/0x544 > [ 19.529143] Register x11 information: 0-page vmalloc region starting at 0xffff800011bb0000 allocated at paging_init+0x1d8/0x544 > [ 19.529513] Register x12 information: non-paged memory > .. > [ 19.544953] Register x20 information: slab kmalloc-128 start ffff0000c3a34280 data offset 128 pointer offset 0 size 128 allocated at meminfo_proc_show+0x44/0x588 > [ 19.545432] ___slab_alloc+0x638/0x658 > [ 19.545576] __slab_alloc.isra.0+0x2c/0x58 > [ 19.545728] kmem_cache_alloc+0x584/0x598 > [ 19.545877] meminfo_proc_show+0x44/0x588 > [ 19.546022] seq_read_iter+0x258/0x460 > [ 19.546160] proc_reg_read_iter+0x90/0xd0 > [ 19.546308] generic_file_splice_read+0xd0/0x188 > [ 19.546474] do_splice_to+0x90/0xe0 > [ 19.546609] splice_direct_to_actor+0xbc/0x240 > [ 19.546768] do_splice_direct+0x8c/0xe8 > [ 19.546911] do_sendfile+0x2c4/0x500 > [ 19.547048] __arm64_sys_sendfile64+0x160/0x168 > [ 19.547205] el0_svc_common.constprop.0+0x60/0x120 > [ 19.547377] do_el0_svc_compat+0x1c/0x40 > [ 19.547524] el0_svc_compat+0x24/0x38 > [ 19.547660] el0_sync_compat_handler+0x90/0x158 > [ 19.547821] Free path: > [ 19.547906] __slab_free+0x3dc/0x538 > [ 19.548051] kfree+0x2d8/0x310 > [ 19.548176] meminfo_proc_show+0x60/0x588 > [ 19.548322] seq_read_iter+0x258/0x460 > [ 19.548459] proc_reg_read_iter+0x90/0xd0 > [ 19.548602] generic_file_splice_read+0xd0/0x188 > [ 19.548761] do_splice_to+0x90/0xe0 > [ 19.548889] splice_direct_to_actor+0xbc/0x240 > [ 19.549040] do_splice_direct+0x8c/0xe8 > [ 19.549183] do_sendfile+0x2c4/0x500 > [ 19.549319] __arm64_sys_sendfile64+0x160/0x168 > [ 19.549477] el0_svc_common.constprop.0+0x60/0x120 > [ 19.549646] do_el0_svc_compat+0x1c/0x40 > [ 19.549782] el0_svc_compat+0x24/0x38 > [ 19.549913] el0_sync_compat_handler+0x90/0x158 > [ 19.550067] el0_sync_compat+0x174/0x180 > .. > > Signed-off-by: Vaneet Narang <v.narang@samsung.com> > Signed-off-by: Maninder Singh <maninder1.s@samsung.com> > --- > arch/arm64/include/asm/system_misc.h | 1 + > arch/arm64/kernel/process.c | 11 +++++++++++ > arch/arm64/kernel/traps.c | 1 + > 3 files changed, 13 insertions(+) > > diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h > index 673be2d1263c..84d5204cdb80 100644 > --- a/arch/arm64/include/asm/system_misc.h > +++ b/arch/arm64/include/asm/system_misc.h > @@ -31,6 +31,7 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned int, > > struct mm_struct; > extern void __show_regs(struct pt_regs *); > +extern void __show_regs_alloc_free(struct pt_regs *regs); > > extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index 6e60aa3b5ea9..d0d0ada332c3 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -306,6 +306,17 @@ void __show_regs(struct pt_regs *regs) > } > } > > +void __show_regs_alloc_free(struct pt_regs *regs) > +{ > + int i; > + > + /* check for x0 - x29 only */ Why x29? The AAPCS says that's the frame pointer, so much like the SP it shouldn't point to a heap object. > + for (i = 0; i <= 29; i++) { > + pr_alert("Register x%d information:", i); > + mem_dump_obj((void *)regs->regs[i]); > + } > +} The pr_alert() is unconditional -- can mem_dumpo_obj() never be disabled? What loglevel does mem_dump_obj() use? Generally we try to keep that matched, so I'm surprised it isn't taken as a parameter. > + > void show_regs(struct pt_regs *regs) > { > __show_regs(regs); > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > index a05d34f0e82a..cb4858c6e57b 100644 > --- a/arch/arm64/kernel/traps.c > +++ b/arch/arm64/kernel/traps.c > @@ -104,6 +104,7 @@ static int __die(const char *str, int err, struct pt_regs *regs) > > print_modules(); > show_regs(regs); > + __show_regs_alloc_free(regs); As above, I'm not sure this is the right place to put this. We can get here for reasons other than UAF, and I'm sure we can trigger panics via UAF without going via this. THanks, Mark. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] arm64: print alloc free paths for address in registers @ 2021-03-24 11:51 ` Mark Rutland 0 siblings, 0 replies; 10+ messages in thread From: Mark Rutland @ 2021-03-24 11:51 UTC (permalink / raw) To: Maninder Singh Cc: catalin.marinas, will, pcc, akpm, 0x7f454c46, amit.kachhap, james.morse, vincenzo.frascino, nsaenzjulienne, keescook, samitolvanen, linux-arm-kernel, linux-kernel, a.sahrawat, Vaneet Narang Hi, On Wed, Mar 24, 2021 at 12:24:59PM +0530, Maninder Singh wrote: > In case of a use after free kernel OOPs, freed path of the object is > required to debug futher. In most of cases the object address is present > in one of the registers. > > Thus check the register's address and if it belongs to slab, print its > alloc and free path. This path is used for a number of failures that might have nothing to do with a use-after-free, and from the trimmed example below it looks like this could significantly bloat the panic and potentially cause important information to be lost from the log, especially given the large number of GPRs arm64 has. Given that, I suspect this is not something we want enabled by default. When is this logging enabled? I assume the kernel doesn't always record the alloc/free paths. Is there a boot-time option to control this? How many lines does this produce on average? > commit a02a25709155 ("mm/slub: add support for free path information of an object") > provides free path along with alloc path of object in mem_dump_obj(). > > Thus call it with register values same as in ARM with > commit 14c0508adcdb ("arm: print alloc free paths for address in registers") > > e.g. in the below issue register x20 belongs to slab, and a use after free > issue occurred on one of its dereferenced values: > > [ 19.516507] Unable to handle kernel paging request at virtual address 006b6b6b6b6b6b73 > .. > .. > [ 19.528784] Register x10 information: 0-page vmalloc region starting at 0xffff800011bb0000 allocated at paging_init+0x1d8/0x544 > [ 19.529143] Register x11 information: 0-page vmalloc region starting at 0xffff800011bb0000 allocated at paging_init+0x1d8/0x544 > [ 19.529513] Register x12 information: non-paged memory > .. > [ 19.544953] Register x20 information: slab kmalloc-128 start ffff0000c3a34280 data offset 128 pointer offset 0 size 128 allocated at meminfo_proc_show+0x44/0x588 > [ 19.545432] ___slab_alloc+0x638/0x658 > [ 19.545576] __slab_alloc.isra.0+0x2c/0x58 > [ 19.545728] kmem_cache_alloc+0x584/0x598 > [ 19.545877] meminfo_proc_show+0x44/0x588 > [ 19.546022] seq_read_iter+0x258/0x460 > [ 19.546160] proc_reg_read_iter+0x90/0xd0 > [ 19.546308] generic_file_splice_read+0xd0/0x188 > [ 19.546474] do_splice_to+0x90/0xe0 > [ 19.546609] splice_direct_to_actor+0xbc/0x240 > [ 19.546768] do_splice_direct+0x8c/0xe8 > [ 19.546911] do_sendfile+0x2c4/0x500 > [ 19.547048] __arm64_sys_sendfile64+0x160/0x168 > [ 19.547205] el0_svc_common.constprop.0+0x60/0x120 > [ 19.547377] do_el0_svc_compat+0x1c/0x40 > [ 19.547524] el0_svc_compat+0x24/0x38 > [ 19.547660] el0_sync_compat_handler+0x90/0x158 > [ 19.547821] Free path: > [ 19.547906] __slab_free+0x3dc/0x538 > [ 19.548051] kfree+0x2d8/0x310 > [ 19.548176] meminfo_proc_show+0x60/0x588 > [ 19.548322] seq_read_iter+0x258/0x460 > [ 19.548459] proc_reg_read_iter+0x90/0xd0 > [ 19.548602] generic_file_splice_read+0xd0/0x188 > [ 19.548761] do_splice_to+0x90/0xe0 > [ 19.548889] splice_direct_to_actor+0xbc/0x240 > [ 19.549040] do_splice_direct+0x8c/0xe8 > [ 19.549183] do_sendfile+0x2c4/0x500 > [ 19.549319] __arm64_sys_sendfile64+0x160/0x168 > [ 19.549477] el0_svc_common.constprop.0+0x60/0x120 > [ 19.549646] do_el0_svc_compat+0x1c/0x40 > [ 19.549782] el0_svc_compat+0x24/0x38 > [ 19.549913] el0_sync_compat_handler+0x90/0x158 > [ 19.550067] el0_sync_compat+0x174/0x180 > .. > > Signed-off-by: Vaneet Narang <v.narang@samsung.com> > Signed-off-by: Maninder Singh <maninder1.s@samsung.com> > --- > arch/arm64/include/asm/system_misc.h | 1 + > arch/arm64/kernel/process.c | 11 +++++++++++ > arch/arm64/kernel/traps.c | 1 + > 3 files changed, 13 insertions(+) > > diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h > index 673be2d1263c..84d5204cdb80 100644 > --- a/arch/arm64/include/asm/system_misc.h > +++ b/arch/arm64/include/asm/system_misc.h > @@ -31,6 +31,7 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned int, > > struct mm_struct; > extern void __show_regs(struct pt_regs *); > +extern void __show_regs_alloc_free(struct pt_regs *regs); > > extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index 6e60aa3b5ea9..d0d0ada332c3 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -306,6 +306,17 @@ void __show_regs(struct pt_regs *regs) > } > } > > +void __show_regs_alloc_free(struct pt_regs *regs) > +{ > + int i; > + > + /* check for x0 - x29 only */ Why x29? The AAPCS says that's the frame pointer, so much like the SP it shouldn't point to a heap object. > + for (i = 0; i <= 29; i++) { > + pr_alert("Register x%d information:", i); > + mem_dump_obj((void *)regs->regs[i]); > + } > +} The pr_alert() is unconditional -- can mem_dumpo_obj() never be disabled? What loglevel does mem_dump_obj() use? Generally we try to keep that matched, so I'm surprised it isn't taken as a parameter. > + > void show_regs(struct pt_regs *regs) > { > __show_regs(regs); > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > index a05d34f0e82a..cb4858c6e57b 100644 > --- a/arch/arm64/kernel/traps.c > +++ b/arch/arm64/kernel/traps.c > @@ -104,6 +104,7 @@ static int __die(const char *str, int err, struct pt_regs *regs) > > print_modules(); > show_regs(regs); > + __show_regs_alloc_free(regs); As above, I'm not sure this is the right place to put this. We can get here for reasons other than UAF, and I'm sure we can trigger panics via UAF without going via this. THanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CGME20210324065516epcas5p450958011b69f5941e8f2bc993b82b904@epcms5p3>]
* RE: [PATCH 2/2] arm64: print alloc free paths for address in registers [not found] ` <CGME20210324065516epcas5p450958011b69f5941e8f2bc993b82b904@epcms5p3> @ 2021-03-24 14:40 ` Maninder Singh 0 siblings, 0 replies; 10+ messages in thread From: Maninder Singh @ 2021-03-24 14:40 UTC (permalink / raw) To: Mark Rutland Cc: catalin.marinas, will, pcc, akpm, 0x7f454c46, amit.kachhap, james.morse, vincenzo.frascino, nsaenzjulienne, keescook, samitolvanen, linux-arm-kernel, linux-kernel, AMIT SAHRAWAT, Vaneet Narang Hi, > >This path is used for a number of failures that might have nothing to do >with a use-after-free, and from the trimmed example below it looks like >this could significantly bloat the panic and potentially cause important >information to be lost from the log, especially given the large number >of GPRs arm64 has. > >Given that, I suspect this is not something we want enabled by default. > Yes it will add a lot of logs in case of normal die also. But it can suggest free and alloc paths which can help in some debugging. >When is this logging enabled? I assume the kernel doesn't always record >the alloc/free paths. Is there a boot-time option to control this? > if SLUB_DEBUG_ON is enabled at build time then it is enabled from boot, otherwise it can be enabled by kernel command line parameter of "slub_debug=u" if SLUB_DEBUG is enabled. >How many lines does this produce on average? > 16 traces at max for alloc and 16 for free path. so in total for slab object 34 lines will be printed, otherwise one line for each object info of vmalloc. >> >> +void __show_regs_alloc_free(struct pt_regs *regs) >> +{ >> + int i; >> + >> + /* check for x0 - x29 only */ > >Why x29? The AAPCS says that's the frame pointer, so much like the SP it >shouldn't point to a heap object. yes x29 can be ignored. > >> + for (i = 0; i <= 29; i++) { >> + pr_alert("Register x%d information:", i); >> + mem_dump_obj((void *)regs->regs[i]); >> + } >> +} > >The pr_alert() is unconditional -- can mem_dumpo_obj() never be >disabled? > mem_dump_obj is dependent on CONFIG_PRINTK >What loglevel does mem_dump_obj() use? Generally we try to keep that >matched, so I'm surprised it isn't taken as a parameter. > loglevel of mem_dump_obj is pr_cont, so it will be with the caller's loglevel /** * mem_dump_obj - Print available provenance information * @object: object for which to find provenance information. * * This function uses pr_cont(), so that the caller is expected to have * printed out whatever preamble is appropriate. But loglvel will be changed to pr_info when it goes for printing of traces. void kmem_dump_obj(void *object) { .... pr_cont("\n"); for (i = 0; i < ARRAY_SIZE(kp.kp_stack); i++) { if (!kp.kp_stack[i]) break; pr_info(" %pS\n", kp.kp_stack[i]); } ... >> + >> void show_regs(struct pt_regs *regs) >> { >> __show_regs(regs); >> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c >> index a05d34f0e82a..cb4858c6e57b 100644 >> --- a/arch/arm64/kernel/traps.c >> +++ b/arch/arm64/kernel/traps.c >> @@ -104,6 +104,7 @@ static int __die(const char *str, int err, struct pt_regs *regs) >> >> print_modules(); >> show_regs(regs); >> + __show_regs_alloc_free(regs); > >As above, I'm not sure this is the right place to put this. We can get >here for reasons other than UAF, and I'm sure we can trigger panics via >UAF without going via this. > Adding call here, because we though in case of use after free __die will be called. due to unhandled page fault of 0x6b6b6 MAGIC value. thats why picked this place. Thanks, Maninder Singh ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] arm64: print alloc free paths for address in registers @ 2021-03-24 14:40 ` Maninder Singh 0 siblings, 0 replies; 10+ messages in thread From: Maninder Singh @ 2021-03-24 14:40 UTC (permalink / raw) To: Mark Rutland Cc: catalin.marinas, will, pcc, akpm, 0x7f454c46, amit.kachhap, james.morse, vincenzo.frascino, nsaenzjulienne, keescook, samitolvanen, linux-arm-kernel, linux-kernel, AMIT SAHRAWAT, Vaneet Narang Hi, > >This path is used for a number of failures that might have nothing to do >with a use-after-free, and from the trimmed example below it looks like >this could significantly bloat the panic and potentially cause important >information to be lost from the log, especially given the large number >of GPRs arm64 has. > >Given that, I suspect this is not something we want enabled by default. > Yes it will add a lot of logs in case of normal die also. But it can suggest free and alloc paths which can help in some debugging. >When is this logging enabled? I assume the kernel doesn't always record >the alloc/free paths. Is there a boot-time option to control this? > if SLUB_DEBUG_ON is enabled at build time then it is enabled from boot, otherwise it can be enabled by kernel command line parameter of "slub_debug=u" if SLUB_DEBUG is enabled. >How many lines does this produce on average? > 16 traces at max for alloc and 16 for free path. so in total for slab object 34 lines will be printed, otherwise one line for each object info of vmalloc. >> >> +void __show_regs_alloc_free(struct pt_regs *regs) >> +{ >> + int i; >> + >> + /* check for x0 - x29 only */ > >Why x29? The AAPCS says that's the frame pointer, so much like the SP it >shouldn't point to a heap object. yes x29 can be ignored. > >> + for (i = 0; i <= 29; i++) { >> + pr_alert("Register x%d information:", i); >> + mem_dump_obj((void *)regs->regs[i]); >> + } >> +} > >The pr_alert() is unconditional -- can mem_dumpo_obj() never be >disabled? > mem_dump_obj is dependent on CONFIG_PRINTK >What loglevel does mem_dump_obj() use? Generally we try to keep that >matched, so I'm surprised it isn't taken as a parameter. > loglevel of mem_dump_obj is pr_cont, so it will be with the caller's loglevel /** * mem_dump_obj - Print available provenance information * @object: object for which to find provenance information. * * This function uses pr_cont(), so that the caller is expected to have * printed out whatever preamble is appropriate. But loglvel will be changed to pr_info when it goes for printing of traces. void kmem_dump_obj(void *object) { .... pr_cont("\n"); for (i = 0; i < ARRAY_SIZE(kp.kp_stack); i++) { if (!kp.kp_stack[i]) break; pr_info(" %pS\n", kp.kp_stack[i]); } ... >> + >> void show_regs(struct pt_regs *regs) >> { >> __show_regs(regs); >> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c >> index a05d34f0e82a..cb4858c6e57b 100644 >> --- a/arch/arm64/kernel/traps.c >> +++ b/arch/arm64/kernel/traps.c >> @@ -104,6 +104,7 @@ static int __die(const char *str, int err, struct pt_regs *regs) >> >> print_modules(); >> show_regs(regs); >> + __show_regs_alloc_free(regs); > >As above, I'm not sure this is the right place to put this. We can get >here for reasons other than UAF, and I'm sure we can trigger panics via >UAF without going via this. > Adding call here, because we though in case of use after free __die will be called. due to unhandled page fault of 0x6b6b6 MAGIC value. thats why picked this place. Thanks, Maninder Singh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: (subset) [PATCH 1/2] arm64/process.c: fix Wmissing-prototypes build warnings 2021-03-24 6:54 ` Maninder Singh @ 2021-03-25 11:19 ` Will Deacon -1 siblings, 0 replies; 10+ messages in thread From: Will Deacon @ 2021-03-25 11:19 UTC (permalink / raw) To: samitolvanen, amit.kachhap, keescook, mark.rutland, 0x7f454c46, Maninder Singh, catalin.marinas, akpm, pcc, james.morse, vincenzo.frascino, nsaenzjulienne Cc: kernel-team, Will Deacon, linux-kernel, linux-arm-kernel, a.sahrawat On Wed, 24 Mar 2021 12:24:58 +0530, Maninder Singh wrote: > function protypes are missed before defination, which > leads to compilation warning with "-Wmissing-prototypes" > flag. > > https://lkml.org/lkml/2021/3/19/840 > > arch/arm64/kernel/process.c:261:6: warning: no previous prototype for '__show_regs' [-Wmissing-prototypes] > 261 | void __show_regs(struct pt_regs *regs) > | ^~~~~~~~~~~ > arch/arm64/kernel/process.c:307:6: warning: no previous prototype for '__show_regs_alloc_free' [-Wmissing-prototypes] > 307 | void __show_regs_alloc_free(struct pt_regs *regs) > | ^~~~~~~~~~~~~~~~~~~~~~ > arch/arm64/kernel/process.c:365:5: warning: no previous prototype for 'arch_dup_task_struct' [-Wmissing-prototypes] > 365 | int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) > | ^~~~~~~~~~~~~~~~~~~~ > arch/arm64/kernel/process.c:546:41: warning: no previous prototype for '__switch_to' [-Wmissing-prototypes] > 546 | __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev, > | ^~~~~~~~~~~ > arch/arm64/kernel/process.c:710:25: warning: no previous prototype for 'arm64_preempt_schedule_irq' [-Wmissing-prototypes] > 710 | asmlinkage void __sched arm64_preempt_schedule_irq(void) > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ Applied first patch ONLY to arm64 (for-next/fixes), thanks! [1/2] arm64/process.c: fix Wmissing-prototypes build warnings https://git.kernel.org/arm64/c/baa96377bc7b Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: (subset) [PATCH 1/2] arm64/process.c: fix Wmissing-prototypes build warnings @ 2021-03-25 11:19 ` Will Deacon 0 siblings, 0 replies; 10+ messages in thread From: Will Deacon @ 2021-03-25 11:19 UTC (permalink / raw) To: samitolvanen, amit.kachhap, keescook, mark.rutland, 0x7f454c46, Maninder Singh, catalin.marinas, akpm, pcc, james.morse, vincenzo.frascino, nsaenzjulienne Cc: kernel-team, Will Deacon, linux-kernel, linux-arm-kernel, a.sahrawat On Wed, 24 Mar 2021 12:24:58 +0530, Maninder Singh wrote: > function protypes are missed before defination, which > leads to compilation warning with "-Wmissing-prototypes" > flag. > > https://lkml.org/lkml/2021/3/19/840 > > arch/arm64/kernel/process.c:261:6: warning: no previous prototype for '__show_regs' [-Wmissing-prototypes] > 261 | void __show_regs(struct pt_regs *regs) > | ^~~~~~~~~~~ > arch/arm64/kernel/process.c:307:6: warning: no previous prototype for '__show_regs_alloc_free' [-Wmissing-prototypes] > 307 | void __show_regs_alloc_free(struct pt_regs *regs) > | ^~~~~~~~~~~~~~~~~~~~~~ > arch/arm64/kernel/process.c:365:5: warning: no previous prototype for 'arch_dup_task_struct' [-Wmissing-prototypes] > 365 | int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) > | ^~~~~~~~~~~~~~~~~~~~ > arch/arm64/kernel/process.c:546:41: warning: no previous prototype for '__switch_to' [-Wmissing-prototypes] > 546 | __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev, > | ^~~~~~~~~~~ > arch/arm64/kernel/process.c:710:25: warning: no previous prototype for 'arm64_preempt_schedule_irq' [-Wmissing-prototypes] > 710 | asmlinkage void __sched arm64_preempt_schedule_irq(void) > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ Applied first patch ONLY to arm64 (for-next/fixes), thanks! [1/2] arm64/process.c: fix Wmissing-prototypes build warnings https://git.kernel.org/arm64/c/baa96377bc7b Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-03-25 11:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20210324065511epcas5p1cb74119660e2d98a381ae69d01b29275@epcas5p1.samsung.com>
2021-03-24 6:54 ` [PATCH 1/2] arm64/process.c: fix Wmissing-prototypes build warnings Maninder Singh
2021-03-24 6:54 ` Maninder Singh
[not found] ` <CGME20210324065516epcas5p450958011b69f5941e8f2bc993b82b904@epcas5p4.samsung.com>
2021-03-24 6:54 ` [PATCH 2/2] arm64: print alloc free paths for address in registers Maninder Singh
2021-03-24 6:54 ` Maninder Singh
2021-03-24 11:51 ` Mark Rutland
2021-03-24 11:51 ` Mark Rutland
[not found] ` <CGME20210324065516epcas5p450958011b69f5941e8f2bc993b82b904@epcms5p3>
2021-03-24 14:40 ` Maninder Singh
2021-03-24 14:40 ` Maninder Singh
2021-03-25 11:19 ` (subset) [PATCH 1/2] arm64/process.c: fix Wmissing-prototypes build warnings Will Deacon
2021-03-25 11:19 ` Will Deacon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.