* [PATCH] mm: zswap: fix missing folio cleanup in writeback race path
@ 2024-01-25 8:51 Yosry Ahmed
2024-01-25 8:54 ` Chengming Zhou
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Yosry Ahmed @ 2024-01-25 8:51 UTC (permalink / raw)
To: Andrew Morton
Cc: Johannes Weiner, Nhat Pham, Chengming Zhou, Domenico Cerasuolo,
linux-mm, linux-kernel, Yosry Ahmed, stable
In zswap_writeback_entry(), after we get a folio from
__read_swap_cache_async(), we grab the tree lock again to check that the
swap entry was not invalidated and recycled. If it was, we delete the
folio we just added to the swap cache and exit.
However, __read_swap_cache_async() returns the folio locked when it is
newly allocated, which is always true for this path, and the folio is
ref'd. Make sure to unlock and put the folio before returning.
This was discovered by code inspection, probably because this path
handles a race condition that should not happen often, and the bug would
not crash the system, it will only strand the folio indefinitely.
Fixes: 04fc7816089c ("mm: fix zswap writeback race condition")
Cc: stable@vger.kernel.org
Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
---
mm/zswap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/zswap.c b/mm/zswap.c
index 8f4a7efc2bdae..00e90b9b5417d 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1448,6 +1448,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
if (zswap_rb_search(&tree->rbroot, swp_offset(entry->swpentry)) != entry) {
spin_unlock(&tree->lock);
delete_from_swap_cache(folio);
+ folio_unlock(folio);
+ folio_put(folio);
return -ENOMEM;
}
spin_unlock(&tree->lock);
--
2.43.0.429.g432eaa2c6b-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mm: zswap: fix missing folio cleanup in writeback race path
2024-01-25 8:51 Yosry Ahmed
@ 2024-01-25 8:54 ` Chengming Zhou
2024-01-25 14:49 ` Johannes Weiner
2024-01-25 18:29 ` Nhat Pham
2 siblings, 0 replies; 9+ messages in thread
From: Chengming Zhou @ 2024-01-25 8:54 UTC (permalink / raw)
To: Yosry Ahmed, Andrew Morton
Cc: Johannes Weiner, Nhat Pham, Domenico Cerasuolo, linux-mm,
linux-kernel, stable
On 2024/1/25 16:51, Yosry Ahmed wrote:
> In zswap_writeback_entry(), after we get a folio from
> __read_swap_cache_async(), we grab the tree lock again to check that the
> swap entry was not invalidated and recycled. If it was, we delete the
> folio we just added to the swap cache and exit.
>
> However, __read_swap_cache_async() returns the folio locked when it is
> newly allocated, which is always true for this path, and the folio is
> ref'd. Make sure to unlock and put the folio before returning.
>
> This was discovered by code inspection, probably because this path
> handles a race condition that should not happen often, and the bug would
> not crash the system, it will only strand the folio indefinitely.
>
> Fixes: 04fc7816089c ("mm: fix zswap writeback race condition")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
LGTM, thanks!
Reviewed-by: Chengming Zhou <zhouchengming@bytedance.com>
> ---
> mm/zswap.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 8f4a7efc2bdae..00e90b9b5417d 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1448,6 +1448,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
> if (zswap_rb_search(&tree->rbroot, swp_offset(entry->swpentry)) != entry) {
> spin_unlock(&tree->lock);
> delete_from_swap_cache(folio);
> + folio_unlock(folio);
> + folio_put(folio);
> return -ENOMEM;
> }
> spin_unlock(&tree->lock);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm: zswap: fix missing folio cleanup in writeback race path
2024-01-25 8:51 Yosry Ahmed
2024-01-25 8:54 ` Chengming Zhou
@ 2024-01-25 14:49 ` Johannes Weiner
2024-01-25 18:29 ` Nhat Pham
2 siblings, 0 replies; 9+ messages in thread
From: Johannes Weiner @ 2024-01-25 14:49 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Andrew Morton, Nhat Pham, Chengming Zhou, Domenico Cerasuolo,
linux-mm, linux-kernel, stable
On Thu, Jan 25, 2024 at 08:51:27AM +0000, Yosry Ahmed wrote:
> In zswap_writeback_entry(), after we get a folio from
> __read_swap_cache_async(), we grab the tree lock again to check that the
> swap entry was not invalidated and recycled. If it was, we delete the
> folio we just added to the swap cache and exit.
>
> However, __read_swap_cache_async() returns the folio locked when it is
> newly allocated, which is always true for this path, and the folio is
> ref'd. Make sure to unlock and put the folio before returning.
>
> This was discovered by code inspection, probably because this path
> handles a race condition that should not happen often, and the bug would
> not crash the system, it will only strand the folio indefinitely.
>
> Fixes: 04fc7816089c ("mm: fix zswap writeback race condition")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
Ouch, good catch.
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm: zswap: fix missing folio cleanup in writeback race path
2024-01-25 8:51 Yosry Ahmed
2024-01-25 8:54 ` Chengming Zhou
2024-01-25 14:49 ` Johannes Weiner
@ 2024-01-25 18:29 ` Nhat Pham
2 siblings, 0 replies; 9+ messages in thread
From: Nhat Pham @ 2024-01-25 18:29 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Andrew Morton, Johannes Weiner, Chengming Zhou,
Domenico Cerasuolo, linux-mm, linux-kernel, stable
On Thu, Jan 25, 2024 at 12:51 AM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> In zswap_writeback_entry(), after we get a folio from
> __read_swap_cache_async(), we grab the tree lock again to check that the
> swap entry was not invalidated and recycled. If it was, we delete the
> folio we just added to the swap cache and exit.
>
> However, __read_swap_cache_async() returns the folio locked when it is
> newly allocated, which is always true for this path, and the folio is
> ref'd. Make sure to unlock and put the folio before returning.
>
> This was discovered by code inspection, probably because this path
> handles a race condition that should not happen often, and the bug would
> not crash the system, it will only strand the folio indefinitely.
>
> Fixes: 04fc7816089c ("mm: fix zswap writeback race condition")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> ---
> mm/zswap.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 8f4a7efc2bdae..00e90b9b5417d 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1448,6 +1448,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
> if (zswap_rb_search(&tree->rbroot, swp_offset(entry->swpentry)) != entry) {
> spin_unlock(&tree->lock);
> delete_from_swap_cache(folio);
> + folio_unlock(folio);
> + folio_put(folio);
> return -ENOMEM;
> }
> spin_unlock(&tree->lock);
> --
> 2.43.0.429.g432eaa2c6b-goog
>
Oof. Yeah this is probably rare IRL (that looks like a very specific
race condition), and the symptoms are rather subtle (no kernel crash).
LGTM.
Reviewed-by: Nhat Pham <nphamcs@gmail.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* FAILED: patch "[PATCH] mm: zswap: fix missing folio cleanup in writeback race path" failed to apply to 6.6-stable tree
@ 2024-02-26 10:30 gregkh
2024-02-26 22:10 ` [PATCH] mm: zswap: fix missing folio cleanup in writeback race path Yosry Ahmed
0 siblings, 1 reply; 9+ messages in thread
From: gregkh @ 2024-02-26 10:30 UTC (permalink / raw)
To: yosryahmed, akpm, cerasuolodomenico, hannes, nphamcs, stable,
zhouchengming
Cc: stable
The patch below does not apply to the 6.6-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
git checkout FETCH_HEAD
git cherry-pick -x e3b63e966cac0bf78aaa1efede1827a252815a1d
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2024022611-tropics-deferred-2483@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..
Possible dependencies:
e3b63e966cac ("mm: zswap: fix missing folio cleanup in writeback race path")
96c7b0b42239 ("mm: return the folio from __read_swap_cache_async()")
e947ba0bbf47 ("mm/zswap: cleanup zswap_writeback_entry()")
32acba4c0483 ("mm/zswap: refactor out __zswap_load()")
c75f5c1e0f1d ("mm/zswap: reuse dstmem when decompress")
b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure")
a65b0e7607cc ("zswap: make shrinking memcg-aware")
ddc1a5cbc05d ("mempolicy: alloc_pages_mpol() for NUMA policy without vma")
23e4883248f0 ("mm: add page_rmappable_folio() wrapper")
c36f6e6dff4d ("mempolicy trivia: slightly more consistent naming")
7f1ee4e20708 ("mempolicy trivia: delete those ancient pr_debug()s")
1cb5d11a370f ("mempolicy: fix migrate_pages(2) syscall return nr_failed")
3657fdc2451a ("mm: move vma_policy() and anon_vma_name() decls to mm_types.h")
3022fd7af960 ("shmem: _add_to_page_cache() before shmem_inode_acct_blocks()")
054a9f7ccd0a ("shmem: move memcg charge out of shmem_add_to_page_cache()")
4199f51a7eb2 ("shmem: shmem_acct_blocks() and shmem_inode_acct_blocks()")
e3e1a5067fd2 ("shmem: remove vma arg from shmem_get_folio_gfp()")
75c70128a673 ("mm: mempolicy: make mpol_misplaced() to take a folio")
cda6d93672ac ("mm: memory: make numa_migrate_prep() to take a folio")
6695cf68b15c ("mm: memory: use a folio in do_numa_page()")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From e3b63e966cac0bf78aaa1efede1827a252815a1d Mon Sep 17 00:00:00 2001
From: Yosry Ahmed <yosryahmed@google.com>
Date: Thu, 25 Jan 2024 08:51:27 +0000
Subject: [PATCH] mm: zswap: fix missing folio cleanup in writeback race path
In zswap_writeback_entry(), after we get a folio from
__read_swap_cache_async(), we grab the tree lock again to check that the
swap entry was not invalidated and recycled. If it was, we delete the
folio we just added to the swap cache and exit.
However, __read_swap_cache_async() returns the folio locked when it is
newly allocated, which is always true for this path, and the folio is
ref'd. Make sure to unlock and put the folio before returning.
This was discovered by code inspection, probably because this path handles
a race condition that should not happen often, and the bug would not crash
the system, it will only strand the folio indefinitely.
Link: https://lkml.kernel.org/r/20240125085127.1327013-1-yosryahmed@google.com
Fixes: 04fc7816089c ("mm: fix zswap writeback race condition")
Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
Reviewed-by: Chengming Zhou <zhouchengming@bytedance.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Reviewed-by: Nhat Pham <nphamcs@gmail.com>
Cc: Domenico Cerasuolo <cerasuolodomenico@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
diff --git a/mm/zswap.c b/mm/zswap.c
index 350dd2fc8159..d2423247acfd 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1440,6 +1440,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
if (zswap_rb_search(&tree->rbroot, swp_offset(entry->swpentry)) != entry) {
spin_unlock(&tree->lock);
delete_from_swap_cache(folio);
+ folio_unlock(folio);
+ folio_put(folio);
return -ENOMEM;
}
spin_unlock(&tree->lock);
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] mm: zswap: fix missing folio cleanup in writeback race path
2024-02-26 10:30 FAILED: patch "[PATCH] mm: zswap: fix missing folio cleanup in writeback race path" failed to apply to 6.6-stable tree gregkh
@ 2024-02-26 22:10 ` Yosry Ahmed
2024-02-27 8:55 ` Greg KH
0 siblings, 1 reply; 9+ messages in thread
From: Yosry Ahmed @ 2024-02-26 22:10 UTC (permalink / raw)
To: stable
Cc: Yosry Ahmed, Chengming Zhou, Johannes Weiner, Nhat Pham,
Domenico Cerasuolo, Andrew Morton
In zswap_writeback_entry(), after we get a folio from
__read_swap_cache_async(), we grab the tree lock again to check that the
swap entry was not invalidated and recycled. If it was, we delete the
folio we just added to the swap cache and exit.
However, __read_swap_cache_async() returns the folio locked when it is
newly allocated, which is always true for this path, and the folio is
ref'd. Make sure to unlock and put the folio before returning.
This was discovered by code inspection, probably because this path handles
a race condition that should not happen often, and the bug would not crash
the system, it will only strand the folio indefinitely.
Link: https://lkml.kernel.org/r/20240125085127.1327013-1-yosryahmed@google.com
Fixes: 04fc7816089c ("mm: fix zswap writeback race condition")
Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
Reviewed-by: Chengming Zhou <zhouchengming@bytedance.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Reviewed-by: Nhat Pham <nphamcs@gmail.com>
Cc: Domenico Cerasuolo <cerasuolodomenico@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit e3b63e966cac0bf78aaa1efede1827a252815a1d)
Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
---
mm/zswap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/zswap.c b/mm/zswap.c
index 37d2b1cb2ecb4..4e0f0a7b30737 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1100,6 +1100,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
if (zswap_rb_search(&tree->rbroot, swp_offset(entry->swpentry)) != entry) {
spin_unlock(&tree->lock);
delete_from_swap_cache(page_folio(page));
+ unlock_page(page);
+ put_page(page);
ret = -ENOMEM;
goto fail;
}
--
2.44.0.rc1.240.g4c46232300-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mm: zswap: fix missing folio cleanup in writeback race path
2024-02-26 22:10 ` [PATCH] mm: zswap: fix missing folio cleanup in writeback race path Yosry Ahmed
@ 2024-02-27 8:55 ` Greg KH
2024-02-27 8:58 ` Yosry Ahmed
0 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2024-02-27 8:55 UTC (permalink / raw)
To: Yosry Ahmed
Cc: stable, Chengming Zhou, Johannes Weiner, Nhat Pham,
Domenico Cerasuolo, Andrew Morton
On Mon, Feb 26, 2024 at 10:10:17PM +0000, Yosry Ahmed wrote:
> In zswap_writeback_entry(), after we get a folio from
> __read_swap_cache_async(), we grab the tree lock again to check that the
> swap entry was not invalidated and recycled. If it was, we delete the
> folio we just added to the swap cache and exit.
>
> However, __read_swap_cache_async() returns the folio locked when it is
> newly allocated, which is always true for this path, and the folio is
> ref'd. Make sure to unlock and put the folio before returning.
>
> This was discovered by code inspection, probably because this path handles
> a race condition that should not happen often, and the bug would not crash
> the system, it will only strand the folio indefinitely.
>
> Link: https://lkml.kernel.org/r/20240125085127.1327013-1-yosryahmed@google.com
> Fixes: 04fc7816089c ("mm: fix zswap writeback race condition")
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> Reviewed-by: Chengming Zhou <zhouchengming@bytedance.com>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> Cc: Domenico Cerasuolo <cerasuolodomenico@gmail.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> (cherry picked from commit e3b63e966cac0bf78aaa1efede1827a252815a1d)
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
What tree is this for?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm: zswap: fix missing folio cleanup in writeback race path
2024-02-27 8:55 ` Greg KH
@ 2024-02-27 8:58 ` Yosry Ahmed
2024-02-27 10:17 ` Greg KH
0 siblings, 1 reply; 9+ messages in thread
From: Yosry Ahmed @ 2024-02-27 8:58 UTC (permalink / raw)
To: Greg KH
Cc: stable, Chengming Zhou, Johannes Weiner, Nhat Pham,
Domenico Cerasuolo, Andrew Morton
On Tue, Feb 27, 2024 at 12:55 AM Greg KH <greg@kroah.com> wrote:
>
> On Mon, Feb 26, 2024 at 10:10:17PM +0000, Yosry Ahmed wrote:
> > In zswap_writeback_entry(), after we get a folio from
> > __read_swap_cache_async(), we grab the tree lock again to check that the
> > swap entry was not invalidated and recycled. If it was, we delete the
> > folio we just added to the swap cache and exit.
> >
> > However, __read_swap_cache_async() returns the folio locked when it is
> > newly allocated, which is always true for this path, and the folio is
> > ref'd. Make sure to unlock and put the folio before returning.
> >
> > This was discovered by code inspection, probably because this path handles
> > a race condition that should not happen often, and the bug would not crash
> > the system, it will only strand the folio indefinitely.
> >
> > Link: https://lkml.kernel.org/r/20240125085127.1327013-1-yosryahmed@google.com
> > Fixes: 04fc7816089c ("mm: fix zswap writeback race condition")
> > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> > Reviewed-by: Chengming Zhou <zhouchengming@bytedance.com>
> > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> > Cc: Domenico Cerasuolo <cerasuolodomenico@gmail.com>
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > (cherry picked from commit e3b63e966cac0bf78aaa1efede1827a252815a1d)
> > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
>
> What tree is this for?
This is for 6.6.y, sorry I messed up the subject prefix. Do I need to resend?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm: zswap: fix missing folio cleanup in writeback race path
2024-02-27 8:58 ` Yosry Ahmed
@ 2024-02-27 10:17 ` Greg KH
0 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2024-02-27 10:17 UTC (permalink / raw)
To: Yosry Ahmed
Cc: stable, Chengming Zhou, Johannes Weiner, Nhat Pham,
Domenico Cerasuolo, Andrew Morton
On Tue, Feb 27, 2024 at 12:58:27AM -0800, Yosry Ahmed wrote:
> On Tue, Feb 27, 2024 at 12:55 AM Greg KH <greg@kroah.com> wrote:
> >
> > On Mon, Feb 26, 2024 at 10:10:17PM +0000, Yosry Ahmed wrote:
> > > In zswap_writeback_entry(), after we get a folio from
> > > __read_swap_cache_async(), we grab the tree lock again to check that the
> > > swap entry was not invalidated and recycled. If it was, we delete the
> > > folio we just added to the swap cache and exit.
> > >
> > > However, __read_swap_cache_async() returns the folio locked when it is
> > > newly allocated, which is always true for this path, and the folio is
> > > ref'd. Make sure to unlock and put the folio before returning.
> > >
> > > This was discovered by code inspection, probably because this path handles
> > > a race condition that should not happen often, and the bug would not crash
> > > the system, it will only strand the folio indefinitely.
> > >
> > > Link: https://lkml.kernel.org/r/20240125085127.1327013-1-yosryahmed@google.com
> > > Fixes: 04fc7816089c ("mm: fix zswap writeback race condition")
> > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> > > Reviewed-by: Chengming Zhou <zhouchengming@bytedance.com>
> > > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > > Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> > > Cc: Domenico Cerasuolo <cerasuolodomenico@gmail.com>
> > > Cc: <stable@vger.kernel.org>
> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > > (cherry picked from commit e3b63e966cac0bf78aaa1efede1827a252815a1d)
> > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> >
> > What tree is this for?
>
> This is for 6.6.y, sorry I messed up the subject prefix. Do I need to resend?
Ah, here is the 6.6.y version, missed it, sorry for the noise. I'll go
queue this and the 6.1.y version up now.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-02-27 10:17 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-26 10:30 FAILED: patch "[PATCH] mm: zswap: fix missing folio cleanup in writeback race path" failed to apply to 6.6-stable tree gregkh
2024-02-26 22:10 ` [PATCH] mm: zswap: fix missing folio cleanup in writeback race path Yosry Ahmed
2024-02-27 8:55 ` Greg KH
2024-02-27 8:58 ` Yosry Ahmed
2024-02-27 10:17 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2024-01-25 8:51 Yosry Ahmed
2024-01-25 8:54 ` Chengming Zhou
2024-01-25 14:49 ` Johannes Weiner
2024-01-25 18:29 ` Nhat Pham
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox