Re: [PATCH v3 02/11] mm, swap: support zswap and zeroswap as vswap backends
Kunwu Chan <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On Thu, 6 Aug 2026 11:42:45 -0700 Nhat Pham <[email protected]> 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