Re: [PATCH v12 14/21] futex: Allow to resize the private local hash

[Lai, Yi]

On Mon, Jun 02, 2025 at 01:00:27PM +0200, Sebastian Andrzej Siewior wrote:
> On 2025-06-01 15:39:47 [+0800], Lai, Yi wrote:
> > Hi Sebastian Andrzej Siewior,
> Hi Yi,
> > Greetings!
> > 
> > I used Syzkaller and found that there is KASAN: null-ptr-deref Read in __futex_pivot_hash in linux-next next-20250527.
> > 
> > After bisection and the first bad commit is:
> > "
> > bd54df5ea7ca futex: Allow to resize the private local hash
> > "
> 
> Thank you for the report. Next time please trim your report. There is no
> need to put your report in the middle of the patch.
> 
> The following fixes it:
>

Will trim my report next time.

After applying following patch on top of lastest linux-next, issue
cannot be reproduced. Thanks.

Regards,
Yi Lai

> ----------->8--------------
> 
> From: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> Date: Mon, 2 Jun 2025 12:11:13 +0200
> Subject: [PATCH] futex: Verify under the lock if global hash is in use
> 
> Once the global hash is requested there is no way back to switch back to
> the per-task private hash. This is checked at the begin of the function.
> 
> It is possible that two threads simultaneously request the global hash
> and both pass the initial check and block later on the
> mm::futex_hash_lock. In this case the first thread performs the switch
> to the global hash. The second thread will also attempt to switch to the
> global hash and while doing so, accessing the nonexisting slot 1 of the
> struct futex_private_hash.
> This has been reported by Yi Lai.
> 
> Verify under mm_struct::futex_phash that the global hash is not in use.
> 
> Reported-by: "Lai, Yi" <yi1.lai@xxxxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/all/aDwDw9Aygqo6oAx+@ly-workstation/
> Fixes: bd54df5ea7cad ("futex: Allow to resize the private local hash")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
>  kernel/futex/core.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/kernel/futex/core.c b/kernel/futex/core.c
> index 1cd3a646c91fd..abbd97c2fcba8 100644
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c
> @@ -1629,6 +1629,16 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags)
>  		mm->futex_phash_new = NULL;
>  
>  		if (fph) {
> +			if (cur && !cur->hash_mask) {
> +				/*
> +				 * If two threads simultaneously request the global
> +				 * hash then the first one performs the switch,
> +				 * the second one returns here.
> +				 */
> +				free = fph;
> +				mm->futex_phash_new = new;
> +				return -EBUSY;
> +			}
>  			if (cur && !new) {
>  				/*
>  				 * If we have an existing hash, but do not yet have
> -- 
> 2.49.0
> 
> 
> Sebastian