linux-kernel - Re: [PATCH] ext4: improve str2hashbuf by processing 4-byte chunks

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: <20251116193513.0f90712a@pumpkin>
Date: Sun, 16 Nov 2025 19:35:13 +0000
From: David Laight <david.laight.linux@...il.com>
To: Guan-Chun Wu <409411716@....tku.edu.tw>
Cc: Theodore Ts'o <tytso@....edu>, Andreas Dilger
 <adilger.kernel@...ger.ca>, linux-ext4@...r.kernel.org,
 linux-kernel@...r.kernel.org, visitorckw@...il.com
Subject: Re: [PATCH] ext4: improve str2hashbuf by processing 4-byte chunks

On Sun, 16 Nov 2025 21:01:05 +0800
Guan-Chun Wu <409411716@....tku.edu.tw> wrote:

> The original byte-by-byte implementation with modulo checks is less
> efficient. Refactor str2hashbuf_unsigned() and str2hashbuf_signed()
> to process input in explicit 4-byte chunks instead of using a
> modulus-based loop to emit words byte by byte.

There are much bigger gains to be made - the current code is horrid.
Not least due to the costs of the indirect calls.
It is better to use conditionals than indirect calls. 


> 
> This change removes per-byte modulo checks and reduces loop iterations,
> improving efficiency.
> 
> Performance test (x86_64, Intel Core i7-10700 @ 2.90GHz, average over 10000
> runs, using kernel module for testing):
> 
>     len | orig_s | new_s | orig_u | new_u
>     ----+--------+-------+--------+-------
>       1 |   70   |   71  |   63   |   63
>       8 |   68   |   64  |   64   |   62
>      32 |   75   |   70  |   75   |   63
>      64 |   96   |   71  |  100   |   68
>     255 |  192   |  108  |  187   |   84
> 
> Signed-off-by: Guan-Chun Wu <409411716@....tku.edu.tw>
> ---
>  fs/ext4/hash.c | 48 ++++++++++++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c
> index 33cd5b6b02d5..75105828e8b4 100644
> --- a/fs/ext4/hash.c
> +++ b/fs/ext4/hash.c
> @@ -141,21 +141,29 @@ static void str2hashbuf_signed(const char *msg, int len, __u32 *buf, int num)
>  	pad = (__u32)len | ((__u32)len << 8);
>  	pad |= pad << 16;
>  
> -	val = pad;
>  	if (len > num*4)
>  		len = num * 4;
> -	for (i = 0; i < len; i++) {
> -		val = ((int) scp[i]) + (val << 8);
> -		if ((i % 4) == 3) {
> -			*buf++ = val;
> -			val = pad;
> -			num--;
> -		}
> +
> +	while (len >= 4) {
> +		val = ((int)scp[0] << 24) + ((int)scp[1] << 16) +
> +				((int)scp[2] << 8) + (int)scp[3];

The (int) casts are unnecessary (throughout), 'char' is always promoted to
'signed int' before any arithmetic.

> +		*buf++ = val;
> +		scp += 4;
> +		len -= 4;
> +		num--;
>  	}
> +
> +	val = pad;
> +
> +	for (i = 0; i < len; i++)
> +		val = (int)scp[i] + (val << 8);
> +
>  	if (--num >= 0)
>  		*buf++ = val;
> +
>  	while (--num >= 0)
>  		*buf++ = pad;
> +
>  }
>  
>  static void str2hashbuf_unsigned(const char *msg, int len, __u32 *buf, int num)
> @@ -167,21 +175,29 @@ static void str2hashbuf_unsigned(const char *msg, int len, __u32 *buf, int num)
>  	pad = (__u32)len | ((__u32)len << 8);
>  	pad |= pad << 16;
>  
> -	val = pad;
>  	if (len > num*4)
>  		len = num * 4;
> -	for (i = 0; i < len; i++) {
> -		val = ((int) ucp[i]) + (val << 8);
> -		if ((i % 4) == 3) {
> -			*buf++ = val;
> -			val = pad;
> -			num--;
> -		}
> +
> +	while (len >= 4) {
> +		val = ((int)ucp[0] << 24) | ((int)ucp[1] << 16) |
> +				((int)ucp[2] << 8) | (int)ucp[3];

Isn't that get_misaligned_be32() ?

	David 

> +		*buf++ = val;
> +		ucp += 4;
> +		len -= 4;
> +		num--;
>  	}
> +
> +	val = pad;
> +
> +	for (i = 0; i < len; i++)
> +		val = (int)ucp[i] + (val << 8);
> +
>  	if (--num >= 0)
>  		*buf++ = val;
> +
>  	while (--num >= 0)
>  		*buf++ = pad;
> +
>  }
>  
>  /*