From e0428a266d5a29a3c2eec287fcd49be9e8e2468e Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 9 Oct 2017 12:15:36 -0700 Subject: fscrypt: switch from ->is_encrypted() to IS_ENCRYPTED() IS_ENCRYPTED() now gives the same information as i_sb->s_cop->is_encrypted() but is more efficient, since IS_ENCRYPTED() is just a simple flag check. Prepare to remove ->is_encrypted() by switching all callers to IS_ENCRYPTED(). Acked-by: Dave Chinner Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/crypto/crypto.c') diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c index c7835df7e7b8..608f6bbe0f31 100644 --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -340,7 +340,7 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags) return -ECHILD; dir = dget_parent(dentry); - if (!d_inode(dir)->i_sb->s_cop->is_encrypted(d_inode(dir))) { + if (!IS_ENCRYPTED(d_inode(dir))) { dput(dir); return 0; } -- cgit v1.2.3 From a0b3bc855374c50b5ea85273553485af48caf2f7 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sun, 29 Oct 2017 06:30:19 -0400 Subject: fscrypt: lock mutex before checking for bounce page pool 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. Cc: stable@vger.kernel.org # v4.1+ Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/crypto.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'fs/crypto/crypto.c') diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c index 608f6bbe0f31..472326737717 100644 --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -410,11 +410,8 @@ int fscrypt_initialize(unsigned int cop_flags) { 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); -- cgit v1.2.3