linux-hardening - Re: [RFC][PATCH 4/4] pstore: Replace classic kmalloc code pattern with typed argument

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: <093108ea-f567-4210-9f2e-eb8e5039d1eb@intel.com>
Date: Tue, 9 Jul 2024 09:06:58 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Kees Cook <kees@...nel.org>, Vlastimil Babka <vbabka@...e.cz>
CC: Tony Luck <tony.luck@...el.com>, "Guilherme G. Piccoli"
	<gpiccoli@...lia.com>, <linux-hardening@...r.kernel.org>, Jann Horn
	<jannh@...gle.com>, Nick Desaulniers <ndesaulniers@...gle.com>, Miguel Ojeda
	<ojeda@...nel.org>, Marco Elver <elver@...gle.com>, Nathan Chancellor
	<nathan@...nel.org>, Hao Luo <haoluo@...gle.com>, 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>, Roman Gushchin <roman.gushchin@...ux.dev>,
	Hyeonggon Yoo <42.hyeyoo@...il.com>, Mark Rutland <mark.rutland@....com>,
	Jakub Kicinski <kuba@...nel.org>, Petr Pavlu <petr.pavlu@...e.com>,
	"Alexander Lobakin" <aleksander.lobakin@...el.com>, Tony Ambardar
	<tony.ambardar@...il.com>, <linux-kernel@...r.kernel.org>,
	<linux-mm@...ck.org>
Subject: Re: [RFC][PATCH 4/4] pstore: Replace classic kmalloc code pattern
 with typed argument

On 7/8/24 21:18, Kees Cook wrote:
> Using a short Coccinelle script, it is possible to replace the classic
> kmalloc code patterns with the typed information:
> 
> @alloc@
> type TYPE;
> TYPE *P;
> expression GFP;
> identifier ALLOC =~ "k[mz]alloc";
> @@
> 
> 	P = ALLOC(
> -		\(sizeof(*P)\|sizeof(TYPE)\), GFP)
> +		P, GFP)
> 
> Show this just for kmalloc/kzalloc usage in fs/pstore as an example.
> 
> Doing this for all allocator calls in the kernel touches much more:
> 
>   11941 files changed, 22459 insertions(+), 22345 deletions(-)
> 
> And obviously requires some more wrappers for kv*alloc, devm_*alloc,
> etc.
> 
> Signed-off-by: Kees Cook <kees@...nel.org>
> ---
> Cc: Tony Luck <tony.luck@...el.com>
> Cc: "Guilherme G. Piccoli" <gpiccoli@...lia.com>
> Cc: linux-hardening@...r.kernel.org
> ---
>   fs/pstore/blk.c      | 2 +-
>   fs/pstore/platform.c | 2 +-
>   fs/pstore/ram.c      | 3 +--
>   fs/pstore/ram_core.c | 2 +-
>   fs/pstore/zone.c     | 2 +-
>   5 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
> index de8cf5d75f34..7bb9cacb380f 100644
> --- a/fs/pstore/blk.c
> +++ b/fs/pstore/blk.c
> @@ -297,7 +297,7 @@ static int __init __best_effort_init(void)
>   		return -EINVAL;
>   	}
>   
> -	best_effort_dev = kzalloc(sizeof(*best_effort_dev), GFP_KERNEL);
> +	best_effort_dev = kzalloc(best_effort_dev, GFP_KERNEL);
>   	if (!best_effort_dev)
>   		return -ENOMEM;
I expect raised eyebrows and typical vocalizations of amusement :D -
IOW: we should consider changing the name of the macro, although I like
it as is :)

other:
you repeat the pointer name twice, and code is magic anyway, so perhaps:
	kzalloc_me(best_effort_dev, GFP_KERNEL);
and another variant to cover declaration-with-init.