linux-kernel - Re: [PATCH] mm: slub: Panic if the object corruption is checked.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250121023240.GA645761@tiffany>
Date: Tue, 21 Jan 2025 11:32:40 +0900
From: Hyesoo Yu <hyesoo.yu@...sung.com>
To: Hyeonggon Yoo <hyeonggon.yoo@...com>
Cc: Hyeonggon Yoo <42.hyeyoo@...il.com>, kernel_team@...ynix.com,
	"janghyuck.kim@...sung.com" <janghyuck.kim@...sung.com>, Andrew Morton
	<akpm@...ux-foundation.org>, Jonathan Corbet <corbet@....net>, Christoph
	Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>, David Rientjes
	<rientjes@...gle.com>, Joonsoo Kim <iamjoonsoo.kim@....com>, Vlastimil Babka
	<vbabka@...e.cz>, Roman Gushchin <roman.gushchin@...ux.dev>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm: slub: Panic if the object corruption is checked.

On Tue, Jan 21, 2025 at 10:48:08AM +0900, Hyeonggon Yoo wrote:
> 
> 
> On 1/21/2025 9:54 AM, Hyesoo Yu wrote:
> > On Tue, Jan 21, 2025 at 12:41:01AM +0900, Hyeonggon Yoo wrote:
> > > On Mon, Jan 20, 2025 at 5:30 PM Hyesoo Yu <hyesoo.yu@...sung.com> wrote:
> > > > 
> > > > If a slab object is corrupted or an error occurs in its internal
> > > > value, continuing after restoration may cause other side effects.
> > > > At this point, it is difficult to debug because the problem occurred
> > > > in the past. A flag has been added that can cause a panic when there
> > > > is a problem with the object.
> > > 
> > > Hi Hyesoo,
> > > 
> > > I'm concerned about this because it goes against the effort to avoid
> > > introducing new BUG() calls [1].
> > > 
> > > And I think it would be more appropriate to use existing panic_on_warn
> > > functionality [2] which causes
> > > a panic on WARN(), rather than introducing a SLUB-specific knob to do
> > > the same thing.
> > > 
> > > However SLUB does not call WARN() and uses pr_err() instead when
> > > reporting an error.
> > > Vlastimil and I talked about changing it to use WARN() a while ago
> > > [3], but neither of us
> > > have done that yet.
> > > 
> > > Probably you may want to look at it, as it also aligns with your purpose?
> > > FYI, if you would like to work on it, please make sure that it WARN()
> > > is suppressed during kunit test.
> > > 
> > > [1] https://docs.kernel.org/process/deprecated.html#bug-and-bug-on
> > > [2] https://www.kernel.org/doc/html/v6.9/admin-guide/sysctl/kernel.html#panic-on-warn
> > > [3] https://lore.kernel.org/linux-mm/d4219cd9-32d3-4697-93b9-6a44bf77d50c@suse.cz
> > > 
> > > Best,
> > > Hyeonggon
> > 
> > Thanks for response.
> > 
> > Using warn() instead of panic, is a great idea.
> > Thanks for pointing out what I missed.
> 
> Just for clarification, I think changing the common error reporting
> logic (like, slab_bug()) to use WARN() will be preferable to inserting
> new WARN()s at random points, which is what this patch does now.
> 
> Best,
> Hyeonggon
> 

Thanks you for clarification.

Actually, I considered adding BUG_ON() to slab_bug. However if we add BUG_ON() to slab_bug,
it will prevent many meaningful error log from being printed subsequently.
As you know, slab_bug is the log that usually is printed at the biginning of a bug
in the slab. As a result, it would be difficult to figure out the problems based on
the logs during our large-scale test-bed.

Similary, even if I use WARN() in slab_bug, we won't be able to obtain the logs
when panic_on_warn is enabled. I don't think it is useful to include WARN in slab_bug.

Instead, I will implement a solution where WARN is only used in slab_fix before
slab object is restored. If I add it to slab_fix, I think warning is suppressed on
kunit test by slab_add_kunit_errors handling.

Thanks,
Regards.