* FAILED: patch "[PATCH] ext4: fix deadlock allocating crypto bounce page from mempool" failed to apply to 4.19-stable tree
@ 2020-02-09 11:26 gregkh
2020-02-09 15:57 ` Sasha Levin
0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2020-02-09 11:26 UTC (permalink / raw)
To: ebiggers, tytso; +Cc: stable
The patch below does not apply to the 4.19-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>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 547c556f4db7c09447ecf5f833ab6aaae0c5ab58 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Tue, 31 Dec 2019 12:11:49 -0600
Subject: [PATCH] ext4: fix deadlock allocating crypto bounce page from mempool
ext4_writepages() on an encrypted file has to encrypt the data, but it
can't modify the pagecache pages in-place, so it encrypts the data into
bounce pages and writes those instead. All bounce pages are allocated
from a mempool using GFP_NOFS.
This is not correct use of a mempool, and it can deadlock. This is
because GFP_NOFS includes __GFP_DIRECT_RECLAIM, which enables the "never
fail" mode for mempool_alloc() where a failed allocation will fall back
to waiting for one of the preallocated elements in the pool.
But since this mode is used for all a bio's pages and not just the
first, it can deadlock waiting for pages already in the bio to be freed.
This deadlock can be reproduced by patching mempool_alloc() to pretend
that pool->alloc() always fails (so that it always falls back to the
preallocations), and then creating an encrypted file of size > 128 KiB.
Fix it by only using GFP_NOFS for the first page in the bio. For
subsequent pages just use GFP_NOWAIT, and if any of those fail, just
submit the bio and start a new one.
This will need to be fixed in f2fs too, but that's less straightforward.
Fixes: c9af28fdd449 ("ext4 crypto: don't let data integrity writebacks fail with ENOMEM")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
Link: https://lore.kernel.org/r/20191231181149.47619-1-ebiggers@kernel.org
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 24aeedb8fc75..68b39e75446a 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -512,17 +512,26 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
gfp_t gfp_flags = GFP_NOFS;
unsigned int enc_bytes = round_up(len, i_blocksize(inode));
+ /*
+ * Since bounce page allocation uses a mempool, we can only use
+ * a waiting mask (i.e. request guaranteed allocation) on the
+ * first page of the bio. Otherwise it can deadlock.
+ */
+ if (io->io_bio)
+ gfp_flags = GFP_NOWAIT | __GFP_NOWARN;
retry_encrypt:
bounce_page = fscrypt_encrypt_pagecache_blocks(page, enc_bytes,
0, gfp_flags);
if (IS_ERR(bounce_page)) {
ret = PTR_ERR(bounce_page);
- if (ret == -ENOMEM && wbc->sync_mode == WB_SYNC_ALL) {
- if (io->io_bio) {
+ if (ret == -ENOMEM &&
+ (io->io_bio || wbc->sync_mode == WB_SYNC_ALL)) {
+ gfp_flags = GFP_NOFS;
+ if (io->io_bio)
ext4_io_submit(io);
- congestion_wait(BLK_RW_ASYNC, HZ/50);
- }
- gfp_flags |= __GFP_NOFAIL;
+ else
+ gfp_flags |= __GFP_NOFAIL;
+ congestion_wait(BLK_RW_ASYNC, HZ/50);
goto retry_encrypt;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: FAILED: patch "[PATCH] ext4: fix deadlock allocating crypto bounce page from mempool" failed to apply to 4.19-stable tree
2020-02-09 11:26 FAILED: patch "[PATCH] ext4: fix deadlock allocating crypto bounce page from mempool" failed to apply to 4.19-stable tree gregkh
@ 2020-02-09 15:57 ` Sasha Levin
2020-02-09 17:12 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Sasha Levin @ 2020-02-09 15:57 UTC (permalink / raw)
To: gregkh; +Cc: ebiggers, tytso, stable
On Sun, Feb 09, 2020 at 12:26:01PM +0100, gregkh@linuxfoundation.org wrote:
>
>The patch below does not apply to the 4.19-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>.
>
>thanks,
>
>greg k-h
>
>------------------ original commit in Linus's tree ------------------
>
>From 547c556f4db7c09447ecf5f833ab6aaae0c5ab58 Mon Sep 17 00:00:00 2001
>From: Eric Biggers <ebiggers@google.com>
>Date: Tue, 31 Dec 2019 12:11:49 -0600
>Subject: [PATCH] ext4: fix deadlock allocating crypto bounce page from mempool
>
>ext4_writepages() on an encrypted file has to encrypt the data, but it
>can't modify the pagecache pages in-place, so it encrypts the data into
>bounce pages and writes those instead. All bounce pages are allocated
>from a mempool using GFP_NOFS.
>
>This is not correct use of a mempool, and it can deadlock. This is
>because GFP_NOFS includes __GFP_DIRECT_RECLAIM, which enables the "never
>fail" mode for mempool_alloc() where a failed allocation will fall back
>to waiting for one of the preallocated elements in the pool.
>
>But since this mode is used for all a bio's pages and not just the
>first, it can deadlock waiting for pages already in the bio to be freed.
>
>This deadlock can be reproduced by patching mempool_alloc() to pretend
>that pool->alloc() always fails (so that it always falls back to the
>preallocations), and then creating an encrypted file of size > 128 KiB.
>
>Fix it by only using GFP_NOFS for the first page in the bio. For
>subsequent pages just use GFP_NOWAIT, and if any of those fail, just
>submit the bio and start a new one.
>
>This will need to be fixed in f2fs too, but that's less straightforward.
>
>Fixes: c9af28fdd449 ("ext4 crypto: don't let data integrity writebacks fail with ENOMEM")
>Cc: stable@vger.kernel.org
>Signed-off-by: Eric Biggers <ebiggers@google.com>
>Link: https://lore.kernel.org/r/20191231181149.47619-1-ebiggers@kernel.org
>Signed-off-by: Theodore Ts'o <tytso@mit.edu>
I've worked around not having these patches:
592ddec7578a ("ext4: use IS_ENCRYPTED() to check encryption status")
53bc1d854c64 ("fscrypt: support encrypting multiple filesystem blocks per page")
d2d0727b1654 ("fscrypt: simplify bounce page handling")
And queued the backport to 4.19-4.4.
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: FAILED: patch "[PATCH] ext4: fix deadlock allocating crypto bounce page from mempool" failed to apply to 4.19-stable tree
2020-02-09 15:57 ` Sasha Levin
@ 2020-02-09 17:12 ` Greg KH
0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2020-02-09 17:12 UTC (permalink / raw)
To: Sasha Levin; +Cc: ebiggers, tytso, stable
On Sun, Feb 09, 2020 at 10:57:57AM -0500, Sasha Levin wrote:
> On Sun, Feb 09, 2020 at 12:26:01PM +0100, gregkh@linuxfoundation.org wrote:
> >
> > The patch below does not apply to the 4.19-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>.
> >
> > thanks,
> >
> > greg k-h
> >
> > ------------------ original commit in Linus's tree ------------------
> >
> > > From 547c556f4db7c09447ecf5f833ab6aaae0c5ab58 Mon Sep 17 00:00:00 2001
> > From: Eric Biggers <ebiggers@google.com>
> > Date: Tue, 31 Dec 2019 12:11:49 -0600
> > Subject: [PATCH] ext4: fix deadlock allocating crypto bounce page from mempool
> >
> > ext4_writepages() on an encrypted file has to encrypt the data, but it
> > can't modify the pagecache pages in-place, so it encrypts the data into
> > bounce pages and writes those instead. All bounce pages are allocated
> > from a mempool using GFP_NOFS.
> >
> > This is not correct use of a mempool, and it can deadlock. This is
> > because GFP_NOFS includes __GFP_DIRECT_RECLAIM, which enables the "never
> > fail" mode for mempool_alloc() where a failed allocation will fall back
> > to waiting for one of the preallocated elements in the pool.
> >
> > But since this mode is used for all a bio's pages and not just the
> > first, it can deadlock waiting for pages already in the bio to be freed.
> >
> > This deadlock can be reproduced by patching mempool_alloc() to pretend
> > that pool->alloc() always fails (so that it always falls back to the
> > preallocations), and then creating an encrypted file of size > 128 KiB.
> >
> > Fix it by only using GFP_NOFS for the first page in the bio. For
> > subsequent pages just use GFP_NOWAIT, and if any of those fail, just
> > submit the bio and start a new one.
> >
> > This will need to be fixed in f2fs too, but that's less straightforward.
> >
> > Fixes: c9af28fdd449 ("ext4 crypto: don't let data integrity writebacks fail with ENOMEM")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > Link: https://lore.kernel.org/r/20191231181149.47619-1-ebiggers@kernel.org
> > Signed-off-by: Theodore Ts'o <tytso@mit.edu>
>
> I've worked around not having these patches:
>
> 592ddec7578a ("ext4: use IS_ENCRYPTED() to check encryption status")
> 53bc1d854c64 ("fscrypt: support encrypting multiple filesystem blocks per page")
> d2d0727b1654 ("fscrypt: simplify bounce page handling")
>
> And queued the backport to 4.19-4.4.
Thanks for fixing this up, and the other ones.
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-09 17:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-09 11:26 FAILED: patch "[PATCH] ext4: fix deadlock allocating crypto bounce page from mempool" failed to apply to 4.19-stable tree gregkh
2020-02-09 15:57 ` Sasha Levin
2020-02-09 17:12 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.