linux-kernel - Re: [PATCH v5 10/10] LoongArch/ftrace: Fix unwind state when option func_stack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Fri, 28 Oct 2022 13:27:16 +0800
From:   Qing Zhang <zhangqing@...ngson.cn>
To:     Huacai Chen <chenhuacai@...nel.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>, loongarch@...ts.linux.dev,
        linux-kernel@...r.kernel.org, Jeff Xie <xiehuan09@...il.com>,
        Jinyang He <hejinyang@...ngson.cn>
Subject: Re: [PATCH v5 10/10] LoongArch/ftrace: Fix unwind state when option
 func_stack_trace



On 2022/10/28 上午10:56, Huacai Chen wrote:
> Hi, Qing,
> 
> Can this patch be squashed to a previous one?
>
ok, I'll combine this patch with the previous patch.

Thanks
- Qing

> Huacai
> 
> On Mon, Sep 19, 2022 at 11:19 AM Qing Zhang <zhangqing@...ngson.cn> wrote:
>>
>> Ftrace plays like function head exception, prologue analysis will stop soon
>> because PC is at entry.
>>
>>     90000000004c5a54 <callee>:
>>     90000000004c5a54:    03400000  andi   $zero, $zero, 0x0   ==>move  t0, ra
>>     90000000004c5a58:    03400000  andi   $zero, $zero, 0x0   ==>bl callsite
>> ==>90000000004c5a5c:    02fcc063  addi.d $sp, $sp, -208(0xf30)
>>     ...
>>
>> When encountering ftrace_call, save trace function ra at PT_ERA, save
>> parent ra at PT_R1, At this time, pc is the position after the two nops
>> of callee. There is no conventional prologue operation between this
>> position and the function entry, so we need to reset the first flag to
>> make the caller continue to unwind.
>>
>> testing method:
>>    echo path_openat > ./set_ftrace_filter
>>    echo 1 > ./options/func_stack_trace
>>    echo function > ./current_tracer
>>
>> Reported-by: Jeff Xie <xiehuan09@...il.com>
>> Tested-by: Jinyang He <hejinyang@...ngson.cn>
>> Tested-by: Jeff Xie <xiehuan09@...il.com>
>> Signed-off-by: Qing Zhang <zhangqing@...ngson.cn>
>> ---
>>   arch/loongarch/include/asm/unwind.h     |  2 +-
>>   arch/loongarch/kernel/unwind_prologue.c | 35 +++++++++++++++++++++----
>>   2 files changed, 31 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/unwind.h b/arch/loongarch/include/asm/unwind.h
>> index f66b07c3e6a1..f2b52b9ea93d 100644
>> --- a/arch/loongarch/include/asm/unwind.h
>> +++ b/arch/loongarch/include/asm/unwind.h
>> @@ -20,7 +20,7 @@ struct unwind_state {
>>          char type; /* UNWINDER_XXX */
>>          struct stack_info stack_info;
>>          struct task_struct *task;
>> -       bool first, error;
>> +       bool first, error, is_ftrace;
>>          int graph_idx;
>>          unsigned long sp, pc, ra;
>>   };
>> diff --git a/arch/loongarch/kernel/unwind_prologue.c b/arch/loongarch/kernel/unwind_prologue.c
>> index f77f3b6f3f06..4fb4923b68cc 100644
>> --- a/arch/loongarch/kernel/unwind_prologue.c
>> +++ b/arch/loongarch/kernel/unwind_prologue.c
>> @@ -14,9 +14,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
>>
>>          if (unwind_done(state))
>>                  return 0;
>> -       else if (state->type)
>> -               return state->pc;
>> -       else if (state->first)
>> +       else if (state->type || state->first)
>>                  return state->pc;
>>
>>          return *(unsigned long *)(state->sp);
>> @@ -42,16 +40,41 @@ static bool unwind_by_guess(struct unwind_state *state)
>>          return false;
>>   }
>>
>> +static inline void unwind_state_fixup(struct unwind_state *state)
>> +{
>> +#ifdef CONFIG_FUNCTION_TRACER
>> +       static unsigned long ftrace_case = (unsigned long)ftrace_call + 4;
>> +
>> +       if (state->pc == ftrace_case)
>> +               state->is_ftrace = true;
>> +#endif
>> +}
>> +
>>   static bool unwind_by_prologue(struct unwind_state *state)
>>   {
>>          struct stack_info *info = &state->stack_info;
>>          union loongarch_instruction *ip, *ip_end;
>>          unsigned long frame_size = 0, frame_ra = -1;
>>          unsigned long size, offset, pc = state->pc;
>> +       struct pt_regs *regs;
>>
>>          if (state->sp >= info->end || state->sp < info->begin)
>>                  return false;
>>
>> +       if (state->is_ftrace) {
>> +               /*
>> +                * As we meet ftrace_regs_entry, reset first flag like first doing
>> +                * tracing, Prologue analysis will stop soon because PC is at entry.
>> +                */
>> +               regs = (struct pt_regs *)state->sp;
>> +               state->pc = regs->csr_era;
>> +               state->ra = regs->regs[1];
>> +               state->sp = regs->regs[3];
>> +               state->first = true;
>> +               state->is_ftrace = false;
>> +               return true;
>> +       }
>> +
>>          if (!kallsyms_lookup_size_offset(pc, &size, &offset))
>>                  return false;
>>
>> @@ -97,7 +120,7 @@ static bool unwind_by_prologue(struct unwind_state *state)
>>
>>          state->pc = *(unsigned long *)(state->sp + frame_ra);
>>          state->sp = state->sp + frame_size;
>> -       return !!__kernel_text_address(state->pc);
>> +       goto out;
>>
>>   first:
>>          state->first = false;
>> @@ -106,7 +129,9 @@ static bool unwind_by_prologue(struct unwind_state *state)
>>
>>          state->pc = state->ra;
>>
>> -       return !!__kernel_text_address(state->ra);
>> +out:
>> +       unwind_state_fixup(state);
>> +       return !!__kernel_text_address(state->pc);
>>   }
>>
>>   void unwind_start(struct unwind_state *state, struct task_struct *task,
>> --
>> 2.36.1
>>