From: Kairui Song <ryncsn@gmail.com>
To: linux-mm@kvack.org
Cc: Andrew Morton <akpm@linux-foundation.org>,
Chris Li <chrisl@kernel.org>, Barry Song <v-songbaohua@oppo.com>,
Ryan Roberts <ryan.roberts@arm.com>,
Hugh Dickins <hughd@google.com>,
Yosry Ahmed <yosryahmed@google.com>,
"Huang, Ying" <ying.huang@intel.com>,
Tim Chen <tim.c.chen@linux.intel.com>,
Nhat Pham <nphamcs@gmail.com>,
linux-kernel@vger.kernel.org, Kairui Song <kasong@tencent.com>
Subject: [PATCH 07/13] mm, swap: hold a reference of si during scan and clean up flags
Date: Wed, 23 Oct 2024 03:24:45 +0800 [thread overview]
Message-ID: <20241022192451.38138-8-ryncsn@gmail.com> (raw)
In-Reply-To: <20241022192451.38138-1-ryncsn@gmail.com>
From: Kairui Song <kasong@tencent.com>
The flag SWP_SCANNING was used as an indicator of whether a device
is being scanned, and prevents swap off. But it's already no longer
used. The only thing protects the scanning now is the si lock.
However allocation path may drop the si lock, in theory this could
leaf to UAF.
So clean this up, just hold a reference for whole allocation path.
So per CPU counter killing will wait for existing scan and other
usage. The flag SWP_SCANNING can also be dropped.
Signed-off-by: Kairui Song <kasong@tencent.com>
---
include/linux/swap.h | 1 -
mm/swapfile.c | 62 +++++++++++++++++++++++---------------------
2 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 16dcf8bd1a4e..1651174959c8 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -219,7 +219,6 @@ enum {
SWP_STABLE_WRITES = (1 << 11), /* no overwrite PG_writeback pages */
SWP_SYNCHRONOUS_IO = (1 << 12), /* synchronous IO is efficient */
/* add others here before... */
- SWP_SCANNING = (1 << 14), /* refcount in scan_swap_map */
};
#define SWAP_CLUSTER_MAX 32UL
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4e629536a07c..d6b6e71ccc19 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1088,6 +1088,21 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
return cluster_alloc_swap(si, usage, nr, slots, order);
}
+static bool get_swap_device_info(struct swap_info_struct *si)
+{
+ if (!percpu_ref_tryget_live(&si->users))
+ return false;
+ /*
+ * Guarantee the si->users are checked before accessing other
+ * fields of swap_info_struct.
+ *
+ * Paired with the spin_unlock() after setup_swap_info() in
+ * enable_swap_info().
+ */
+ smp_rmb();
+ return true;
+}
+
int get_swap_pages(int n_goal, swp_entry_t swp_entries[], int entry_order)
{
int order = swap_entry_order(entry_order);
@@ -1115,13 +1130,16 @@ int get_swap_pages(int n_goal, swp_entry_t swp_entries[], int entry_order)
/* requeue si to after same-priority siblings */
plist_requeue(&si->avail_lists[node], &swap_avail_heads[node]);
spin_unlock(&swap_avail_lock);
- spin_lock(&si->lock);
- n_ret = scan_swap_map_slots(si, SWAP_HAS_CACHE,
- n_goal, swp_entries, order);
- spin_unlock(&si->lock);
- if (n_ret || size > 1)
- goto check_out;
- cond_resched();
+ if (get_swap_device_info(si)) {
+ spin_lock(&si->lock);
+ n_ret = scan_swap_map_slots(si, SWAP_HAS_CACHE,
+ n_goal, swp_entries, order);
+ spin_unlock(&si->lock);
+ put_swap_device(si);
+ if (n_ret || size > 1)
+ goto check_out;
+ cond_resched();
+ }
spin_lock(&swap_avail_lock);
/*
@@ -1272,16 +1290,8 @@ struct swap_info_struct *get_swap_device(swp_entry_t entry)
si = swp_swap_info(entry);
if (!si)
goto bad_nofile;
- if (!percpu_ref_tryget_live(&si->users))
+ if (!get_swap_device_info(si))
goto out;
- /*
- * Guarantee the si->users are checked before accessing other
- * fields of swap_info_struct.
- *
- * Paired with the spin_unlock() after setup_swap_info() in
- * enable_swap_info().
- */
- smp_rmb();
offset = swp_offset(entry);
if (offset >= si->max)
goto put_out;
@@ -1761,10 +1771,13 @@ swp_entry_t get_swap_page_of_type(int type)
goto fail;
/* This is called for allocating swap entry, not cache */
- spin_lock(&si->lock);
- if ((si->flags & SWP_WRITEOK) && scan_swap_map_slots(si, 1, 1, &entry, 0))
- atomic_long_dec(&nr_swap_pages);
- spin_unlock(&si->lock);
+ if (get_swap_device_info(si)) {
+ spin_lock(&si->lock);
+ if ((si->flags & SWP_WRITEOK) && scan_swap_map_slots(si, 1, 1, &entry, 0))
+ atomic_long_dec(&nr_swap_pages);
+ spin_unlock(&si->lock);
+ put_swap_device(si);
+ }
fail:
return entry;
}
@@ -2650,15 +2663,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
spin_lock(&p->lock);
drain_mmlist();
- /* wait for anyone still in scan_swap_map_slots */
- while (p->flags >= SWP_SCANNING) {
- spin_unlock(&p->lock);
- spin_unlock(&swap_lock);
- schedule_timeout_uninterruptible(1);
- spin_lock(&swap_lock);
- spin_lock(&p->lock);
- }
-
swap_file = p->swap_file;
p->swap_file = NULL;
p->max = 0;
--
2.47.0
next prev parent reply other threads:[~2024-10-22 19:30 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-22 19:24 [PATCH 00/13] mm, swap: rework of swap allocator locks Kairui Song
2024-10-22 19:24 ` [PATCH 01/13] mm, swap: minor clean up for swap entry allocation Kairui Song
2024-10-22 19:24 ` [PATCH 02/13] mm, swap: fold swap_info_get_cont in the only caller Kairui Song
2024-10-22 19:24 ` [PATCH 03/13] mm, swap: remove old allocation path for HDD Kairui Song
2024-10-22 19:24 ` [PATCH 04/13] mm, swap: use cluster lock " Kairui Song
2024-10-22 19:24 ` [PATCH 05/13] mm, swap: clean up device availability check Kairui Song
2024-10-22 19:24 ` [PATCH 06/13] mm, swap: clean up plist removal and adding Kairui Song
2024-10-22 19:24 ` Kairui Song [this message]
2024-10-22 19:24 ` [PATCH 08/13] mm, swap: use an enum to define all cluster flags and wrap flags changes Kairui Song
2024-10-22 19:24 ` [PATCH 09/13] mm, swap: reduce contention on device lock Kairui Song
2024-10-22 19:24 ` [PATCH 10/13] mm, swap: simplify percpu cluster updating Kairui Song
2024-10-22 19:24 ` [PATCH 11/13] mm, swap: introduce a helper for retrieving cluster from offset Kairui Song
2024-10-22 19:24 ` [PATCH 12/13] mm, swap: use a global swap cluster for non-rotation device Kairui Song
2024-10-22 19:37 ` [PATCH 13/13] mm, swap_slots: remove slot cache for freeing path Kairui Song
2024-10-23 2:24 ` [PATCH 00/13] mm, swap: rework of swap allocator locks Huang, Ying
2024-10-23 18:01 ` Kairui Song
2024-10-24 3:04 ` Huang, Ying
2024-10-24 3:51 ` Kairui Song
2024-10-23 10:27 ` Andrew Morton
2024-10-23 17:56 ` Kairui Song
2024-10-23 17:59 ` Yosry Ahmed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241022192451.38138-8-ryncsn@gmail.com \
--to=ryncsn@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=chrisl@kernel.org \
--cc=hughd@google.com \
--cc=kasong@tencent.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=nphamcs@gmail.com \
--cc=ryan.roberts@arm.com \
--cc=tim.c.chen@linux.intel.com \
--cc=v-songbaohua@oppo.com \
--cc=ying.huang@intel.com \
--cc=yosryahmed@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®