Skip to contentSkip to conversation

Patch "fscrypt: lock mutex before checking for bounce page pool" has been added to the 4.14-stable tree

PATCH [email protected] 1 participant 1 message Nov 27, 2017

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

fscrypt: lock mutex before checking for bounce page pool

to the 4.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
fscrypt-lock-mutex-before-checking-for-bounce-page-pool.patch
and it can be found in the queue-4.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From a0b3bc855374c50b5ea85273553485af48caf2f7 Mon Sep 17 00:00:00 2001

From: Eric Biggers <[email protected]>
Date: Sun, 29 Oct 2017 06:30:19 -0400
Subject: fscrypt: lock mutex before checking for bounce page pool

From: Eric Biggers <[email protected]>

commit a0b3bc855374c50b5ea85273553485af48caf2f7 upstream.

fscrypt_initialize(), which allocates the global bounce page pool when an encrypted file is first accessed, uses "double-checked locking" to try to avoid locking fscrypt_init_mutex. However, it doesn't use any memory barriers, so it's theoretically possible for a thread to observe a bounce page pool which has not been fully initialized. This is a classic bug with "double-checked locking".

While "only a theoretical issue" in the latest kernel, in pre-4.8 kernels the pointer that was checked was not even the last to be initialized, so it was easily possible for a crash (NULL pointer dereference) to happen. This was changed only incidentally by the large refactor to use fs/crypto/.

Solve both problems in a trivial way that can easily be backported: just always take the mutex. It's theoretically less efficient, but it shouldn't be noticeable in practice as the mutex is only acquired very briefly once per encrypted file.

Later I'd like to make this use a helper macro like DO_ONCE(). However, DO_ONCE() runs in atomic context, so we'd need to add a new macro that allows blocking.

---
 fs/crypto/crypto.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
fs/crypto/crypto.c +2 -5
@@ -410,11 +410,8 @@ int fscrypt_initialize(unsigned int cop_
 {
 	int i, res = -ENOMEM;
 
-	/*
-	 * No need to allocate a bounce page pool if there already is one or
-	 * this FS won't use it.
-	 */
-	if (cop_flags & FS_CFLG_OWN_PAGES || fscrypt_bounce_page_pool)
+	/* No need to allocate a bounce page pool if this FS won't use it. */
+	if (cop_flags & FS_CFLG_OWN_PAGES)
 		return 0;
 
 	mutex_lock(&fscrypt_init_mutex);

Patches currently in stable-queue which might be from [email protected] are

queue-4.14/lib-mpi-call-cond_resched-from-mpi_powm-loop.patch queue-4.14/fscrypt-lock-mutex-before-checking-for-bounce-page-pool.patch queue-4.14/dm-bufio-fix-integer-overflow-when-limiting-maximum-cache-size.patch queue-4.14/libceph-don-t-warn-if-user-tries-to-add-invalid-key.patch