From patchwork Wed Nov 18 06:43:08 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: AKASHI Takahiro X-Patchwork-Id: 56914 Delivered-To: patch@linaro.org Received: by 10.112.155.196 with SMTP id vy4csp2383094lbb; Tue, 17 Nov 2015 22:47:51 -0800 (PST) X-Received: by 10.68.248.102 with SMTP id yl6mr70049572pbc.10.1447829270352; Tue, 17 Nov 2015 22:47:50 -0800 (PST) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id kn10si656302pbd.134.2015.11.17.22.47.50 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 17 Nov 2015 22:47:50 -0800 (PST) Received-SPF: pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org; dkim=neutral (body hash did not verify) header.i=@linaro_org.20150623.gappssmtp.com Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZywVF-000518-Vw; Wed, 18 Nov 2015 06:46:02 +0000 Received: from mail-pa0-x22d.google.com ([2607:f8b0:400e:c03::22d]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZywUj-0003Xh-7m for linux-arm-kernel@lists.infradead.org; Wed, 18 Nov 2015 06:45:38 +0000 Received: by padhx2 with SMTP id hx2so35017134pad.1 for ; Tue, 17 Nov 2015 22:45:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro_org.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=JvHj/7p1WNiby8cgPsCdmXTaO7z0H1XieVD5VOY4bmo=; b=WgnoT3IGw1zvb2jsfMvfNPQksry3UP5cYkOqEv2SmvH90BydTXqro3yo/xa1JT1MOg Wl7e6sVUO2Fx6J++to8sb+Atk39tM1u7MnEOXtRHLKcV+p5E3xS5VW6WgIvhmbj2Fow0 3YzIT1/AQbrP+uSHRKjpqEkc4rcD4tRo/fnCHyl+JjLqTajQwMpKePucL7x5BLB42US/ 1Q29HLnwRg7gmcj9TTkFU5InzDK6jCaNi2bzmsmoMt4giY4KWIIqHdzeYNL55+LMa8Q6 VbVmybntIg8rv9gmvhkP1SwVAC29SiyxIw4o+I9d/6U8HBp9QyJN3DeI5qtgNcxPQOgx Xj0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=JvHj/7p1WNiby8cgPsCdmXTaO7z0H1XieVD5VOY4bmo=; b=CZila/MPMBo4nkEGVYl/rIOKnweNpjLDZCfWiNRUuvnHFsTEgzeuSJNomhCR6dDk3f /eeAtO9dNEk58HmzQRc9OWW0nLV3hDQGwqkSqPgTBe+Go3aC30u3h33wTqLK2NttlOo5 UW94xERD37gL8e8D/UJlr5PLQKhb70lOUp/XROCOXrusiZNQGYQiOCSpIVbi8PPwIV// hOY2jkYAh97rxP7jJBM++u+NznvAjdRwS/GkMpOEcAIPzTQ4uqoErNsrbl/vDm1J4p1W bNcN43yYwtYKpFnnDbo49lIhe+Uqc8mVtlh8zPvrdnRB3Nf7+chmvLiM6dovB9//ypPe f/pA== X-Gm-Message-State: ALoCoQmoTykUz2wii+5yIC2eXoZdctb72OJ50HCmZwwN5F9WCVUtBmgvRR81H9SR3lnSamcsc9+Q X-Received: by 10.66.102.4 with SMTP id fk4mr57421297pab.85.1447829108136; Tue, 17 Nov 2015 22:45:08 -0800 (PST) Received: from localhost.localdomain (61-205-7-47m5.grp1.mineo.jp. [61.205.7.47]) by smtp.googlemail.com with ESMTPSA id xr8sm1685523pab.26.2015.11.17.22.45.03 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 17 Nov 2015 22:45:06 -0800 (PST) From: AKASHI Takahiro To: catalin.marinas@arm.com, will.deacon@arm.com, rostedt@goodmis.org Subject: [PATCH v6 5/6] arm64: ftrace: add arch-specific stack tracer Date: Wed, 18 Nov 2015 15:43:08 +0900 Message-Id: <1447828989-4980-6-git-send-email-takahiro.akashi@linaro.org> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1447828989-4980-1-git-send-email-takahiro.akashi@linaro.org> References: <1447828989-4980-1-git-send-email-takahiro.akashi@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20151117_224529_789045_5113B0D0 X-CRM114-Status: GOOD ( 30.26 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [2607:f8b0:400e:c03:0:0:0:22d listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: jungseoklee85@gmail.com, AKASHI Takahiro , broonie@kernel.org, yalin.wang2010@gmail.com, david.griego@linaro.org, linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org Background and issues on generic check_stack(): 1) slurping stack Assume that a given function A was invoked, and it was invoked again in another context, then it called another function B which allocated a large size of local variables on the stack, but it has not modified the variable(s) yet. When stack tracer, check_stack(), examines the stack looking for B, then A, we may have a chance to accidentally find a stale, not current, stack frame for A because the old frame might reside on the memory for the variable which has not been overwritten. (issue) The stack_trace output may have stale entries. 2) differences between x86 and arm64 On x86, "call" instruction automatically pushes a return address on the top of the stack and decrements a stack pointer. Then child function allocates its local variables on the stack. On arm64, a child function is responsible for allocating memory for local variables as well as a stack frame, and explicitly pushes a return address (LR) and old frame pointer in its function prologue *after* decreasing a stack pointer. Generic check_stack() recogizes 'idxB,' which is the next address of the location where 'fpB' is found, in the picture below as an estimated stack pointer. This seems to fine with x86, but on arm64, 'idxB' is not appropriate just because it contains child function's "local variables." We should instead use spB, if possible, for better interpretation of func_B's stack usage. LOW | ... | fpA +--------+ func_A (pcA, fpA, spA) | fpB | idxB + - - - -+ | pcB | | ... <----------- static local variables in func_A | ... | and extra function args to func_A spB + - - - -+ | ... <----------- dynamically allocated variables in func_B fpB +--------+ func_B (pcB, fpB, spB) | fpC | idxC + - - - -+ | pcC | | ... <----------- static local variables in func_B | ... | and extra function args to func_B spC + - - - -+ | ... | fpC +--------+ func_C (pcC, fpC, spC) HIGH | | (issue) Stack size for a function in stack_trace output is inaccurate, or rather wrong. It looks as if field is one-line offset against . Depth Size Location (49 entries) ----- ---- -------- 40) 1416 64 path_openat+0x128/0xe00 -> 176 41) 1352 176 do_filp_open+0x74/0xf0 -> 256 42) 1176 256 do_open_execat+0x74/0x1c8 -> 80 43) 920 80 open_exec+0x3c/0x70 -> 32 44) 840 32 load_elf_binary+0x294/0x10c8 Stack tracer on arm64: Our approach in check_stack() is uniqeue in the following points: * analyze a function prologue of a traced function to estimate a more accurate stack pointer value, replacing naive ' + 0x10.' * use walk_stackframe(), instead of slurping stack contents as orignal check_stack() does, to identify a stack frame and a stack index (height) for every callsite. Regarding a function prologue analyzer, there is no guarantee that we can handle all the possible patterns of function prologue as gcc does not use any fixed templates to generate them. 'Instruction scheduling' is another issue here. Nevertheless, the current version will surely cover almost all the cases in the kernel image and give us useful information on stack pointers. So this patch utilizes a function prologue only for stack tracer, and does not affect the behaviors of existing stacktrace functions. Reviewed-by: Jungseok Lee Tested-by: Jungseok Lee Signed-off-by: AKASHI Takahiro --- arch/arm64/include/asm/ftrace.h | 2 +- arch/arm64/include/asm/stacktrace.h | 4 + arch/arm64/kernel/ftrace.c | 64 ++++++++++++ arch/arm64/kernel/stacktrace.c | 190 ++++++++++++++++++++++++++++++++++- 4 files changed, 256 insertions(+), 4 deletions(-) -- 1.7.9.5 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h index 3c60f37..6795219 100644 --- a/arch/arm64/include/asm/ftrace.h +++ b/arch/arm64/include/asm/ftrace.h @@ -26,7 +26,7 @@ struct dyn_arch_ftrace { /* No extra data needed for arm64 */ }; -extern unsigned long ftrace_graph_call; +extern u32 ftrace_graph_call; extern void return_to_handler(void); diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h index 801a16db..0eee008 100644 --- a/arch/arm64/include/asm/stacktrace.h +++ b/arch/arm64/include/asm/stacktrace.h @@ -30,5 +30,9 @@ struct stackframe { extern int unwind_frame(struct task_struct *tsk, struct stackframe *frame); extern void walk_stackframe(struct task_struct *tsk, struct stackframe *frame, int (*fn)(struct stackframe *, void *), void *data); +#ifdef CONFIG_STACK_TRACER +struct stack_trace; +extern void save_stack_trace_sp(struct stack_trace *trace, unsigned long *sp); +#endif #endif /* __ASM_STACKTRACE_H */ diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c index 314f82d..102ed59 100644 --- a/arch/arm64/kernel/ftrace.c +++ b/arch/arm64/kernel/ftrace.c @@ -9,6 +9,7 @@ * published by the Free Software Foundation. */ +#include #include #include #include @@ -16,6 +17,7 @@ #include #include #include +#include #ifdef CONFIG_DYNAMIC_FTRACE /* @@ -173,3 +175,65 @@ int ftrace_disable_ftrace_graph_caller(void) } #endif /* CONFIG_DYNAMIC_FTRACE */ #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ + +#ifdef CONFIG_STACK_TRACER +static unsigned long stack_trace_sp[STACK_TRACE_ENTRIES]; +static unsigned long raw_stack_trace_max_size; + +void check_stack(unsigned long ip, unsigned long *stack) +{ + unsigned long this_size, flags; + unsigned long top; + int i, j; + + this_size = ((unsigned long)stack) & (THREAD_SIZE-1); + this_size = THREAD_SIZE - this_size; + + if (this_size <= raw_stack_trace_max_size) + return; + + /* we do not handle an interrupt stack yet */ + if (!object_is_on_stack(stack)) + return; + + local_irq_save(flags); + arch_spin_lock(&stack_trace_max_lock); + + /* check again */ + if (this_size <= raw_stack_trace_max_size) + goto out; + + /* find out stack frames */ + stack_trace_max.nr_entries = 0; + stack_trace_max.skip = 0; + save_stack_trace_sp(&stack_trace_max, stack_trace_sp); + stack_trace_max.nr_entries--; /* for the last entry ('-1') */ + + /* calculate a stack index for each function */ + top = ((unsigned long)stack & ~(THREAD_SIZE-1)) + THREAD_SIZE; + for (i = 0; i < stack_trace_max.nr_entries; i++) + stack_trace_index[i] = top - stack_trace_sp[i]; + raw_stack_trace_max_size = this_size; + + /* Skip over the overhead of the stack tracer itself */ + for (i = 0; i < stack_trace_max.nr_entries; i++) + if (stack_trace_max.entries[i] == ip) + break; + + stack_trace_max.nr_entries -= i; + for (j = 0; j < stack_trace_max.nr_entries; j++) { + stack_trace_index[j] = stack_trace_index[j + i]; + stack_trace_max.entries[j] = stack_trace_max.entries[j + i]; + } + stack_trace_max_size = stack_trace_index[0]; + + if (task_stack_end_corrupted(current)) { + WARN(1, "task stack is corrupted.\n"); + stack_trace_print(); + } + + out: + arch_spin_unlock(&stack_trace_max_lock); + local_irq_restore(flags); +} +#endif /* CONFIG_STACK_TRACER */ diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index 0a39049..1d18bc4 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -24,6 +24,149 @@ #include #include +#ifdef CONFIG_STACK_TRACER +/* + * This function parses a function prologue of a traced function and + * determines its stack size. + * A return value indicates a location of @pc in a function prologue. + * @return value: + * + * 1: + * sub sp, sp, #XX sub sp, sp, #XX + * 2: + * stp x29, x30, [sp, #YY] stp x29, x30, [sp, #--ZZ]! + * 3: + * add x29, sp, #YY mov x29, sp + * 0: + * + * + * 1: + * stp x29, x30, [sp, #-XX]! + * 3: + * mov x29, sp + * 0: + * + * @size: sp offset from calller's sp (XX or XX + ZZ) + * @size2: fp offset from new sp (YY or 0) + */ +static int analyze_function_prologue(unsigned long pc, + unsigned long *size, unsigned long *size2) +{ + unsigned long offset; + u32 *addr, insn; + int pos = -1; + enum aarch64_insn_register src, dst, reg1, reg2, base; + int imm; + enum aarch64_insn_variant variant; + enum aarch64_insn_adsb_type adsb_type; + enum aarch64_insn_ldst_type ldst_type; + + *size = *size2 = 0; + + if (!pc) + goto out; + + if (unlikely(!kallsyms_lookup_size_offset(pc, NULL, &offset))) + goto out; + + addr = (u32 *)(pc - offset); +#ifdef CONFIG_FUNCTION_GRAPH_TRACER + if (addr == (u32 *)ftrace_graph_caller) +#ifdef CONFIG_DYNAMIC_FTRACE + addr = (u32 *)ftrace_caller; +#else + addr = (u32 *)_mcount; +#endif + else +#endif +#ifdef CONFIG_DYNAMIC_FTRACE + if (addr == (u32 *)ftrace_call) + addr = (u32 *)ftrace_caller; +#ifdef CONFIG_FUNCTION_GRAPH_TRACER + else if (addr == &ftrace_graph_call) + addr = (u32 *)ftrace_caller; +#endif +#endif + + insn = le32_to_cpu(*addr); + pos = 1; + + /* analyze a function prologue */ + while ((unsigned long)addr < pc) { + if (aarch64_insn_is_branch_imm(insn) || + aarch64_insn_is_br(insn) || + aarch64_insn_is_blr(insn) || + aarch64_insn_is_ret(insn) || + aarch64_insn_is_eret(insn)) + /* exiting a basic block */ + goto out; + + if (aarch64_insn_decode_add_sub_imm(insn, &dst, &src, + &imm, &variant, &adsb_type)) { + if ((adsb_type == AARCH64_INSN_ADSB_SUB) && + (dst == AARCH64_INSN_REG_SP) && + (src == AARCH64_INSN_REG_SP)) { + /* + * Starting the following sequence: + * sub sp, sp, #xx + * stp x29, x30, [sp, #yy] + * add x29, sp, #yy + */ + WARN_ON(pos != 1); + pos = 2; + *size += imm; + } else if ((adsb_type == AARCH64_INSN_ADSB_ADD) && + (dst == AARCH64_INSN_REG_29) && + (src == AARCH64_INSN_REG_SP)) { + /* + * add x29, sp, #yy + * or + * mov x29, sp + */ + WARN_ON(pos != 3); + pos = 0; + *size2 = imm; + + break; + } + } else if (aarch64_insn_decode_load_store_pair(insn, + ®1, ®2, &base, &imm, + &variant, &ldst_type)) { + if ((ldst_type == + AARCH64_INSN_LDST_STORE_PAIR_PRE_INDEX) && + (reg1 == AARCH64_INSN_REG_29) && + (reg2 == AARCH64_INSN_REG_30) && + (base == AARCH64_INSN_REG_SP)) { + /* + * Starting the following sequence: + * stp x29, x30, [sp, #-xx]! + * mov x29, sp + */ + WARN_ON(!((pos == 1) || (pos == 2))); + pos = 3; + *size += -imm; + } else if ((ldst_type == + AARCH64_INSN_LDST_STORE_PAIR) && + (reg1 == AARCH64_INSN_REG_29) && + (reg2 == AARCH64_INSN_REG_30) && + (base == AARCH64_INSN_REG_SP)) { + /* + * stp x29, x30, [sp, #yy] + */ + WARN_ON(pos != 2); + pos = 3; + } + } + + addr++; + insn = le32_to_cpu(*addr); + } + +out: + return pos; +} +#endif + /* * AArch64 PCS assigns the frame pointer to x29. * @@ -112,6 +255,9 @@ struct stack_trace_data { struct stack_trace *trace; unsigned int no_sched_functions; unsigned int skip; +#ifdef CONFIG_STACK_TRACER + unsigned long *sp; +#endif }; static int save_trace(struct stackframe *frame, void *d) @@ -127,18 +273,42 @@ static int save_trace(struct stackframe *frame, void *d) return 0; } +#ifdef CONFIG_STACK_TRACER + if (data->sp) { + if (trace->nr_entries) { + unsigned long child_pc, sp_off, fp_off; + int pos; + + child_pc = trace->entries[trace->nr_entries - 1]; + pos = analyze_function_prologue(child_pc, + &sp_off, &fp_off); + /* + * frame->sp - 0x10 is actually a child's fp. + * See above. + */ + data->sp[trace->nr_entries] = (pos < 0 ? frame->sp : + (frame->sp - 0x10) + sp_off - fp_off); + } else { + data->sp[0] = frame->sp; + } + } +#endif trace->entries[trace->nr_entries++] = addr; return trace->nr_entries >= trace->max_entries; } -void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) +static void __save_stack_trace_tsk(struct task_struct *tsk, + struct stack_trace *trace, unsigned long *stack_dump_sp) { struct stack_trace_data data; struct stackframe frame; data.trace = trace; data.skip = trace->skip; +#ifdef CONFIG_STACK_TRACER + data.sp = stack_dump_sp; +#endif if (tsk != current) { data.no_sched_functions = 1; @@ -149,7 +319,8 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) data.no_sched_functions = 0; frame.fp = (unsigned long)__builtin_frame_address(0); frame.sp = current_stack_pointer; - frame.pc = (unsigned long)save_stack_trace_tsk; + asm("1:"); + asm("ldr %0, =1b" : "=r" (frame.pc)); } #ifdef CONFIG_FUNCTION_GRAPH_TRACER frame.graph = tsk->curr_ret_stack; @@ -160,9 +331,22 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) trace->entries[trace->nr_entries++] = ULONG_MAX; } +void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) +{ + __save_stack_trace_tsk(tsk, trace, NULL); +} + void save_stack_trace(struct stack_trace *trace) { - save_stack_trace_tsk(current, trace); + __save_stack_trace_tsk(current, trace, NULL); } EXPORT_SYMBOL_GPL(save_stack_trace); + +#ifdef CONFIG_STACK_TRACER +void save_stack_trace_sp(struct stack_trace *trace, + unsigned long *stack_dump_sp) +{ + __save_stack_trace_tsk(current, trace, stack_dump_sp); +} +#endif /* CONFIG_STACK_TRACER */ #endif