diff mbox series

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

Message ID 20191122112621.204798-2-glider@google.com (mailing list archive)
State New, archived
Headers show
Series Add KernelMemorySanitizer infrastructure | expand

Commit Message

Alexander Potapenko Nov. 22, 2019, 11:25 a.m. UTC
Avoid crashes on corrupted stack ids.

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(-)

Comments

Marco Elver Nov. 27, 2019, 2:22 p.m. UTC | #1
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
>
diff mbox series

Patch

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);
+		*entries = NULL;
+		return 0;
+	}
+	slab = stack_slabs[parts.slabindex];
+	stack = slab + offset;
+	if (!stack) {
+		entries = NULL;
+		return 0;
+	}
 
 	*entries = stack->entries;
 	return stack->size;