linux-kernel - Re: [PATCH v2] ksmbd: vfs_cache: avoid integer overflow in inode

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: <20251115170253.64ee8828@pumpkin>
Date: Sat, 15 Nov 2025 17:02:53 +0000
From: David Laight <david.laight.linux@...il.com>
To: Qianchang Zhao <pioooooooooip@...il.com>
Cc: Namjae Jeon <linkinjeon@...nel.org>, Steve French <smfrench@...il.com>,
 gregkh@...uxfoundation.org, linux-cifs@...r.kernel.org,
 linux-kernel@...r.kernel.org, Zhitong Liu <liuzhitong1993@...il.com>,
 stable@...r.kernel.org
Subject: Re: [PATCH v2] ksmbd: vfs_cache: avoid integer overflow in
 inode_hash()

On Sat, 15 Nov 2025 23:48:36 +0900
Qianchang Zhao <pioooooooooip@...il.com> wrote:

> inode_hash() currently mixes a hash value with the super_block pointer
> using an unbounded multiplication:
> 
>     tmp = (hashval * (unsigned long)sb) ^
>           (GOLDEN_RATIO_PRIME + hashval) / L1_CACHE_BYTES;
> 
> On 64-bit kernels this multiplication can overflow and wrap in unsigned
> long arithmetic.

The same happens on 32bits.

> While this is not a memory-safety issue, it is an
> unbounded integer operation and weakens the mixing properties of the
> hash.

Are you sure, I'd have thought all the 'carry' operations in the multiply
would make it better.

> Replace the pointer*hash multiply with hash_long() over a mixed value
> (hashval ^ (unsigned long)sb) and keep the existing shift/mask. This
> removes the overflow source and reuses the standard hash helper already
> used in other kernel code.
> 
> This is an integer wraparound / robustness issue (CWE-190/CWE-407),
> not a memory-safety bug.

It isn't really an integer wraparound bug either.
The fact that only the low bits of the product are used shouldn't matter
at all.

OTOH it might be worth sifting 'sb' right some bits - quite a few of
its low bits are zero and they end up as zeros in the product.

	David

> 
> Reported-by: Qianchang Zhao <pioooooooooip@...il.com>
> Reported-by: Zhitong Liu <liuzhitong1993@...il.com>
> Cc: stable@...r.kernel.org
> Signed-off-by: Qianchang Zhao <pioooooooooip@...il.com>
> ---
>  fs/smb/server/vfs_cache.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
> index dfed6fce8..a62ea5aae 100644
> --- a/fs/smb/server/vfs_cache.c
> +++ b/fs/smb/server/vfs_cache.c
> @@ -10,6 +10,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/kthread.h>
>  #include <linux/freezer.h>
> +#include <linux/hash.h>
>  
>  #include "glob.h"
>  #include "vfs_cache.h"
> @@ -65,12 +66,8 @@ static void fd_limit_close(void)
>  
>  static unsigned long inode_hash(struct super_block *sb, unsigned long hashval)
>  {
> -	unsigned long tmp;
> -
> -	tmp = (hashval * (unsigned long)sb) ^ (GOLDEN_RATIO_PRIME + hashval) /
> -		L1_CACHE_BYTES;
> -	tmp = tmp ^ ((tmp ^ GOLDEN_RATIO_PRIME) >> inode_hash_shift);
> -	return tmp & inode_hash_mask;
> +	unsigned long mixed = hashval ^ (unsigned long)sb;
> +	return hash_long(mixed, inode_hash_shift) & inode_hash_mask;
>  }
>  
>  static struct ksmbd_inode *__ksmbd_inode_lookup(struct dentry *de)