* [PATCH] lib/stackdepot: fix global out-of-bounds in stack_slabs
@ 2020-02-03 10:29 glider
2020-02-03 11:02 ` Tetsuo Handa
0 siblings, 1 reply; 5+ messages in thread
From: glider @ 2020-02-03 10:29 UTC (permalink / raw)
To: walter-zh.wu, dvyukov, gregkh, akpm
Cc: matthias.bgg, tglx, jpoimboe, kstewart, linux-mm, Alexander Potapenko
Walter Wu has reported a potential case in which init_stack_slab() is
called after stack_slabs[STACK_ALLOC_MAX_SLABS - 1] has already been
initialized. In that case init_stack_slab() will overwrite
stack_slabs[STACK_ALLOC_MAX_SLABS], which may result in a memory
corruption.
Fixes: cd11016e5f521 ("mm, kasan: stackdepot implementation. Enable stackdepot for SLAB")
Reported-by: Walter Wu <walter-zh.wu@mediatek.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Alexander Potapenko <glider@google.com>
---
lib/stackdepot.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index ed717dd08ff3..3580e92cac99 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -84,7 +84,9 @@ static bool init_stack_slab(void **prealloc)
if (stack_slabs[depot_index] == NULL) {
stack_slabs[depot_index] = *prealloc;
} else {
- stack_slabs[depot_index + 1] = *prealloc;
+ /* If this is the last depot slab, do not touch the next one. */
+ if (depot_index + 1 < STACK_ALLOC_MAX_SLABS)
+ stack_slabs[depot_index + 1] = *prealloc;
/*
* This smp_store_release pairs with smp_load_acquire() from
* |next_slab_inited| above and in stack_depot_save().
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] lib/stackdepot: fix global out-of-bounds in stack_slabs
2020-02-03 10:29 [PATCH] lib/stackdepot: fix global out-of-bounds in stack_slabs glider
@ 2020-02-03 11:02 ` Tetsuo Handa
2020-02-03 11:10 ` Alexander Potapenko
0 siblings, 1 reply; 5+ messages in thread
From: Tetsuo Handa @ 2020-02-03 11:02 UTC (permalink / raw)
To: glider
Cc: walter-zh.wu, dvyukov, gregkh, akpm, matthias.bgg, tglx,
jpoimboe, kstewart, linux-mm
On 2020/02/03 19:29, glider@google.com wrote:
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -84,7 +84,9 @@ static bool init_stack_slab(void **prealloc)
> if (stack_slabs[depot_index] == NULL) {
> stack_slabs[depot_index] = *prealloc;
> } else {
> - stack_slabs[depot_index + 1] = *prealloc;
> + /* If this is the last depot slab, do not touch the next one. */
> + if (depot_index + 1 < STACK_ALLOC_MAX_SLABS)
> + stack_slabs[depot_index + 1] = *prealloc;
What prevents memory leak (caused by "*prealloc = NULL;")
when we hit depot_index + 1 >= STACK_ALLOC_MAX_SLABS condition?
> /*
> * This smp_store_release pairs with smp_load_acquire() from
> * |next_slab_inited| above and in stack_depot_save().
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] lib/stackdepot: fix global out-of-bounds in stack_slabs
2020-02-03 11:02 ` Tetsuo Handa
@ 2020-02-03 11:10 ` Alexander Potapenko
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Potapenko @ 2020-02-03 11:10 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Walter Wu, Dmitriy Vyukov, Greg Kroah-Hartman, Andrew Morton,
Matthias Brugger, Thomas Gleixner, Josh Poimboeuf, Kate Stewart,
Linux Memory Management List
> > - stack_slabs[depot_index + 1] = *prealloc;
> > + /* If this is the last depot slab, do not touch the next one. */
> > + if (depot_index + 1 < STACK_ALLOC_MAX_SLABS)
> > + stack_slabs[depot_index + 1] = *prealloc;
>
> What prevents memory leak (caused by "*prealloc = NULL;")
> when we hit depot_index + 1 >= STACK_ALLOC_MAX_SLABS condition?
>
Nice catch!
We must return from this function instead of setting *prealloc to NULL.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] lib/stackdepot: fix global out-of-bounds in stack_slabs
@ 2020-02-18 10:29 glider
2020-02-19 21:37 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: glider @ 2020-02-18 10:29 UTC (permalink / raw)
To: walter-zh.wu, dvyukov, gregkh, akpm
Cc: matthias.bgg, tglx, jpoimboe, kstewart, linux-mm, penguin-kernel,
Alexander Potapenko
Walter Wu has reported a potential case in which init_stack_slab() is
called after stack_slabs[STACK_ALLOC_MAX_SLABS - 1] has already been
initialized. In that case init_stack_slab() will overwrite
stack_slabs[STACK_ALLOC_MAX_SLABS], which may result in a memory
corruption.
Fixes: cd11016e5f521 ("mm, kasan: stackdepot implementation. Enable stackdepot for SLAB")
Reported-by: Walter Wu <walter-zh.wu@mediatek.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Alexander Potapenko <glider@google.com>
---
v2:
- prevent a leak of unused preallocated stack slab (spotted by Tetsuo
Handa)
---
lib/stackdepot.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index ed717dd08ff3..81c69c08d1d1 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -83,15 +83,19 @@ static bool init_stack_slab(void **prealloc)
return true;
if (stack_slabs[depot_index] == NULL) {
stack_slabs[depot_index] = *prealloc;
+ *prealloc = NULL;
} else {
- stack_slabs[depot_index + 1] = *prealloc;
+ /* If this is the last depot slab, do not touch the next one. */
+ if (depot_index + 1 < STACK_ALLOC_MAX_SLABS) {
+ stack_slabs[depot_index + 1] = *prealloc;
+ *prealloc = NULL;
+ }
/*
* This smp_store_release pairs with smp_load_acquire() from
* |next_slab_inited| above and in stack_depot_save().
*/
smp_store_release(&next_slab_inited, 1);
}
- *prealloc = NULL;
return true;
}
--
2.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] lib/stackdepot: fix global out-of-bounds in stack_slabs
2020-02-18 10:29 glider
@ 2020-02-19 21:37 ` Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2020-02-19 21:37 UTC (permalink / raw)
To: glider
Cc: walter-zh.wu, dvyukov, gregkh, matthias.bgg, tglx, jpoimboe,
kstewart, linux-mm, penguin-kernel
On Tue, 18 Feb 2020 11:29:50 +0100 glider@google.com wrote:
> Walter Wu has reported a potential case in which init_stack_slab() is
> called after stack_slabs[STACK_ALLOC_MAX_SLABS - 1] has already been
> initialized. In that case init_stack_slab() will overwrite
> stack_slabs[STACK_ALLOC_MAX_SLABS], which may result in a memory
> corruption.
I added cc:stable to this. Please let me know it that is undesirable.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-19 21:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-03 10:29 [PATCH] lib/stackdepot: fix global out-of-bounds in stack_slabs glider
2020-02-03 11:02 ` Tetsuo Handa
2020-02-03 11:10 ` Alexander Potapenko
2020-02-18 10:29 glider
2020-02-19 21:37 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).