Skip to content

Commit e679b26

Browse files
Kemeng Shigregkh
authored andcommitted
mm: swap: correctly use maxpages in swapon syscall to avoid potential deadloop
commit 255116c upstream. 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> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 9128ecb commit e679b26

1 file changed

Lines changed: 26 additions & 27 deletions

File tree

mm/swapfile.c

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

3141-
static int setup_swap_map_and_extents(struct swap_info_struct *si,
3142-
union swap_header *swap_header,
3143-
unsigned char *swap_map,
3144-
unsigned long maxpages,
3145-
sector_t *span)
3141+
static int setup_swap_map(struct swap_info_struct *si,
3142+
union swap_header *swap_header,
3143+
unsigned char *swap_map,
3144+
unsigned long maxpages)
31463145
{
3147-
unsigned int nr_good_pages;
31483146
unsigned long i;
3149-
int nr_extents;
3150-
3151-
nr_good_pages = maxpages - 1; /* omit header page */
31523147

3148+
swap_map[0] = SWAP_MAP_BAD; /* omit header page */
31533149
for (i = 0; i < swap_header->info.nr_badpages; i++) {
31543150
unsigned int page_nr = swap_header->info.badpages[i];
31553151
if (page_nr == 0 || page_nr > swap_header->info.last_page)
31563152
return -EINVAL;
31573153
if (page_nr < maxpages) {
31583154
swap_map[page_nr] = SWAP_MAP_BAD;
3159-
nr_good_pages--;
3155+
si->pages--;
31603156
}
31613157
}
31623158

3163-
if (nr_good_pages) {
3164-
swap_map[0] = SWAP_MAP_BAD;
3165-
si->max = maxpages;
3166-
si->pages = nr_good_pages;
3167-
nr_extents = setup_swap_extents(si, span);
3168-
if (nr_extents < 0)
3169-
return nr_extents;
3170-
nr_good_pages = si->pages;
3171-
}
3172-
if (!nr_good_pages) {
3159+
if (!si->pages) {
31733160
pr_warn("Empty swap-file\n");
31743161
return -EINVAL;
31753162
}
31763163

3177-
return nr_extents;
3164+
return 0;
31783165
}
31793166

31803167
#define SWAP_CLUSTER_INFO_COLS \
@@ -3214,7 +3201,7 @@ static struct swap_cluster_info *setup_clusters(struct swap_info_struct *si,
32143201
* Mark unusable pages as unavailable. The clusters aren't
32153202
* marked free yet, so no list operations are involved yet.
32163203
*
3217-
* See setup_swap_map_and_extents(): header page, bad pages,
3204+
* See setup_swap_map(): header page, bad pages,
32183205
* and the EOF part of the last cluster.
32193206
*/
32203207
inc_cluster_info_page(si, cluster_info, 0);
@@ -3360,6 +3347,21 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
33603347
goto bad_swap_unlock_inode;
33613348
}
33623349

3350+
si->max = maxpages;
3351+
si->pages = maxpages - 1;
3352+
nr_extents = setup_swap_extents(si, &span);
3353+
if (nr_extents < 0) {
3354+
error = nr_extents;
3355+
goto bad_swap_unlock_inode;
3356+
}
3357+
if (si->pages != si->max - 1) {
3358+
pr_err("swap:%u != (max:%u - 1)\n", si->pages, si->max);
3359+
error = -EINVAL;
3360+
goto bad_swap_unlock_inode;
3361+
}
3362+
3363+
maxpages = si->max;
3364+
33633365
/* OK, set up the swap map and apply the bad block list */
33643366
swap_map = vzalloc(maxpages);
33653367
if (!swap_map) {
@@ -3371,12 +3373,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
33713373
if (error)
33723374
goto bad_swap_unlock_inode;
33733375

3374-
nr_extents = setup_swap_map_and_extents(si, swap_header, swap_map,
3375-
maxpages, &span);
3376-
if (unlikely(nr_extents < 0)) {
3377-
error = nr_extents;
3376+
error = setup_swap_map(si, swap_header, swap_map, maxpages);
3377+
if (error)
33783378
goto bad_swap_unlock_inode;
3379-
}
33803379

33813380
/*
33823381
* Use kvmalloc_array instead of bitmap_zalloc as the allocation order might

0 commit comments

Comments
 (0)