2020-08-07 16:07:29

by Marco Elver

[permalink] [raw]
Subject: Odd-sized kmem_cache_alloc and slub_debug=Z

Hi,

I found that the below debug-code using kmem_cache_alloc(), when using
slub_debug=Z, results in the following crash:

general protection fault, probably for non-canonical address 0xcccccca41caea170: 0000 [#1] PREEMPT SMP PTI
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.8.0+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014
RIP: 0010:freelist_dereference mm/slub.c:272 [inline]
RIP: 0010:get_freepointer mm/slub.c:278 [inline]
RIP: 0010:deactivate_slab+0x54/0x460 mm/slub.c:2111
Code: 8b bc c7 e0 00 00 00 48 85 d2 0f 84 00 01 00 00 49 89 d5 31 c0 48 89 44 24 08 66 66 2e 0f 1f 84 00 00 00 00 00 90 44 8b 43 20 <4b> 8b 44 05 00 48 85 c0 0f 84 1e 01 00 00 4c 89 ed 49 89 c5 8b 43
RSP: 0000:ffffffffa7e03e18 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffffa3a41c972340 RCX: 0000000000000000
RDX: cccccca41caea160 RSI: ffffe7c6a072ba80 RDI: ffffa3a41c972340
RBP: ffffa3a41caea008 R08: 0000000000000010 R09: ffffa3a41caea01d
R10: ffffffffa7f8dc50 R11: ffffffffa68f44c0 R12: ffffa3a41c972340
R13: cccccca41caea160 R14: ffffe7c6a072ba80 R15: ffffa3a41c96d540
FS: 0000000000000000(0000) GS:ffffa3a41fc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffa3a051c01000 CR3: 000000045140a001 CR4: 0000000000770ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 00000000
Call Trace:
___slab_alloc+0x336/0x340 mm/slub.c:2690
__slab_alloc mm/slub.c:2714 [inline]
slab_alloc_node mm/slub.c:2788 [inline]
slab_alloc mm/slub.c:2832 [inline]
kmem_cache_alloc+0x135/0x200 mm/slub.c:2837
start_kernel+0x3d6/0x44e init/main.c:1049
secondary_startup_64+0xb6/0xc0 arch/x86/kernel/head_64.S:243

Any ideas what might be wrong?

This does not crash when redzones are not enabled.

Thanks,
-- Marco

------ >8 ------

diff --git a/init/main.c b/init/main.c
index 15bd0efff3df..f4aa5bb3f2ec 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1041,6 +1041,16 @@ asmlinkage __visible void __init start_kernel(void)
sfi_init_late();
kcsan_init();

+ /* DEBUG CODE */
+ {
+ struct kmem_cache *c = kmem_cache_create("test", 21, 1, 0, NULL);
+ char *buf;
+ BUG_ON(!c);
+ buf = kmem_cache_alloc(c, GFP_KERNEL);
+ kmem_cache_free(c, buf);
+ kmem_cache_destroy(c);
+ }
+
/* Do the rest non-__init'ed, we're now alive */
arch_call_rest_init();


2020-08-07 17:08:26

by Pekka Enberg

[permalink] [raw]
Subject: Re: Odd-sized kmem_cache_alloc and slub_debug=Z

Hi Marco,

On Fri, Aug 7, 2020 at 7:07 PM Marco Elver <[email protected]> wrote:
> I found that the below debug-code using kmem_cache_alloc(), when using
> slub_debug=Z, results in the following crash:
>
> general protection fault, probably for non-canonical address 0xcccccca41caea170: 0000 [#1] PREEMPT SMP PTI
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.8.0+ #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014
> RIP: 0010:freelist_dereference mm/slub.c:272 [inline]
> RIP: 0010:get_freepointer mm/slub.c:278 [inline]
> RIP: 0010:deactivate_slab+0x54/0x460 mm/slub.c:2111
> Code: 8b bc c7 e0 00 00 00 48 85 d2 0f 84 00 01 00 00 49 89 d5 31 c0 48 89 44 24 08 66 66 2e 0f 1f 84 00 00 00 00 00 90 44 8b 43 20 <4b> 8b 44 05 00 48 85 c0 0f 84 1e 01 00 00 4c 89 ed 49 89 c5 8b 43
> RSP: 0000:ffffffffa7e03e18 EFLAGS: 00010046
> RAX: 0000000000000000 RBX: ffffa3a41c972340 RCX: 0000000000000000
> RDX: cccccca41caea160 RSI: ffffe7c6a072ba80 RDI: ffffa3a41c972340
> RBP: ffffa3a41caea008 R08: 0000000000000010 R09: ffffa3a41caea01d
> R10: ffffffffa7f8dc50 R11: ffffffffa68f44c0 R12: ffffa3a41c972340
> R13: cccccca41caea160 R14: ffffe7c6a072ba80 R15: ffffa3a41c96d540
> FS: 0000000000000000(0000) GS:ffffa3a41fc00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffa3a051c01000 CR3: 000000045140a001 CR4: 0000000000770ef0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 00000000
> Call Trace:
> ___slab_alloc+0x336/0x340 mm/slub.c:2690
> __slab_alloc mm/slub.c:2714 [inline]
> slab_alloc_node mm/slub.c:2788 [inline]
> slab_alloc mm/slub.c:2832 [inline]
> kmem_cache_alloc+0x135/0x200 mm/slub.c:2837
> start_kernel+0x3d6/0x44e init/main.c:1049
> secondary_startup_64+0xb6/0xc0 arch/x86/kernel/head_64.S:243
>
> Any ideas what might be wrong?
>
> This does not crash when redzones are not enabled.
>
> Thanks,
> -- Marco
>
> ------ >8 ------
>
> diff --git a/init/main.c b/init/main.c
> index 15bd0efff3df..f4aa5bb3f2ec 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1041,6 +1041,16 @@ asmlinkage __visible void __init start_kernel(void)
> sfi_init_late();
> kcsan_init();
>
> + /* DEBUG CODE */
> + {
> + struct kmem_cache *c = kmem_cache_create("test", 21, 1, 0, NULL);
> + char *buf;
> + BUG_ON(!c);
> + buf = kmem_cache_alloc(c, GFP_KERNEL);
> + kmem_cache_free(c, buf);
> + kmem_cache_destroy(c);
> + }
> +
> /* Do the rest non-__init'ed, we're now alive */
> arch_call_rest_init();

Anything interesting in your .config? The fault does not reproduce
with 5.8.0 + x86-64 defconfig.

- Pekka

2020-08-07 17:17:26

by Kees Cook

[permalink] [raw]
Subject: Re: Odd-sized kmem_cache_alloc and slub_debug=Z

On Fri, Aug 07, 2020 at 06:06:27PM +0200, Marco Elver wrote:
> I found that the below debug-code using kmem_cache_alloc(), when using
> slub_debug=Z, results in the following crash:
>
> general protection fault, probably for non-canonical address 0xcccccca41caea170: 0000 [#1] PREEMPT SMP PTI
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.8.0+ #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014
> RIP: 0010:freelist_dereference mm/slub.c:272 [inline]
> RIP: 0010:get_freepointer mm/slub.c:278 [inline]

That really looks like more fun from my moving the freelist pointer...

> R13: cccccca41caea160 R14: ffffe7c6a072ba80 R15: ffffa3a41c96d540

Except that it's all cccc at the start, which doesn't look like "data"
nor the hardened freelist obfuscation.

> FS: 0000000000000000(0000) GS:ffffa3a41fc00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffa3a051c01000 CR3: 000000045140a001 CR4: 0000000000770ef0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 00000000
> Call Trace:
> ___slab_alloc+0x336/0x340 mm/slub.c:2690
> __slab_alloc mm/slub.c:2714 [inline]
> slab_alloc_node mm/slub.c:2788 [inline]
> slab_alloc mm/slub.c:2832 [inline]
> kmem_cache_alloc+0x135/0x200 mm/slub.c:2837
> start_kernel+0x3d6/0x44e init/main.c:1049
> secondary_startup_64+0xb6/0xc0 arch/x86/kernel/head_64.S:243
>
> Any ideas what might be wrong?
>
> This does not crash when redzones are not enabled.
>
> Thanks,
> -- Marco
>
> ------ >8 ------
>
> diff --git a/init/main.c b/init/main.c
> index 15bd0efff3df..f4aa5bb3f2ec 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1041,6 +1041,16 @@ asmlinkage __visible void __init start_kernel(void)
> sfi_init_late();
> kcsan_init();
>
> + /* DEBUG CODE */
> + {
> + struct kmem_cache *c = kmem_cache_create("test", 21, 1, 0, NULL);
> + char *buf;
> + BUG_ON(!c);
> + buf = kmem_cache_alloc(c, GFP_KERNEL);
> + kmem_cache_free(c, buf);
> + kmem_cache_destroy(c);
> + }
> +
> /* Do the rest non-__init'ed, we're now alive */
> arch_call_rest_init();
>

Which kernel version? Can you send your CONFIG too?

--
Kees Cook

2020-08-07 17:21:02

by Marco Elver

[permalink] [raw]
Subject: Re: Odd-sized kmem_cache_alloc and slub_debug=Z

On Fri, Aug 07, 2020 at 08:06PM +0300, Pekka Enberg wrote:
> Hi Marco,
>
> On Fri, Aug 7, 2020 at 7:07 PM Marco Elver <[email protected]> wrote:
> > I found that the below debug-code using kmem_cache_alloc(), when using
> > slub_debug=Z, results in the following crash:
> >
> > general protection fault, probably for non-canonical address 0xcccccca41caea170: 0000 [#1] PREEMPT SMP PTI
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.8.0+ #1
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014
> > RIP: 0010:freelist_dereference mm/slub.c:272 [inline]
> > RIP: 0010:get_freepointer mm/slub.c:278 [inline]
> > RIP: 0010:deactivate_slab+0x54/0x460 mm/slub.c:2111
> > Code: 8b bc c7 e0 00 00 00 48 85 d2 0f 84 00 01 00 00 49 89 d5 31 c0 48 89 44 24 08 66 66 2e 0f 1f 84 00 00 00 00 00 90 44 8b 43 20 <4b> 8b 44 05 00 48 85 c0 0f 84 1e 01 00 00 4c 89 ed 49 89 c5 8b 43
> > RSP: 0000:ffffffffa7e03e18 EFLAGS: 00010046
> > RAX: 0000000000000000 RBX: ffffa3a41c972340 RCX: 0000000000000000
> > RDX: cccccca41caea160 RSI: ffffe7c6a072ba80 RDI: ffffa3a41c972340
> > RBP: ffffa3a41caea008 R08: 0000000000000010 R09: ffffa3a41caea01d
> > R10: ffffffffa7f8dc50 R11: ffffffffa68f44c0 R12: ffffa3a41c972340
> > R13: cccccca41caea160 R14: ffffe7c6a072ba80 R15: ffffa3a41c96d540
> > FS: 0000000000000000(0000) GS:ffffa3a41fc00000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: ffffa3a051c01000 CR3: 000000045140a001 CR4: 0000000000770ef0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > PKRU: 00000000
> > Call Trace:
> > ___slab_alloc+0x336/0x340 mm/slub.c:2690
> > __slab_alloc mm/slub.c:2714 [inline]
> > slab_alloc_node mm/slub.c:2788 [inline]
> > slab_alloc mm/slub.c:2832 [inline]
> > kmem_cache_alloc+0x135/0x200 mm/slub.c:2837
> > start_kernel+0x3d6/0x44e init/main.c:1049
> > secondary_startup_64+0xb6/0xc0 arch/x86/kernel/head_64.S:243
> >
> > Any ideas what might be wrong?
> >
> > This does not crash when redzones are not enabled.
> >
> > Thanks,
> > -- Marco
> >
> > ------ >8 ------
> >
> > diff --git a/init/main.c b/init/main.c
> > index 15bd0efff3df..f4aa5bb3f2ec 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -1041,6 +1041,16 @@ asmlinkage __visible void __init start_kernel(void)
> > sfi_init_late();
> > kcsan_init();
> >
> > + /* DEBUG CODE */
> > + {
> > + struct kmem_cache *c = kmem_cache_create("test", 21, 1, 0, NULL);
> > + char *buf;
> > + BUG_ON(!c);
> > + buf = kmem_cache_alloc(c, GFP_KERNEL);
> > + kmem_cache_free(c, buf);
> > + kmem_cache_destroy(c);
> > + }
> > +
> > /* Do the rest non-__init'ed, we're now alive */
> > arch_call_rest_init();
>
> Anything interesting in your .config? The fault does not reproduce
> with 5.8.0 + x86-64 defconfig.

It's quite close to defconfig, just some extra options for my test
environment. But none that I'd imagine change this behaviour -- but
maybe I missed something. I've attached my config. Also, just in case,
I'm on mainline from Tuesday: 2324d50d051ec0f14a548e78554fb02513d6dcef.

Thanks,
-- Marco

> - Pekka


Attachments:
(No filename) (3.51 kB)
.config (131.03 kB)
Download all attachments

2020-08-07 17:24:09

by Marco Elver

[permalink] [raw]
Subject: Re: Odd-sized kmem_cache_alloc and slub_debug=Z

On Fri, Aug 07, 2020 at 10:16AM -0700, Kees Cook wrote:
> On Fri, Aug 07, 2020 at 06:06:27PM +0200, Marco Elver wrote:
> > I found that the below debug-code using kmem_cache_alloc(), when using
> > slub_debug=Z, results in the following crash:
> >
> > general protection fault, probably for non-canonical address 0xcccccca41caea170: 0000 [#1] PREEMPT SMP PTI
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.8.0+ #1
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014
> > RIP: 0010:freelist_dereference mm/slub.c:272 [inline]
> > RIP: 0010:get_freepointer mm/slub.c:278 [inline]
>
> That really looks like more fun from my moving the freelist pointer...
>
> > R13: cccccca41caea160 R14: ffffe7c6a072ba80 R15: ffffa3a41c96d540
>
> Except that it's all cccc at the start, which doesn't look like "data"
> nor the hardened freelist obfuscation.
>
> > FS: 0000000000000000(0000) GS:ffffa3a41fc00000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: ffffa3a051c01000 CR3: 000000045140a001 CR4: 0000000000770ef0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > PKRU: 00000000
> > Call Trace:
> > ___slab_alloc+0x336/0x340 mm/slub.c:2690
> > __slab_alloc mm/slub.c:2714 [inline]
> > slab_alloc_node mm/slub.c:2788 [inline]
> > slab_alloc mm/slub.c:2832 [inline]
> > kmem_cache_alloc+0x135/0x200 mm/slub.c:2837
> > start_kernel+0x3d6/0x44e init/main.c:1049
> > secondary_startup_64+0xb6/0xc0 arch/x86/kernel/head_64.S:243
> >
> > Any ideas what might be wrong?
> >
> > This does not crash when redzones are not enabled.
> >
> > Thanks,
> > -- Marco
> >
> > ------ >8 ------
> >
> > diff --git a/init/main.c b/init/main.c
> > index 15bd0efff3df..f4aa5bb3f2ec 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -1041,6 +1041,16 @@ asmlinkage __visible void __init start_kernel(void)
> > sfi_init_late();
> > kcsan_init();
> >
> > + /* DEBUG CODE */
> > + {
> > + struct kmem_cache *c = kmem_cache_create("test", 21, 1, 0, NULL);
> > + char *buf;
> > + BUG_ON(!c);
> > + buf = kmem_cache_alloc(c, GFP_KERNEL);
> > + kmem_cache_free(c, buf);
> > + kmem_cache_destroy(c);
> > + }
> > +
> > /* Do the rest non-__init'ed, we're now alive */
> > arch_call_rest_init();
> >
>
> Which kernel version? Can you send your CONFIG too?

Sorry, didn't see this before I replied to the other -- it's here:
https://lkml.kernel.org/r/[email protected]

Thanks,
-- Marco

2020-08-07 19:07:36

by Pekka Enberg

[permalink] [raw]
Subject: Re: Odd-sized kmem_cache_alloc and slub_debug=Z

Hi Marco and Kees,

On Fri, Aug 07, 2020 at 08:06PM +0300, Pekka Enberg wrote:
> > Anything interesting in your .config? The fault does not reproduce
> > with 5.8.0 + x86-64 defconfig.

On Fri, Aug 7, 2020 at 8:18 PM Marco Elver <[email protected]> wrote:
> It's quite close to defconfig, just some extra options for my test
> environment. But none that I'd imagine change this behaviour -- but
> maybe I missed something. I've attached my config. Also, just in case,
> I'm on mainline from Tuesday: 2324d50d051ec0f14a548e78554fb02513d6dcef.

Yeah, it reproduces with defconfig too, as long as you remember to
pass "slub_debug=Z"... :-/

The following seems to be the culprit:

commit 3202fa62fb43087387c65bfa9c100feffac74aa6
Author: Kees Cook <[email protected]>
Date: Wed Apr 1 21:04:27 2020 -0700

slub: relocate freelist pointer to middle of object

Reverting this commit and one of it's follow up fixes from Kees from
v5.8 makes the issue go away for me. Btw, please note that caches with
size 24 and larger do not trigger this bug, so the issue is that with
small enough object size, we're stomping on allocator metadata (I
assume part of the freelist).

- Pekka

2020-08-17 22:41:56

by Marco Elver

[permalink] [raw]
Subject: Re: Odd-sized kmem_cache_alloc and slub_debug=Z

On Fri, 7 Aug 2020 at 21:06, Pekka Enberg <[email protected]> wrote:
...
> Yeah, it reproduces with defconfig too, as long as you remember to
> pass "slub_debug=Z"... :-/
>
> The following seems to be the culprit:
>
> commit 3202fa62fb43087387c65bfa9c100feffac74aa6
> Author: Kees Cook <[email protected]>
> Date: Wed Apr 1 21:04:27 2020 -0700
>
> slub: relocate freelist pointer to middle of object
>
> Reverting this commit and one of it's follow up fixes from Kees from
> v5.8 makes the issue go away for me. Btw, please note that caches with
> size 24 and larger do not trigger this bug, so the issue is that with
> small enough object size, we're stomping on allocator metadata (I
> assume part of the freelist).

Was there a patch to fix this? Checking, just in case I missed it.

Thanks,
-- Marco

2020-10-09 00:35:16

by Kees Cook

[permalink] [raw]
Subject: Re: Odd-sized kmem_cache_alloc and slub_debug=Z

On Mon, Aug 17, 2020 at 08:31:35PM +0200, Marco Elver wrote:
> On Fri, 7 Aug 2020 at 21:06, Pekka Enberg <[email protected]> wrote:
> ...
> > Yeah, it reproduces with defconfig too, as long as you remember to
> > pass "slub_debug=Z"... :-/
> >
> > The following seems to be the culprit:
> >
> > commit 3202fa62fb43087387c65bfa9c100feffac74aa6
> > Author: Kees Cook <[email protected]>
> > Date: Wed Apr 1 21:04:27 2020 -0700
> >
> > slub: relocate freelist pointer to middle of object
> >
> > Reverting this commit and one of it's follow up fixes from Kees from
> > v5.8 makes the issue go away for me. Btw, please note that caches with
> > size 24 and larger do not trigger this bug, so the issue is that with
> > small enough object size, we're stomping on allocator metadata (I
> > assume part of the freelist).
>
> Was there a patch to fix this? Checking, just in case I missed it.

Hi! I've finally carved out time to look at this, and I've figured it
out. My prior redzoning fix was actually wrong; I'll send a patch to
fix it harder. In the meantime, I've also discovered that the redzoning
code wasn't safe for sizes smaller than sizeof(void *). This oversight
is, I think, what caused me to incorrectly handle the changed offset the
first time. :P

Anyway, patches incoming...

--
Kees Cook