Re: [PATCH RFC v3 01/36] stackdepot: check depot_index before accessing the stack slab

[Marco Elver <elver@google.com>]

On Fri, 22 Nov 2019 at 12:26, <glider@google.com> wrote:
>
> Avoid crashes on corrupted stack ids.

Under what circumstances can they be corrupted?

> Signed-off-by: Alexander Potapenko <glider@google.com>
> To: Alexander Potapenko <glider@google.com>
> Cc: Vegard Nossum <vegard.nossum@oracle.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: linux-mm@kvack.org
>
> ---
> v3:
>  - fix the return statement
>
> Change-Id: I0a0b38ed5057090696a2c6ff0be7cfcc24ae6738
> ---
>  lib/stackdepot.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index ed717dd08ff3..0bc6182bc7a6 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -198,9 +198,22 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle,
>                                unsigned long **entries)
>  {
>         union handle_parts parts = { .handle = handle };
> -       void *slab = stack_slabs[parts.slabindex];
> +       void *slab;
>         size_t offset = parts.offset << STACK_ALLOC_ALIGN;
> -       struct stack_record *stack = slab + offset;
> +       struct stack_record *stack;
> +
> +       if (parts.slabindex > depot_index) {
> +               WARN(1, "slab index %d out of bounds (%d) for stack id %08x\n",
> +                       parts.slabindex, depot_index, handle);

On syzbot with panic_on_warn this will crash the kernel. Is this
desirable? Or is a pr_err here more appropriate?


> +               *entries = NULL;
> +               return 0;
> +       }
> +       slab = stack_slabs[parts.slabindex];
> +       stack = slab + offset;
> +       if (!stack) {
> +               entries = NULL;
> +               return 0;
> +       }
>
>         *entries = stack->entries;
>         return stack->size;
> --
> 2.24.0.432.g9d3f5f5b63-goog
>