On Mon, Nov 10, 2025 at 9:56 AM Huang, Ying ying.huang@linux.alibaba.com wrote:
Hi, Kairui,
Kairui Song via B4 Relay devnull+kasong.tencent.com@kernel.org writes:
From: Kairui Song kasong@tencent.com
This reverts commit 78524b05f1a3e16a5d00cc9c6259c41a9d6003ce.
While reviewing recent leaf entry changes, I noticed that commit 78524b05f1a3 ("mm, swap: avoid redundant swap device pinning") isn't correct. It's true that most all callers of __read_swap_cache_async are already holding a swap entry reference, so the repeated swap device pinning isn't needed on the same swap device, but it is possible that VMA readahead (swap_vma_readahead()) may encounter swap entries from a different swap device when there are multiple swap devices, and call __read_swap_cache_async without holding a reference to that swap device.
So it is possible to cause a UAF if swapoff of device A raced with swapin on device B, and VMA readahead tries to read swap entries from device A. It's not easy to trigger but in theory possible to cause real issues. And besides, that commit made swap more vulnerable to issues like corrupted page tables.
Just revert it. __read_swap_cache_async isn't that sensitive to performance after all, as it's mostly used for SSD/HDD swap devices with readahead. SYNCHRONOUS_IO devices may fallback onto it for swap count > 1 entries, but very soon we will have a new helper and routine for such devices, so they will never touch this helper or have redundant swap device reference overhead.
Is it better to add get_swap_device() in swap_vma_readahead()? Whenever we get a swap entry, the first thing we need to do is call get_swap_device() to check the validity of the swap entry and prevent the backing swap device from going under us. This helps us to avoid checking the validity of the swap entry in every swap function. Does this sound reasonable?
Hi Ying, thanks for the suggestion!
Yes, that's also a feasible approach.
What I was thinking is that, currently except the readahead path, all swapin entry goes through the get_swap_device() helper, that helper also helps to mitigate swap entry corruption that may causes OOB or NULL deref. Although I think it's really not that helpful at all to mitigate page table corruption from the kernel side, but seems not a really bad idea to have.
And the code is simpler this way, and seems more suitable for a stable & mainline fix. If we want to add get_swap_device() in swap_vma_readahead(), we need to do that for every entry that doesn't match the target entry's swap device. The reference overhead is trivial compared to readhead and bio layer, and only non SYNCHRONOUS_IO devices use this helper (madvise is a special case, we may optimize that later). ZRAM may fallback to the readahead path but this fallback will be eliminated very soon in swap table p2.
Another approach I thought about is that we might want readahead to stop when it sees entries from a different swap device. That swap device might be ZRAM where VMA readahead is not helpful.
How do you think?