netdev - Re: [PATCH ipsec-next] xfrm: remove hash table alloc/free helpers

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: <20250225080440.GE53094@unreal>
Date: Tue, 25 Feb 2025 10:04:40 +0200
From: Leon Romanovsky <leon@...nel.org>
To: Florian Westphal <fw@...len.de>
Cc: netdev@...r.kernel.org, steffen.klassert@...unet.com,
	herbert@...dor.apana.org.au
Subject: Re: [PATCH ipsec-next] xfrm: remove hash table alloc/free helpers

On Mon, Feb 24, 2025 at 06:10:50PM +0100, Florian Westphal wrote:
> These functions predate kvmalloc, update xfrm to use that instead.
> This also allows to drop the 'size' argument passed to xfrm_hash_free().
> 
> xfrm_hash_free() is kept around because of 'struct hlist_head *' arg type
> instead of 'void *'.

<...>

> -struct hlist_head *xfrm_hash_alloc(unsigned int sz);
> -void xfrm_hash_free(struct hlist_head *n, unsigned int sz);
> +static inline struct hlist_head *xfrm_hash_alloc(unsigned int sz)
> +{
> +	return kvzalloc(sz, GFP_KERNEL);
> +}
>  
> +static inline void xfrm_hash_free(struct hlist_head *n)
> +{
> +	kvfree(n);
> +}

Sorry, what does this wrapper give us?
You are passing pointer as is and there is no any pointer type check
that this construction will give us.

I would say that there is no need to hide basic kernel primitives
like kvzalloc and kvfree, and better leave them as is without wrappers.

The change itself looks good,
Reviewed-by: Leon Romanovsky <leonro@...dia.com>