Re: [PATCH] mm/huge_memory: Fix iterator variable usage after swap()

[Lance Yang]

On 2026/1/21 16:13, zenghongling wrote:
> The iterator variable 'folio' is swapped with 'prev' in the else
> branch. Using 'folio' after swap() checks the potentially NULL
> 'prev' value, not the original iterator value.
>
> Fix by moving folio_put() call before the swap operation in the
> path where swap() occurs.
>
> Found by:
> ./huge_memory.c:4225:6-11: ERROR: iterator variable bound on line 4178 cannot be NULL

Good catch!

But which tree is your patch based on?

Seems like that was already fixed in commit 776bde7caf80[1]. The
whole thing deferred_split_scan() was refactored using folio_batch,
so the buggy code with swap(folio, prev) is gone ...

Ccing Muchun and Qi who fixed that.

[1] 
https://lore.kernel.org/all/59cb6b6fb5ffcff9d23b81890b252960139ad8e7.1762762324.git.zhengqi.arch@xxxxxxxxxxxxx/

Thanks,
Lance

> Signed-off-by: zenghongling <zenghongling@xxxxxxxxxx>
> ---
>   mm/huge_memory.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 6cba1cb14b23..258bf4725aea 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4212,6 +4212,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>   			; /* folio already removed from list */
>   		} else if (!folio_test_partially_mapped(folio)) {
>   			list_del_init(&folio->_deferred_list);
> +			folio_put(folio);
>   			removed++;
>   		} else {
>   			/*
> @@ -4220,10 +4221,9 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>   			 * left on the list (which may be concurrently unqueued)
>   			 * by one safe folio with refcount still raised.
>   			 */
> +			folio_put(folio);
>   			swap(folio, prev);
>   		}
> -		if (folio)
> -			folio_put(folio);
>   	}
>   
>   	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);