Re: [PATCH v2 1/4] mm, swap: Fix potential NULL dereference when trying a sleep table allocation

[Kairui Song <ryncsn@gmail.com>]

On Thu, Aug 20, 2026 at 07:55:02PM +0800, Kemeng Shi wrote:
> The root cause of this issue is because multi-tables are updated in non
> atomic context. To be more specific, the issue could be triggerred as
> following:
> 
> swap_alloc_fast         swap_cluster_populate()
>                          /* Try a sleep allocation */
>                          spin_unlock(&ci->lock);
>                          swap_cluster_alloc_table()
>                           rcu_assign_pointer(ci->table, table);
> 
>  ci = swap_cluster_lock(si, offset)
>  cluster_is_usable(ci, order）
>   if (!cluster_table_is_alloced(ci)) // ok
>  alloc_swap_scan_cluster()
>   cluster_scan_range()
>    __swap_table_get()
> 
>                           /* free table when more table allocation fails */
>                           ci->memcg_table = kzalloc_obj(*ci->memcg_table,
>                                                         gfp);
>                           if (!ci->memcg_table)
>                            swap_cluster_free_table()
>                             rcu_assign_pointer(ci->table, NULL);
> 
>     table = rcu_dereference_check(ci->table, lockdep_is_held(&ci->lock));
>      atomic_long_read(&table[off]); // NULL dereference
> 
> Fix the issue by making tables visible at end of swap_cluster_populate().

I think you mean "end of swap_cluster_alloc_table".

> 
> Fixes: 2fe7a6f5024b8 ("mm/memcg, swap: store cgroup id in cluster table directly")

Hmm, wrong commit id again, should be

Fixes: b197d41462c2 ("mm/memcg, swap: store cgroup id in cluster table directly")

> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  mm/swapfile.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 615d90867111..4561c864f806 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -466,8 +466,6 @@ static int swap_cluster_alloc_table(struct swap_cluster_info *ci, gfp_t gfp)
>  	if (!table)
>  		return -ENOMEM;
>  
> -	rcu_assign_pointer(ci->table, table);
> -
>  #ifdef CONFIG_MEMCG
>  	if (!mem_cgroup_disabled()) {
>  		VM_WARN_ON_ONCE(ci->memcg_table);
> @@ -487,6 +485,12 @@ static int swap_cluster_alloc_table(struct swap_cluster_info *ci, gfp_t gfp)
>  		return -ENOMEM;
>  	}
>  #endif
> +
> +	/*
> +	 * Make tables visible to cluster_is_usable() after everything is
> +	 * ready.
> +	 */
> +	rcu_assign_pointer(ci->table, table);

Hmm, but for the error paths above, they will leak the new allocated table?