diff mbox series

mm/slub: do not place freelist pointer to middle of object if redzone is on

Message ID 20200425091338.24283-1-changbin.du@gmail.com (mailing list archive)
State New, archived
Headers show
Series mm/slub: do not place freelist pointer to middle of object if redzone is on | expand

Commit Message

Changbin Du April 25, 2020, 9:13 a.m. UTC
The recent kernel fails to boot when slub redzone is turned on. This is
caused by commit 3202fa62fb ("slub: relocate freelist pointer to middle of
object") which relocates freelist pointer to middle of object. In this
case, get_track() gets a wrong address and then the redzone is overwritten.

This patch fixes it by relocating freelist pointer after the object, as
what slub posion does.

[    2.390256][    T0] =============================================================================
[    2.392816][    T0] BUG kmem_cache_node (Not tainted): Redzone overwritten
[    2.393735][    T0] -----------------------------------------------------------------------------
[    2.393735][    T0]
[    2.395168][    T0] Disabling lock debugging due to kernel taint
[    2.395923][    T0] INFO: 0xffff88805c000380-0xffff88805c000387 @offset=896. First byte 0x0 instead of 0xbb
[    2.397175][    T0] INFO: Slab 0xffffea0001700000 objects=25 used=25 fp=0x0000000000000000 flags=0xfffffc0010200
[    2.398882][    T0] INFO: Object 0xffff88805c000300 @offset=768 fp=0xffff88805c000580
[    2.398882][    T0]
[    2.400145][    T0] Redzone ffff88805c000280: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................
[    2.401593][    T0] Redzone ffff88805c000290: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................
[    2.403825][    T0] Redzone ffff88805c0002a0: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................
[    2.405853][    T0] Redzone ffff88805c0002b0: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................
[    2.407714][    T0] Redzone ffff88805c0002c0: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................
[    2.411066][    T0] Redzone ffff88805c0002d0: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................
[    2.413818][    T0] Redzone ffff88805c0002e0: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................
[    2.415482][    T0] Redzone ffff88805c0002f0: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................
[    2.416975][    T0] Object ffff88805c000300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.418445][    T0] Object ffff88805c000310: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.420183][    T0] Object ffff88805c000320: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.421911][    T0] Object ffff88805c000330: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.423491][    T0] Object ffff88805c000340: 00 00 00 00 00 00 00 00 e8 ed 4a b9 09 7b 20 83  ..........J..{ .
[    2.425186][    T0] Object ffff88805c000350: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.426901][    T0] Object ffff88805c000360: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.428673][    T0] Object ffff88805c000370: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.430431][    T0] Redzone ffff88805c000380: 00 00 00 00 00 00 00 00                          ........
[    2.485141][    T0] Padding ffff88805c000490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.494198][    T0] Padding ffff88805c0004a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.503094][    T0] Padding ffff88805c0004b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.578370][    T0] Padding ffff88805c0004c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.584420][    T0] Padding ffff88805c0004d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.590139][    T0] Padding ffff88805c0004e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.596915][    T0] Padding ffff88805c0004f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
[    2.604002][    T0] CPU: 0 PID: 0 Comm: swapper Tainted: G    B             5.7.0-rc1+ #53
[    2.608493][    T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
[    2.612959][    T0] Call Trace:
[    2.613953][    T0]  dump_stack+0x8c/0xc0
[    2.615063][    T0]  check_bytes_and_report.cold+0x60/0x95
[    2.616572][    T0]  check_object+0x1c1/0x280
[    2.617462][    T0]  ? pvclock_clocksource_read+0xf6/0x1c0
[    2.618646][    T0]  ? __kmem_cache_create+0x16a/0x670
[    2.619692][    T0]  alloc_debug_processing+0x129/0x170
[    2.620766][    T0]  ___slab_alloc+0x58c/0x630
[    2.621804][    T0]  ? __kmem_cache_create+0x16a/0x670
[    2.622631][    T0]  ? print_unlock_imbalance_bug+0x40/0x40
[    2.623630][    T0]  ? lock_acquire+0x11f/0x200
[    2.624421][    T0]  ? __kmem_cache_create+0x16a/0x670
[    2.625342][    T0]  ? __slab_alloc+0x1c/0x30
[    2.626106][    T0]  ? ___slab_alloc+0x5/0x630
[    2.626890][    T0]  __slab_alloc+0x1c/0x30
[    2.627500][    T0]  kmem_cache_alloc_node+0xab/0x2e0
[    2.628222][    T0]  __kmem_cache_create+0x16a/0x670
[    2.628883][    T0]  ? mem_init_print_info+0x2af/0x2be
[    2.629596][    T0]  create_boot_cache+0xa4/0xc8
[    2.630282][    T0]  kmem_cache_init+0x80/0x14a
[    2.631233][    T0]  start_kernel+0x67a/0xa2a
[    2.631905][    T0]  ? thread_stack_cache_init+0x6/0x6
[    2.632553][    T0]  ? x86_family+0x5/0x20
[    2.633085][    T0]  ? load_ucode_bsp+0x50/0x216
[    2.633629][    T0]  secondary_startup_64+0xa4/0xb0
[    2.634243][    T0] FIX kmem_cache_node: Restoring 0xffff88805c000380-0xffff88805c000387=0xbb
[    2.634243][    T0]
[    2.635532][    T0] FIX kmem_cache_node: Marking all objects used
[    2.636287][    T0] =============================================================================

Fixes: 3202fa62fb ("slub: relocate freelist pointer to middle of object")
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 mm/slub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kees Cook April 25, 2020, 10:48 p.m. UTC | #1
On Sat, Apr 25, 2020 at 05:13:38PM +0800, Changbin Du wrote:
> The recent kernel fails to boot when slub redzone is turned on. This is
> caused by commit 3202fa62fb ("slub: relocate freelist pointer to middle of
> object") which relocates freelist pointer to middle of object. In this
> case, get_track() gets a wrong address and then the redzone is overwritten.

Hi! A fix for this is already in -next:

https://www.ozlabs.org/~akpm/mmotm/broken-out/slub-avoid-redzone-when-choosing-freepointer-location.patch

the above doesn't disable the mitigation when using redzones, so I
prefer that to this suggested solution.
Changbin Du April 26, 2020, 12:07 a.m. UTC | #2
Hi Kees,
On Sat, Apr 25, 2020 at 03:48:31PM -0700, Kees Cook wrote:
> On Sat, Apr 25, 2020 at 05:13:38PM +0800, Changbin Du wrote:
> > The recent kernel fails to boot when slub redzone is turned on. This is
> > caused by commit 3202fa62fb ("slub: relocate freelist pointer to middle of
> > object") which relocates freelist pointer to middle of object. In this
> > case, get_track() gets a wrong address and then the redzone is overwritten.
> 
> Hi! A fix for this is already in -next:
> 
> https://www.ozlabs.org/~akpm/mmotm/broken-out/slub-avoid-redzone-when-choosing-freepointer-location.patch
> 
> the above doesn't disable the mitigation when using redzones, so I
> prefer that to this suggested solution.
>
Glade to see it's been reported. But I am sorry that your patch cannot fix it.

With your fix, I suppose the layout of slub is:
|obj-fp-obj|redzone|track|...

While get_track():
	p = object + s->offset + sizeof(void *);

Then we still get a wrong location. I just tested linux-next and the problem is
still there.

Is the right and left redzone good enough to protect the freepointer? If not,
I will send a patch to fix get_track() along with your patch.

> -- 
> Kees Cook
diff mbox series

Patch

diff --git a/mm/slub.c b/mm/slub.c
index 332d4b459a90..59c5b49038b0 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3570,7 +3570,7 @@  static int calculate_sizes(struct kmem_cache *s, int forced_order)
 	 */
 	s->inuse = size;
 
-	if (((flags & (SLAB_TYPESAFE_BY_RCU | SLAB_POISON)) ||
+	if (((flags & (SLAB_TYPESAFE_BY_RCU | SLAB_RED_ZONE | SLAB_POISON)) ||
 		s->ctor)) {
 		/*
 		 * Relocate free pointer after the object if it is not