Re: [PATCH v3 02/11] mm, swap: support zswap and zeroswap as vswap backends

[Kunwu Chan]

On Thu,  6 Aug 2026 11:42:45 -0700 Nhat Pham <nphamcs@xxxxxxxxx> wrote:

Hi Nhat,
[...]
syzbot reported a NULL dereference in the v3 series:

  __vtable_get()
    vswap_to_phys()
      swap_entry_backend_has_flag()

Seems like the underlying issue is introduced by this patch's
`virtual_table` lifetime management.

This patch adds:

> @@ -70,6 +70,7 @@ struct swap_cluster_info_dynamic {
>       struct swap_cluster_info ci;
>       unsigned int index;             /* for cluster_index() */
>       struct rcu_head rcu;
> +     atomic_long_t *virtual_table;   /* Backing pointers for vswap slots */
>  };

[...]

while the read side does:

> +static inline unsigned long __vtable_get(struct swap_cluster_info_dynamic *ci_dyn,
> +                                      unsigned int off)
> +{
> +     VM_WARN_ON_ONCE(off >= SWAPFILE_CLUSTER);
> +     return atomic_long_read(&ci_dyn->virtual_table[off]);
> +}
> +

`atomic_long_read()` only makes the access atomic; it does not protect
the lifetime of the allocation being accessed.

[...]

> +static inline void vswap_cluster_free_vtable(struct swap_cluster_info *ci)
> +{
> +     struct swap_cluster_info_dynamic *ci_dyn;
> +

and frees it synchronously:

> +     ci_dyn = container_of(ci, struct swap_cluster_info_dynamic, ci);
> +     kfree(ci_dyn->virtual_table);
> +     ci_dyn->virtual_table = NULL;
> +}
> +

The existing `ci->table` already has an RCU-aware lifetime: readers use
the corresponding RCU access rules, and the storage is not freed until
after the appropriate grace period. `virtual_table` introduced here does
not have an equivalent lifetime rule.

The syzbot crash shows that the current teardown/read-side synchronization
is insufficient: `__vtable_get()` can observe a torn-down
`virtual_table` and dereference NULL.

I don't think a NULL check in `__vtable_get()` alone would be the right
fix. The NULL dereference is a symptom of the missing lifetime guarantee.
`virtual_table` needs to remain valid for as long as a reader can reach
and access the corresponding dynamic cluster.

It probably makes sense to make `virtual_table` follow the same lifetime
scheme as the existing cluster table, or otherwise tie its freeing to the
lifetime of `swap_cluster_info_dynamic`.

[...]


Thanks,
KunWu