Skip to content

Commit 255116c

Browse files
Kemeng Shiakpm00
authored andcommitted
mm: swap: correctly use maxpages in swapon syscall to avoid potential deadloop
We use maxpages from read_swap_header() to initialize swap_info_struct, however the maxpages might be reduced in setup_swap_extents() and the si->max is assigned with the reduced maxpages from the setup_swap_extents(). Obviously, this could lead to memory waste as we allocated memory based on larger maxpages, besides, this could lead to a potential deadloop as following: 1) When calling setup_clusters() with larger maxpages, unavailable pages within range [si->max, larger maxpages) are not accounted with inc_cluster_info_page(). As a result, these pages are assumed available but can not be allocated. The cluster contains these pages can be moved to frag_clusters list after it's all available pages were allocated. 2) When the cluster mentioned in 1) is the only cluster in frag_clusters list, cluster_alloc_swap_entry() assume order 0 allocation will never failed and will enter a deadloop by keep trying to allocate page from the only cluster in frag_clusters which contains no actually available page. Call setup_swap_extents() to get the final maxpages before swap_info_struct initialization to fix the issue. After this change, span will include badblocks and will become large value which I think is correct value: In summary, there are two kinds of swapfile_activate operations. 1. Filesystem style: Treat all blocks logical continuity and find usable physical extents in logical range. In this way, si->pages will be actual usable physical blocks and span will be "1 + highest_block - lowest_block". 2. Block device style: Treat all blocks physically continue and only one single extent is added. In this way, si->pages will be si->max and span will be "si->pages - 1". Actually, si->pages and si->max is only used in block device style and span value is set with si->pages. As a result, span value in block device style will become a larger value as you mentioned. I think larger value is correct based on: 1. Span value in filesystem style is "1 + highest_block - lowest_block" which is the range cover all possible phisical blocks including the badblocks. 2. For block device style, si->pages is the actual usable block number and is already in pr_info. The original span value before this patch is also refer to usable block number which is redundant in pr_info. [shikemeng@huaweicloud.com: ensure si->pages == si->max - 1 after setup_swap_extents()] Link: https://lkml.kernel.org/r/20250522122554.12209-3-shikemeng@huaweicloud.com Link: https://lkml.kernel.org/r/20250718065139.61989-1-shikemeng@huaweicloud.com Link: https://lkml.kernel.org/r/20250522122554.12209-3-shikemeng@huaweicloud.com Fixes: 661383c ("mm: swap: relaim the cached parts that got scanned") Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> Reviewed-by: Baoquan He <bhe@redhat.com> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Kairui Song <kasong@tencent.com> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
1 parent 4f78252 commit 255116c

File tree

1 file changed

+26
-27
lines changed

1 file changed

+26
-27
lines changed

mm/swapfile.c

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3141,43 +3141,30 @@ static unsigned long read_swap_header(struct swap_info_struct *si,
31413141
return maxpages;
31423142
}
31433143

3144-
static int setup_swap_map_and_extents(struct swap_info_struct *si,
3145-
union swap_header *swap_header,
3146-
unsigned char *swap_map,
3147-
unsigned long maxpages,
3148-
sector_t *span)
3144+
static int setup_swap_map(struct swap_info_struct *si,
3145+
union swap_header *swap_header,
3146+
unsigned char *swap_map,
3147+
unsigned long maxpages)
31493148
{
3150-
unsigned int nr_good_pages;
31513149
unsigned long i;
3152-
int nr_extents;
3153-
3154-
nr_good_pages = maxpages - 1; /* omit header page */
31553150

3151+
swap_map[0] = SWAP_MAP_BAD; /* omit header page */
31563152
for (i = 0; i < swap_header->info.nr_badpages; i++) {
31573153
unsigned int page_nr = swap_header->info.badpages[i];
31583154
if (page_nr == 0 || page_nr > swap_header->info.last_page)
31593155
return -EINVAL;
31603156
if (page_nr < maxpages) {
31613157
swap_map[page_nr] = SWAP_MAP_BAD;
3162-
nr_good_pages--;
3158+
si->pages--;
31633159
}
31643160
}
31653161

3166-
if (nr_good_pages) {
3167-
swap_map[0] = SWAP_MAP_BAD;
3168-
si->max = maxpages;
3169-
si->pages = nr_good_pages;
3170-
nr_extents = setup_swap_extents(si, span);
3171-
if (nr_extents < 0)
3172-
return nr_extents;
3173-
nr_good_pages = si->pages;
3174-
}
3175-
if (!nr_good_pages) {
3162+
if (!si->pages) {
31763163
pr_warn("Empty swap-file\n");
31773164
return -EINVAL;
31783165
}
31793166

3180-
return nr_extents;
3167+
return 0;
31813168
}
31823169

31833170
#define SWAP_CLUSTER_INFO_COLS \
@@ -3217,7 +3204,7 @@ static struct swap_cluster_info *setup_clusters(struct swap_info_struct *si,
32173204
* Mark unusable pages as unavailable. The clusters aren't
32183205
* marked free yet, so no list operations are involved yet.
32193206
*
3220-
* See setup_swap_map_and_extents(): header page, bad pages,
3207+
* See setup_swap_map(): header page, bad pages,
32213208
* and the EOF part of the last cluster.
32223209
*/
32233210
inc_cluster_info_page(si, cluster_info, 0);
@@ -3363,6 +3350,21 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
33633350
goto bad_swap_unlock_inode;
33643351
}
33653352

3353+
si->max = maxpages;
3354+
si->pages = maxpages - 1;
3355+
nr_extents = setup_swap_extents(si, &span);
3356+
if (nr_extents < 0) {
3357+
error = nr_extents;
3358+
goto bad_swap_unlock_inode;
3359+
}
3360+
if (si->pages != si->max - 1) {
3361+
pr_err("swap:%u != (max:%u - 1)\n", si->pages, si->max);
3362+
error = -EINVAL;
3363+
goto bad_swap_unlock_inode;
3364+
}
3365+
3366+
maxpages = si->max;
3367+
33663368
/* OK, set up the swap map and apply the bad block list */
33673369
swap_map = vzalloc(maxpages);
33683370
if (!swap_map) {
@@ -3374,12 +3376,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
33743376
if (error)
33753377
goto bad_swap_unlock_inode;
33763378

3377-
nr_extents = setup_swap_map_and_extents(si, swap_header, swap_map,
3378-
maxpages, &span);
3379-
if (unlikely(nr_extents < 0)) {
3380-
error = nr_extents;
3379+
error = setup_swap_map(si, swap_header, swap_map, maxpages);
3380+
if (error)
33813381
goto bad_swap_unlock_inode;
3382-
}
33833382

33843383
/*
33853384
* Use kvmalloc_array instead of bitmap_zalloc as the allocation order might

0 commit comments

Comments
 (0)