linux-kernel - Re: [PATCH V2 1/2] mm/slub: Add panic function when slub leaks

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] [day] [month] [year] [list]

Message-ID: <2024092602-porcupine-pacifier-a795@gregkh>
Date: Thu, 26 Sep 2024 10:26:10 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Fangzheng Zhang <fangzheng.zhang@...soc.com>
Cc: Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>,
	David Rientjes <rientjes@...gle.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Vlastimil Babka <vbabka@...e.cz>,
	Roman Gushchin <roman.gushchin@...ux.dev>,
	Hyeonggon Yoo <42.hyeyoo@...il.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, tkjos@...gle.com,
	Fangzheng Zhang <fangzheng.zhang1003@...il.com>,
	Yuming Han <yuming.han@...soc.com>
Subject: Re: [PATCH V2 1/2] mm/slub: Add panic function when slub leaks

On Wed, Sep 25, 2024 at 09:25:04PM +0800, Fangzheng Zhang wrote:
> Perform real-time memory usage monitoring on the slub page
> allocation paths, ie, kmalloc_large_alloced and alloc_slab_page.
> When the usage exceeds the set threshole value, the panic function
> will be triggered.
> 
> Signed-off-by: Fangzheng Zhang <fangzheng.zhang@...soc.com>
> ---
>  mm/Kconfig | 11 ++++++++
>  mm/slub.c  | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 09aebca1cae3..60cf72d4f0da 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -255,6 +255,17 @@ config SLUB_TINY
>  
>  	   If unsure, say N.
>  
> +config SLUB_LEAK_PANIC
> +	bool "Trigger panic when slub leaks"
> +	default y

Again, only use "default y" if you can not boot a machine without your
option enabled.  That is not the case here so please remove this line.

> +	help
> +	  Detect slub leaks by monitoring its usage in real time on the page
> +	  allocation path of the slub. When the slub occupancy exceeds the
> +	  user-set value, it is considered that the slub is leaking at this
> +	  time, and a panic operation will be triggered immediately. Uers
> +	  can enable and set leak threshold by using the kernel command line
> +	  parameters "slub.leak_panic" and "slub.leak_panic_threshold".
> +
>  config SLAB_MERGE_DEFAULT
>  	bool "Allow slab caches to be merged"
>  	default y
> diff --git a/mm/slub.c b/mm/slub.c
> index 21f71cb6cc06..a0b514626de1 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -42,6 +42,7 @@
>  #include <kunit/test.h>
>  #include <kunit/test-bug.h>
>  #include <linux/sort.h>
> +#include <linux/vmstat.h>
>  
>  #include <linux/debugfs.h>
>  #include <trace/events/kmem.h>
> @@ -218,6 +219,15 @@ DEFINE_STATIC_KEY_FALSE(slub_debug_enabled);
>  #endif
>  #endif		/* CONFIG_SLUB_DEBUG */
>  
> +/* Internal slub_leak_panic definitions */
> +#ifdef CONFIG_SLUB_LEAK_PANIC
> +#define K(x) ((x) << (PAGE_SHIFT-10))

That is a very bad macro name, can you not use something more
descriptive?

> +static bool __read_mostly slub_leak_panic_enabled;
> +static unsigned int __read_mostly slub_leak_panic_threshold;
> +static long max_slab_count, temp_slab_count;
> +#endif

Also, again, please remove #ifdef from .c files as asked for before.

thanks,

greg k-h