mm, swap: fix SWAP_USAGE_OFFLIST_BIT collision with real usage count
Subsystems (2)
Nhat Pham
reviewerPATCH 1/1 mm, swap: fix SWAP_USAGE_OFFLIST_BIT collision with real usage count
SWAP_USAGE_OFFLIST_BIT is embedded in the si->inuse_pages usage counter, and is meant to sit above any value that counter can reach. However, it is defined from BITS_PER_TYPE(atomic_t), so it is bit 30. On a system with 4 KiB pages the flag collides with the usage count once that count reaches 4 TiB.
swap_usage_in_pages() masks bit 30 out, so whenever the real count has that bit set, every caller of it reads 4 TiB low:
* /proc/swaps understates Used by 4 TiB.
* A raw count of exactly 2^30 masks to zero, so try_to_unuse() takes its "if (!swap_usage_in_pages(si)) goto success;" early exit and swapoff tears the device down while pages are still swapped out. Nothing in the rest of swapoff aborts the teardown, so those pages are lost.
Independently of swapoff, the collision also corrupts the counter and the plist. On a device in normal use, a free that leaves bit 30 set in the count makes swap_usage_sub() see the flag where there is only count, and call add_to_avail_list(). It clears the bit with fetch_and(~SWAP_USAGE_OFFLIST_BIT), leaving the stored count 4 TiB below the real one, and calls plist_add() on a device that is already listed, tripping the WARN_ON(!plist_node_empty(node)) in plist_add() and linking the node a second time.
Change the definition of SWAP_USAGE_OFFLIST_BIT to be based on atomic_long_t instead. Note that the usage counter field itself is of this same type, so it is still a valid bit.
--- mm/swapfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/swapfile.c +1 -1
@@ -156,7 +156,7 @@ static struct swap_info_struct *swap_entry_to_info(swp_entry_t entry)* This bit will be set if the device is not on the plist and not * usable, will be cleared if the device is on the plist. */ -#define SWAP_USAGE_OFFLIST_BIT (1UL << (BITS_PER_TYPE(atomic_t) - 2)) +#define SWAP_USAGE_OFFLIST_BIT (1UL << (BITS_PER_TYPE(atomic_long_t) - 2))#define SWAP_USAGE_COUNTER_MASK (~SWAP_USAGE_OFFLIST_BIT)static long swap_usage_in_pages (struct swap_info_struct *si) {
base-commit: efecab401cb15fd3bb9bc05990609acb6b267ff2
Signature · 1 line
2.53.0-Meta
Quoted 26 lines · Nhat Pham
SWAP_USAGE_OFFLIST_BITis embedded in thesi->inuse_pagesusage counter, and is meant to sit above any value that counter can reach. However, it is defined fromBITS_PER_TYPE(atomic_t), so it is bit 30. On a system with 4 KiB pages the flag collides with the usage count once that count reaches 4 TiB.
swap_usage_in_pages()masks bit 30 out, so whenever the real count has that bit set, every caller of it reads 4 TiB low:*
/proc/swapsunderstates Used by 4 TiB.* A raw count of exactly 2^30 masks to zero, so
try_to_unuse()takes its "if (!swap_usage_in_pages(si)) goto success;" early exit and swapoff tears the device down while pages are still swapped out. Nothing in the rest of swapoff aborts the teardown, so those pages are lost.Independently of swapoff, the collision also corrupts the counter and the plist. On a device in normal use, a free that leaves bit 30 set in the count makes
swap_usage_sub()see the flag where there is only count, and calladd_to_avail_list(). It clears the bit withfetch_and(~SWAP_USAGE_OFFLIST_BIT), leaving the stored count 4 TiB below the real one, and callsplist_add()on a device that is already listed, tripping the WARN_ON(!plist_node_empty(node)) inplist_add()and linking the node a second time.
Quoted 7 lines
Change the definition of
SWAP_USAGE_OFFLIST_BITto be based onatomic_long_tinstead. Note that the usage counter field itself is of this same type, so it is still a valid bit.Fixes b228386cf237 ("mm, swap: clean up plist removal and adding")Reported-by Sashiko <[email protected]>
It's really nice that AI can help catch more bugs.
Quoted 12 lines
Suggested-by Andrew Morton <[email protected]>Cc <[email protected]>Signed-off-by Nhat Pham <[email protected]>--- mm/swapfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/swapfile.c +0
-0
mm/swapfile.c static struct swap_info_struct *swap_entry_to_info(swp_entry_t entry) · line 159
Quoted 5 lines
mm/swapfile.c +1
-1@@ -156,7 +156,7 @@ static struct swap_info_struct *swap_entry_to_info(swp_entry_t entry)* This bit will be set if the device is not on the plist and not * usable, will be cleared if the device is on the plist. */ -#define SWAP_USAGE_OFFLIST_BIT (1UL << (BITS_PER_TYPE(atomic_t) - 2)) +#define SWAP_USAGE_OFFLIST_BIT (1UL << (BITS_PER_TYPE(atomic_long_t) - 2))
That's a very clean fix, thanks!