All of lore.kernel.org
 help / color / mirror / Atom feed
From: Feng Tang <feng.tang@intel.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Theodore Ts'o <tytso@mit.edu>,
	Aaro Koskinen <aaro.koskinen@iki.fi>,
	"Torvalds, Linus" <torvalds@linux-foundation.org>,
	Paul Cercueil <paul@crapouillou.net>,
	"42.hyeyoo@gmail.com" <42.hyeyoo@gmail.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"cl@linux.com" <cl@linux.com>,
	"iamjoonsoo.kim@lge.com" <iamjoonsoo.kim@lge.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"penberg@kernel.org" <penberg@kernel.org>,
	"rientjes@google.com" <rientjes@google.com>,
	"rkovhaev@gmail.com" <rkovhaev@gmail.com>,
	"roman.gushchin@linux.dev" <roman.gushchin@linux.dev>,
	"willy@infradead.org" <willy@infradead.org>
Subject: Re: Deprecating and removing SLOB
Date: Thu, 10 Nov 2022 15:54:37 +0800	[thread overview]
Message-ID: <Y2yuPUxruFuJN+qM@feng-clx> (raw)
In-Reply-To: <e610b6fa-aa71-d612-0eb2-03ba6c4a6b46@suse.cz>

On Thu, Nov 10, 2022 at 03:31:31PM +0800, Vlastimil Babka wrote:
> On 11/10/22 05:40, Theodore Ts'o wrote:
> > On Thu, Nov 10, 2022 at 01:48:32AM +0200, Aaro Koskinen wrote:
> >> 
> >> Some of the reported SLOB issues have been actually real driver bugs,
> >> that go unnoticed when SLUB/SLAB are used (unless perhaps debug stuff
> >> is enabled). I'm not saying kernel should keep SLOB, but it's good at
> >> failing early when there is a bug. See e.g. commit 120ee599b5bf ("staging:
> >> octeon-usb: prevent memory corruption")
> > 
> > Out of curiosity, are these bugs that would have been found using
> > KASAN or some of the other kernel sanitizers and/or other debugging
> > tools we have at our disposal?
> 
> Hopefully slub_debug redzoning would be able to trigger the bug described in
> commit 120ee599b5bf above, which is:
> 
> >     octeon-hcd will crash the kernel when SLOB is used. This usually happens
> >     after the 18-byte control transfer when a device descriptor is read.
> >     The DMA engine is always transfering full 32-bit words and if the
> >     transfer is shorter, some random garbage appears after the buffer.
> >     The problem is not visible with SLUB since it rounds up the allocations
> >     to word boundary, and the extra bytes will go undetected.
> 
> Ah, actually it wouldn't *now* as SLUB would make the allocation fall into
> kmalloc-32 cache and only add redzone beyond 32 bytes. But with upcoming
> changes by Feng Tang, this should work.
 
I wrote a simple case trying simulating this:

	static noinline void dma_align_test(void)
	{
		char *buf;

		buf = kmalloc(18, GFP_KERNEL);
		buf[18] = 0;
		buf[19] = 0;
		kfree(buf);
	}

And with slub_debug on and the slub_redzone patchset[1], it did
catch the out-of-bound access, as in the dmesg:

"
 =============================================================================
 BUG kmalloc-32 (Not tainted): kmalloc Redzone overwritten
 -----------------------------------------------------------------------------

 0xffff888005ebb032-0xffff888005ebb033 @offset=50. First byte 0x0 instead of 0xcc
 Allocated in dma_align_test+0x1b/0x29 age=6554 cpu=1 pid=1
  __kmem_cache_alloc_node+0x2a7/0x320
  kmalloc_trace+0x27/0xa0
  dma_align_test+0x1b/0x29
  late_slub_debug+0xa/0x11
  do_one_initcall+0x87/0x2a0
  ...

 Slab 0xffffea000017aec0 objects=21 used=19 fp=0xffff888005ebbf20 flags=0xfffffc0000200(slab|node=0|zone=1|lastcpupid=0x1fffff)
 Object 0xffff888005ebb020 @offset=32 fp=0x0000000000000000

 Redzone  ffff888005ebb000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
 Redzone  ffff888005ebb010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
 Object   ffff888005ebb020: 50 92 28 00 81 88 ff ff 01 00 00 00 11 00 b6 07  P.(.............
 Object   ffff888005ebb030: 6b a5 00 00 cc cc cc cc cc cc cc cc cc cc cc cc  k...............
 Redzone  ffff888005ebb040: cc cc cc cc cc cc cc cc                          ........
 Padding  ffff888005ebb0a4: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a  ZZZZZZZZZZZZZZZZ
 Padding  ffff888005ebb0b4: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a              ZZZZZZZZZZZZ
"

[1]. https://lore.kernel.org/lkml/20221021032405.1825078-1-feng.tang@intel.com/ 

Thanks,
Feng

> slub_debug would also have a chance of catching buffer overflows by kernel
> code itself, not DMA, and tell you about it more sooner and gracefully than
> crashing. KASAN also, even with a higher chance and precision, if it's
> available for your arch and your device constraints can tolerate its larger
> overhead.
> 
> >        		      	    	      	    	 - Ted
> > 
> > 						 
> 
> 

  reply	other threads:[~2022-11-10  7:58 UTC|newest]

Thread overview: 108+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-09 20:56 Deprecating and removing SLOB Paul Cercueil
2022-11-09 21:39 ` Linus Torvalds
2022-11-09 23:48   ` Aaro Koskinen
2022-11-09 23:51     ` Aaro Koskinen
2022-11-10  4:40     ` Theodore Ts'o
2022-11-10  7:31       ` Vlastimil Babka
2022-11-10  7:54         ` Feng Tang [this message]
2022-11-10 16:20         ` Matthew Wilcox
2022-11-11  9:37           ` David Laight
  -- strict thread matches above, loose matches on Subject: below --
2022-11-08 15:55 Vlastimil Babka
2022-11-08 18:18 ` Christophe Leroy
2022-11-08 19:17   ` Andrew Morton
2022-11-08 18:46 ` Roman Gushchin
2022-11-08 20:13   ` Yosry Ahmed
2022-11-09  9:09     ` Vlastimil Babka
2022-11-08 21:44 ` Pasha Tatashin
2022-11-09  9:00   ` Vlastimil Babka
2022-11-09  9:00     ` Vlastimil Babka
2022-11-09  9:00     ` Vlastimil Babka
2022-11-09  9:00     ` Vlastimil Babka
2022-11-09 15:50     ` Aaro Koskinen
2022-11-09 15:50       ` Aaro Koskinen
2022-11-09 15:50       ` Aaro Koskinen
2022-11-09 15:50       ` Aaro Koskinen
2022-11-09 16:45       ` Geert Uytterhoeven
2022-11-09 16:45         ` Geert Uytterhoeven
2022-11-09 16:45         ` Geert Uytterhoeven
2022-11-09 16:45         ` Geert Uytterhoeven
2022-11-09 17:45       ` Mike Rapoport
2022-11-09 17:45         ` Mike Rapoport
2022-11-09 17:45         ` Mike Rapoport
2022-11-09 17:45         ` Mike Rapoport
2022-11-09 21:16       ` Janusz Krzysztofik
2022-11-09 21:16         ` Janusz Krzysztofik
2022-11-09 21:16         ` Janusz Krzysztofik
2022-11-09 21:16         ` Janusz Krzysztofik
2022-11-09 17:57   ` Conor.Dooley
2022-11-09 23:00     ` Damien Le Moal
2022-11-11 10:25       ` Vlastimil Babka
2022-11-12  1:40         ` Damien Le Moal
2022-11-11 10:33   ` Vlastimil Babka
2022-11-11 10:33     ` Vlastimil Babka
2022-11-11 10:33     ` Vlastimil Babka
2022-11-11 10:33     ` Vlastimil Babka
2022-11-11 20:46     ` Conor Dooley
2022-11-11 20:46       ` Conor Dooley
2022-11-11 20:46       ` Conor Dooley
2022-11-11 20:46       ` Conor Dooley
2022-11-12  1:40       ` Damien Le Moal
2022-11-12  1:40         ` Damien Le Moal
2022-11-12  1:40         ` Damien Le Moal
2022-11-12  1:40         ` Damien Le Moal
2022-11-14  1:55       ` Damien Le Moal
2022-11-14  1:55         ` Damien Le Moal
2022-11-14  1:55         ` Damien Le Moal
2022-11-14  1:55         ` Damien Le Moal
2022-11-14  5:48         ` Damien Le Moal
2022-11-14  5:48           ` Damien Le Moal
2022-11-14  5:48           ` Damien Le Moal
2022-11-14  5:48           ` Damien Le Moal
2022-11-14  9:36           ` Vlastimil Babka
2022-11-14  9:36             ` Vlastimil Babka
2022-11-14  9:36             ` Vlastimil Babka
2022-11-14  9:36             ` Vlastimil Babka
2022-11-14 11:35             ` Damien Le Moal
2022-11-14 11:35               ` Damien Le Moal
2022-11-14 11:35               ` Damien Le Moal
2022-11-14 11:35               ` Damien Le Moal
2022-11-14 14:47               ` Hyeonggon Yoo
2022-11-14 14:47                 ` Hyeonggon Yoo
2022-11-14 14:47                 ` Hyeonggon Yoo
2022-11-14 14:47                 ` Hyeonggon Yoo
2022-11-15  4:24                 ` Damien Le Moal
2022-11-15  4:24                   ` Damien Le Moal
2022-11-15  4:24                   ` Damien Le Moal
2022-11-15  4:24                   ` Damien Le Moal
2022-11-15  4:28                   ` Damien Le Moal
2022-11-15  4:28                     ` Damien Le Moal
2022-11-15  4:28                     ` Damien Le Moal
2022-11-15  4:28                     ` Damien Le Moal
2022-11-16  7:57                     ` Matthew Wilcox
2022-11-16  7:57                       ` Matthew Wilcox
2022-11-16  7:57                       ` Matthew Wilcox
2022-11-16  7:57                       ` Matthew Wilcox
2022-11-16  8:02                       ` Damien Le Moal
2022-11-16  8:02                         ` Damien Le Moal
2022-11-16  8:02                         ` Damien Le Moal
2022-11-16  8:02                         ` Damien Le Moal
2022-11-16 17:51                   ` Vlastimil Babka
2022-11-16 17:51                     ` Vlastimil Babka
2022-11-16 17:51                     ` Vlastimil Babka
2022-11-16 17:51                     ` Vlastimil Babka
2022-11-17  0:22                     ` Damien Le Moal
2022-11-17  0:22                       ` Damien Le Moal
2022-11-17  0:22                       ` Damien Le Moal
2022-11-17  0:22                       ` Damien Le Moal
2022-11-21  4:30                     ` Damien Le Moal
2022-11-21  4:30                       ` Damien Le Moal
2022-11-21  4:30                       ` Damien Le Moal
2022-11-21  4:30                       ` Damien Le Moal
2022-11-21 17:02                       ` Vlastimil Babka
2022-11-21 17:02                         ` Vlastimil Babka
2022-11-21 17:02                         ` Vlastimil Babka
2022-11-21 17:02                         ` Vlastimil Babka
2022-11-14 11:50             ` Hyeonggon Yoo
2022-11-14 11:50               ` Hyeonggon Yoo
2022-11-14 11:50               ` Hyeonggon Yoo
2022-11-14 11:50               ` Hyeonggon Yoo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y2yuPUxruFuJN+qM@feng-clx \
    --to=feng.tang@intel.com \
    --cc=42.hyeyoo@gmail.com \
    --cc=aaro.koskinen@iki.fi \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=cl@linux.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=paul@crapouillou.net \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=rkovhaev@gmail.com \
    --cc=roman.gushchin@linux.dev \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=vbabka@suse.cz \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.