[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAeHK+w-EWDivYTNiUAeSUVZVGOpUyxbbcC8_nMM1=CcpsJ9Ug@mail.gmail.com>
Date: Wed, 13 Feb 2019 11:31:07 +0100
From: Andrey Konovalov <andreyknvl@...gle.com>
To: Qian Cai <cai@....pw>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Christoph Lameter <cl@...ux.com>,
Pekka Enberg <penberg@...nel.org>,
David Rientjes <rientjes@...gle.com>,
Joonsoo Kim <iamjoonsoo.kim@....com>,
Linux Memory Management List <linux-mm@...ck.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] slub: untag object before slab end
On Wed, Feb 13, 2019 at 3:06 AM Qian Cai <cai@....pw> wrote:
>
> get_freepointer() could return NULL if there is no more free objects in
> the slab. However, it could return a tagged pointer (like
> 0x2200000000000000) with KASAN_SW_TAGS which would escape the NULL
> object checking in check_valid_pointer() and trigger errors below, so
> untag the object before checking for a NULL object there.
I think this solution is just masking the issue. get_freepointer()
shouldn't return tagged NULLs. Apparently when we save a freelist
pointer, the object where the pointer gets written is tagged
differently, than this same object when the pointer gets read. I found
one case where this happens (the last patch out my 5 patch series),
but apparently there are more.
>
> [ 35.797667] BUG kmalloc-256 (Not tainted): Freepointer corrupt
> [ 35.803584] -----------------------------------------------------------------------------
> [ 35.803584]
> [ 35.813368] Disabling lock debugging due to kernel taint
> [ 35.818766] INFO: Allocated in build_sched_domains+0x28c/0x495c age=92 cpu=0 pid=1
> [ 35.826443] __kmalloc_node+0x4ac/0x508
> [ 35.830343] build_sched_domains+0x28c/0x495c
> [ 35.834764] sched_init_domains+0x184/0x1d8
> [ 35.839012] sched_init_smp+0x38/0xd4
> [ 35.842732] kernel_init_freeable+0x67c/0x1104
> [ 35.847243] kernel_init+0x18/0x2a4
> [ 35.850790] ret_from_fork+0x10/0x18
> [ 35.854423] INFO: Freed in destroy_sched_domain+0xa0/0xcc age=11 cpu=0 pid=1
> [ 35.861569] destroy_sched_domain+0xa0/0xcc
> [ 35.865814] cpu_attach_domain+0x304/0xb34
> [ 35.869971] build_sched_domains+0x4654/0x495c
> [ 35.874480] sched_init_domains+0x184/0x1d8
> [ 35.878724] sched_init_smp+0x38/0xd4
> [ 35.882443] kernel_init_freeable+0x67c/0x1104
> [ 35.886953] kernel_init+0x18/0x2a4
> [ 35.890495] ret_from_fork+0x10/0x18
> [ 35.894128] INFO: Slab 0x(____ptrval____) objects=85 used=0 fp=0x(____ptrval____) flags=0x7ffffffc000200
> [ 35.903733] INFO: Object 0x(____ptrval____) @offset=38528 fp=0x(____ptrval____)
> [ 35.903733]
> [ 35.912637] Redzone (____ptrval____): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
> [ 35.922155] Redzone (____ptrval____): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
> [ 35.931672] Redzone (____ptrval____): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
> [ 35.941190] Redzone (____ptrval____): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
> [ 35.950707] Redzone (____ptrval____): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
> [ 35.960224] Redzone (____ptrval____): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
> [ 35.969741] Redzone (____ptrval____): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
> [ 35.979258] Redzone (____ptrval____): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
> [ 35.988776] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 35.998206] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.007636] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.017065] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.026494] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.035923] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.045353] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.054783] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.064212] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.073642] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.083071] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.092501] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.101930] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.111359] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.120788] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
> [ 36.130218] Object (____ptrval____): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk.
> [ 36.139647] Redzone (____ptrval____): bb bb bb bb bb bb bb bb ........
> [ 36.148462] Padding (____ptrval____): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
> [ 36.157979] Padding (____ptrval____): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
> [ 36.167496] Padding (____ptrval____): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
> [ 36.177021] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G B 5.0.0-rc6+ #41
> [ 36.184854] Call trace:
> [ 36.187328] dump_backtrace+0x0/0x450
> [ 36.191032] show_stack+0x20/0x2c
> [ 36.194385] __dump_stack+0x20/0x28
> [ 36.197911] dump_stack+0xa0/0xfc
> [ 36.201265] print_trailer+0x1a8/0x1bc
> [ 36.205057] object_err+0x40/0x50
> [ 36.208408] check_object+0x214/0x2b8
> [ 36.212111] __free_slab+0x9c/0x31c
> [ 36.215638] discard_slab+0x78/0xa8
> [ 36.219165] kfree+0x918/0x980
> [ 36.222259] destroy_sched_domain+0xa0/0xcc
> [ 36.226489] cpu_attach_domain+0x304/0xb34
> [ 36.230631] build_sched_domains+0x4654/0x495c
> [ 36.235125] sched_init_domains+0x184/0x1d8
> [ 36.239357] sched_init_smp+0x38/0xd4
> [ 36.243060] kernel_init_freeable+0x67c/0x1104
> [ 36.247555] kernel_init+0x18/0x2a4
> [ 36.251083] ret_from_fork+0x10/0x18
>
> Signed-off-by: Qian Cai <cai@....pw>
> ---
>
> Depends on slub-fix-slab_consistency_checks-kasan_sw_tags.patch.
>
> mm/slub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 4a61959e1887..2fd1cf39914c 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -503,11 +503,11 @@ static inline int check_valid_pointer(struct kmem_cache *s,
> {
> void *base;
>
> + object = kasan_reset_tag(object);
> if (!object)
> return 1;
>
> base = page_address(page);
> - object = kasan_reset_tag(object);
> object = restore_red_left(s, object);
> if (object < base || object >= base + page->objects * s->size ||
> (object - base) % s->size) {
> --
> 2.17.2 (Apple Git-113)
>
Powered by blists - more mailing lists