Re: [PATCH 1/1] stackdepot: ignore junk last entry in case of switch from user mode.

[Dmitry Vyukov]

On Wed, Oct 11, 2017 at 1:24 PM, Maninder Singh <maninder1.s@xxxxxxxxxxx> wrote:
> Issue observed on ARM.
>
> Whenever there is switch from user mode, we end up with invalid last entry
> with some user space address as below:-
>
>  save_stack+0x40/0xec
>  __set_page_owner+0x2c/0x64
> ....
> ....
>  __handle_domain_irq+0x9c/0x130
>  gic_handle_irq+0x40/0x80
>  __irq_usr+0x4c/0x60
>  0xb6507818
>
> So in this case last entry is not valid, which leads to allocated one more
> new frame for stackdepot although having all above frames exactly same.
>
> (It increases depot_index drastically)
>
> So its better to ignore that last frame in case of switch.
>  save_stack+0x40/0xec
>  __set_page_owner+0x2c/0x64
> ....
> ....
>  __handle_domain_irq+0x9c/0x130
>  gic_handle_irq+0x40/0x80
>  __irq_usr+0x4c/0x60
>
> Signed-off-by: Vaneet Narang <v.narang@xxxxxxxxxxx>
> Signed-off-by: Maninder Singh <maninder1.s@xxxxxxxxxxx>
> ---
>  lib/stackdepot.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index f87d138..bb35b2c 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -214,6 +214,11 @@ depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
>         if (unlikely(trace->nr_entries == 0))
>                 goto fast_exit;
>
> +       if (trace->entries[trace->nr_entries - 1] < MODULES_VADDR) {

I agree with general approach. But isn't kernel text below
MODULES_VADDR on e.g. x86_64?

> +               trace->entries[trace->nr_entries - 1] = ULONG_MAX;

Do we need this?

> +               trace->nr_entries--;
> +       }
> +
>         hash = hash_stack(trace->entries, trace->nr_entries);
>         bucket = &stack_table[hash & STACK_HASH_MASK];
>
> --
> 1.9.1
>