From cd0265fcd2eae9004c68ef2123a9dac0dc5a666a Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 18 Mar 2019 10:23:33 -0700 Subject: [PATCH 01/10] fscrypt: drop inode argument from fscrypt_get_ctx() The only reason the inode is being passed to fscrypt_get_ctx() is to verify that the encryption key is available. However, all callers already ensure this because if we get as far as trying to do I/O to an encrypted file without the key, there's already a bug. Therefore, remove this unnecessary argument. Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/bio.c | 2 +- fs/crypto/crypto.c | 16 +++++----------- fs/ext4/readpage.c | 2 +- include/linux/fscrypt.h | 5 ++--- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c index 5759bcd018cd3..f5b69b9531f68 100644 --- a/fs/crypto/bio.c +++ b/fs/crypto/bio.c @@ -104,7 +104,7 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk, BUG_ON(inode->i_sb->s_blocksize != PAGE_SIZE); - ctx = fscrypt_get_ctx(inode, GFP_NOFS); + ctx = fscrypt_get_ctx(GFP_NOFS); if (IS_ERR(ctx)) return PTR_ERR(ctx); diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c index 4dc788e3bc96b..5efc494a4e38f 100644 --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -87,23 +87,17 @@ EXPORT_SYMBOL(fscrypt_release_ctx); /** * fscrypt_get_ctx() - Gets an encryption context - * @inode: The inode for which we are doing the crypto * @gfp_flags: The gfp flag for memory allocation * * Allocates and initializes an encryption context. * - * Return: An allocated and initialized encryption context on success; error - * value or NULL otherwise. + * Return: A new encryption context on success; an ERR_PTR() otherwise. */ -struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *inode, gfp_t gfp_flags) +struct fscrypt_ctx *fscrypt_get_ctx(gfp_t gfp_flags) { - struct fscrypt_ctx *ctx = NULL; - struct fscrypt_info *ci = inode->i_crypt_info; + struct fscrypt_ctx *ctx; unsigned long flags; - if (ci == NULL) - return ERR_PTR(-ENOKEY); - /* * We first try getting the ctx from a free list because in * the common case the ctx will have an allocated and @@ -258,9 +252,9 @@ struct page *fscrypt_encrypt_page(const struct inode *inode, BUG_ON(!PageLocked(page)); - ctx = fscrypt_get_ctx(inode, gfp_flags); + ctx = fscrypt_get_ctx(gfp_flags); if (IS_ERR(ctx)) - return (struct page *)ctx; + return ERR_CAST(ctx); /* The encryption operation will require a bounce page. */ ciphertext_page = fscrypt_alloc_bounce_page(ctx, gfp_flags); diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c index 3adadf4618253..75cef6af60807 100644 --- a/fs/ext4/readpage.c +++ b/fs/ext4/readpage.c @@ -244,7 +244,7 @@ int ext4_mpage_readpages(struct address_space *mapping, struct fscrypt_ctx *ctx = NULL; if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode)) { - ctx = fscrypt_get_ctx(inode, GFP_NOFS); + ctx = fscrypt_get_ctx(GFP_NOFS); if (IS_ERR(ctx)) goto set_error_page; } diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index e5194fc3983e9..6cf8a34523ffc 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -90,7 +90,7 @@ static inline bool fscrypt_dummy_context_enabled(struct inode *inode) /* crypto.c */ extern void fscrypt_enqueue_decrypt_work(struct work_struct *); -extern struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *, gfp_t); +extern struct fscrypt_ctx *fscrypt_get_ctx(gfp_t); extern void fscrypt_release_ctx(struct fscrypt_ctx *); extern struct page *fscrypt_encrypt_page(const struct inode *, struct page *, unsigned int, unsigned int, @@ -247,8 +247,7 @@ static inline void fscrypt_enqueue_decrypt_work(struct work_struct *work) { } -static inline struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *inode, - gfp_t gfp_flags) +static inline struct fscrypt_ctx *fscrypt_get_ctx(gfp_t gfp_flags) { return ERR_PTR(-EOPNOTSUPP); } From ff5d3a97075c65731a46453d36e75b9cf925e165 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 15 Mar 2019 14:16:32 -0700 Subject: [PATCH 02/10] fscrypt: remove WARN_ON_ONCE() when decryption fails If decrypting a block fails, fscrypt did a WARN_ON_ONCE(). But WARN is meant for kernel bugs, which this isn't; this could be hit by fuzzers using fault injection, for example. Also, there is already a proper warning message logged in fscrypt_do_page_crypto(), so the WARN doesn't add much. Just remove the unnessary WARN. Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/bio.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c index f5b69b9531f68..41dde4578f3b2 100644 --- a/fs/crypto/bio.c +++ b/fs/crypto/bio.c @@ -37,12 +37,10 @@ static void __fscrypt_decrypt_bio(struct bio *bio, bool done) int ret = fscrypt_decrypt_page(page->mapping->host, page, PAGE_SIZE, 0, page->index); - if (ret) { - WARN_ON_ONCE(1); + if (ret) SetPageError(page); - } else if (done) { + else if (done) SetPageUptodate(page); - } if (done) unlock_page(page); } From e37a784d8b6a1e726de5ddc7b4809c086a08db09 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 11 Apr 2019 14:32:15 -0700 Subject: [PATCH 03/10] fscrypt: use READ_ONCE() to access ->i_crypt_info ->i_crypt_info starts out NULL and may later be locklessly set to a non-NULL value by the cmpxchg() in fscrypt_get_encryption_info(). But ->i_crypt_info is used directly, which technically is incorrect. It's a data race, and it doesn't include the data dependency barrier needed to safely dereference the pointer on at least one architecture. Fix this by using READ_ONCE() instead. Note: we don't need to use smp_load_acquire(), since dereferencing the pointer only requires a data dependency barrier, which is already included in READ_ONCE(). We also don't need READ_ONCE() in places where ->i_crypt_info is unconditionally dereferenced, since it must have already been checked. Also downgrade the cmpxchg() to cmpxchg_release(), since RELEASE semantics are sufficient on the write side. Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/crypto.c | 2 +- fs/crypto/fname.c | 4 ++-- fs/crypto/keyinfo.c | 4 ++-- fs/crypto/policy.c | 6 +++--- include/linux/fscrypt.h | 3 ++- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c index 5efc494a4e38f..0988077882574 100644 --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -328,7 +328,7 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags) spin_lock(&dentry->d_lock); cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY; spin_unlock(&dentry->d_lock); - dir_has_key = (d_inode(dir)->i_crypt_info != NULL); + dir_has_key = fscrypt_has_encryption_key(d_inode(dir)); dput(dir); /* diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c index 7ff40a73dbece..050384c79f40e 100644 --- a/fs/crypto/fname.c +++ b/fs/crypto/fname.c @@ -269,7 +269,7 @@ int fscrypt_fname_disk_to_usr(struct inode *inode, if (iname->len < FS_CRYPTO_BLOCK_SIZE) return -EUCLEAN; - if (inode->i_crypt_info) + if (fscrypt_has_encryption_key(inode)) return fname_decrypt(inode, iname, oname); if (iname->len <= FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE) { @@ -336,7 +336,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname, if (ret) return ret; - if (dir->i_crypt_info) { + if (fscrypt_has_encryption_key(dir)) { if (!fscrypt_fname_encrypted_size(dir, iname->len, dir->i_sb->s_cop->max_namelen, &fname->crypto_buf.len)) diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c index 322ce9686bdba..bf291c10c682f 100644 --- a/fs/crypto/keyinfo.c +++ b/fs/crypto/keyinfo.c @@ -509,7 +509,7 @@ int fscrypt_get_encryption_info(struct inode *inode) u8 *raw_key = NULL; int res; - if (inode->i_crypt_info) + if (fscrypt_has_encryption_key(inode)) return 0; res = fscrypt_initialize(inode->i_sb->s_cop->flags); @@ -573,7 +573,7 @@ int fscrypt_get_encryption_info(struct inode *inode) if (res) goto out; - if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) == NULL) + if (cmpxchg_release(&inode->i_crypt_info, NULL, crypt_info) == NULL) crypt_info = NULL; out: if (res == -ENOKEY) diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c index bd7eaf9b3f003..d536889ac31bf 100644 --- a/fs/crypto/policy.c +++ b/fs/crypto/policy.c @@ -194,8 +194,8 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child) res = fscrypt_get_encryption_info(child); if (res) return 0; - parent_ci = parent->i_crypt_info; - child_ci = child->i_crypt_info; + parent_ci = READ_ONCE(parent->i_crypt_info); + child_ci = READ_ONCE(child->i_crypt_info); if (parent_ci && child_ci) { return memcmp(parent_ci->ci_master_key_descriptor, @@ -246,7 +246,7 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child, if (res < 0) return res; - ci = parent->i_crypt_info; + ci = READ_ONCE(parent->i_crypt_info); if (ci == NULL) return -ENOKEY; diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index 6cf8a34523ffc..ec8ab71085994 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -79,7 +79,8 @@ struct fscrypt_ctx { static inline bool fscrypt_has_encryption_key(const struct inode *inode) { - return (inode->i_crypt_info != NULL); + /* pairs with cmpxchg_release() in fscrypt_get_encryption_info() */ + return READ_ONCE(inode->i_crypt_info) != NULL; } static inline bool fscrypt_dummy_context_enabled(struct inode *inode) From 6cc248684d3d23bbd073ae2fa73d3416c0558909 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 20 Mar 2019 11:39:09 -0700 Subject: [PATCH 04/10] fscrypt: clean up and improve dentry revalidation Make various improvements to fscrypt dentry revalidation: - Don't try to handle the case where the per-directory key is removed, as this can't happen without the inode (and dentries) being evicted. - Flag ciphertext dentries rather than plaintext dentries, since it's ciphertext dentries that need the special handling. - Avoid doing unnecessary work for non-ciphertext dentries. - When revalidating ciphertext dentries, try to set up the directory's i_crypt_info to make sure the key is really still absent, rather than invalidating all negative dentries as the previous code did. An old comment suggested we can't do this for locking reasons, but AFAICT this comment was outdated and it actually works fine. Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/crypto.c | 58 +++++++++++++++++++++-------------------- fs/crypto/hooks.c | 4 +-- include/linux/dcache.h | 2 +- include/linux/fscrypt.h | 6 ++--- 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c index 0988077882574..68e2ca4c4af63 100644 --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -307,45 +307,47 @@ int fscrypt_decrypt_page(const struct inode *inode, struct page *page, EXPORT_SYMBOL(fscrypt_decrypt_page); /* - * Validate dentries for encrypted directories to make sure we aren't - * potentially caching stale data after a key has been added or - * removed. + * Validate dentries in encrypted directories to make sure we aren't potentially + * caching stale dentries after a key has been added. */ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags) { struct dentry *dir; - int dir_has_key, cached_with_key; + int err; + int valid; + + /* + * Plaintext names are always valid, since fscrypt doesn't support + * reverting to ciphertext names without evicting the directory's inode + * -- which implies eviction of the dentries in the directory. + */ + if (!(dentry->d_flags & DCACHE_ENCRYPTED_NAME)) + return 1; + + /* + * Ciphertext name; valid if the directory's key is still unavailable. + * + * Although fscrypt forbids rename() on ciphertext names, we still must + * use dget_parent() here rather than use ->d_parent directly. That's + * because a corrupted fs image may contain directory hard links, which + * the VFS handles by moving the directory's dentry tree in the dcache + * each time ->lookup() finds the directory and it already has a dentry + * elsewhere. Thus ->d_parent can be changing, and we must safely grab + * a reference to some ->d_parent to prevent it from being freed. + */ if (flags & LOOKUP_RCU) return -ECHILD; dir = dget_parent(dentry); - if (!IS_ENCRYPTED(d_inode(dir))) { - dput(dir); - return 0; - } - - spin_lock(&dentry->d_lock); - cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY; - spin_unlock(&dentry->d_lock); - dir_has_key = fscrypt_has_encryption_key(d_inode(dir)); + err = fscrypt_get_encryption_info(d_inode(dir)); + valid = !fscrypt_has_encryption_key(d_inode(dir)); dput(dir); - /* - * If the dentry was cached without the key, and it is a - * negative dentry, it might be a valid name. We can't check - * if the key has since been made available due to locking - * reasons, so we fail the validation so ext4_lookup() can do - * this check. - * - * We also fail the validation if the dentry was created with - * the key present, but we no longer have the key, or vice versa. - */ - if ((!cached_with_key && d_is_negative(dentry)) || - (!cached_with_key && dir_has_key) || - (cached_with_key && !dir_has_key)) - return 0; - return 1; + if (err < 0) + return err; + + return valid; } const struct dentry_operations fscrypt_d_ops = { diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c index 56debb1fcf5eb..a9492f75bbe13 100644 --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -101,9 +101,9 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry) if (err) return err; - if (fscrypt_has_encryption_key(dir)) { + if (!fscrypt_has_encryption_key(dir)) { spin_lock(&dentry->d_lock); - dentry->d_flags |= DCACHE_ENCRYPTED_WITH_KEY; + dentry->d_flags |= DCACHE_ENCRYPTED_NAME; spin_unlock(&dentry->d_lock); } diff --git a/include/linux/dcache.h b/include/linux/dcache.h index 60996e64c5798..9b3b75d3bd211 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -212,7 +212,7 @@ struct dentry_operations { #define DCACHE_MAY_FREE 0x00800000 #define DCACHE_FALLTHRU 0x01000000 /* Fall through to lower layer */ -#define DCACHE_ENCRYPTED_WITH_KEY 0x02000000 /* dir is encrypted with a valid key */ +#define DCACHE_ENCRYPTED_NAME 0x02000000 /* Encrypted name (dir key was unavailable) */ #define DCACHE_OP_REAL 0x04000000 #define DCACHE_PAR_LOOKUP 0x10000000 /* being looked up (with parent locked shared) */ diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index ec8ab71085994..09e368a515d10 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -545,10 +545,8 @@ static inline int fscrypt_prepare_rename(struct inode *old_dir, * filenames are presented in encrypted form. Therefore, we'll try to set up * the directory's encryption key, but even without it the lookup can continue. * - * To allow invalidating stale dentries if the directory's encryption key is - * added later, we also install a custom ->d_revalidate() method and use the - * DCACHE_ENCRYPTED_WITH_KEY flag to indicate whether a given dentry is a - * plaintext name (flag set) or a ciphertext name (flag cleared). + * This also installs a custom ->d_revalidate() method which will invalidate the + * dentry if it was created without the key and the key is later added. * * Return: 0 on success, -errno if a problem occurred while setting up the * encryption key From 968dd6d0c6d6b6a989c6ddb9e2584a031b83e7b5 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 20 Mar 2019 11:39:10 -0700 Subject: [PATCH 05/10] fscrypt: fix race allowing rename() and link() of ciphertext dentries Close some race conditions where fscrypt allowed rename() and link() on ciphertext dentries that had been looked up just prior to the key being concurrently added. It's better to return -ENOKEY in this case. This avoids doing the nonsensical thing of encrypting the names a second time when searching for the actual on-disk dir entries. It also guarantees that DCACHE_ENCRYPTED_NAME dentries are never rename()d, so the dcache won't have support all possible combinations of moving DCACHE_ENCRYPTED_NAME around during __d_move(). Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/hooks.c | 12 +++++++++++- include/linux/fscrypt.h | 9 +++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c index a9492f75bbe13..2e7498a821a48 100644 --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -49,7 +49,8 @@ int fscrypt_file_open(struct inode *inode, struct file *filp) } EXPORT_SYMBOL_GPL(fscrypt_file_open); -int __fscrypt_prepare_link(struct inode *inode, struct inode *dir) +int __fscrypt_prepare_link(struct inode *inode, struct inode *dir, + struct dentry *dentry) { int err; @@ -57,6 +58,10 @@ int __fscrypt_prepare_link(struct inode *inode, struct inode *dir) if (err) return err; + /* ... in case we looked up ciphertext name before key was added */ + if (dentry->d_flags & DCACHE_ENCRYPTED_NAME) + return -ENOKEY; + if (!fscrypt_has_permitted_context(dir, inode)) return -EXDEV; @@ -78,6 +83,11 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry, if (err) return err; + /* ... in case we looked up ciphertext name(s) before key was added */ + if ((old_dentry->d_flags | new_dentry->d_flags) & + DCACHE_ENCRYPTED_NAME) + return -ENOKEY; + if (old_dir != new_dir) { if (IS_ENCRYPTED(new_dir) && !fscrypt_has_permitted_context(new_dir, diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index 09e368a515d10..855f743c226e6 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -215,7 +215,8 @@ extern int fscrypt_zeroout_range(const struct inode *, pgoff_t, sector_t, /* hooks.c */ extern int fscrypt_file_open(struct inode *inode, struct file *filp); -extern int __fscrypt_prepare_link(struct inode *inode, struct inode *dir); +extern int __fscrypt_prepare_link(struct inode *inode, struct inode *dir, + struct dentry *dentry); extern int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry, struct inode *new_dir, @@ -401,8 +402,8 @@ static inline int fscrypt_file_open(struct inode *inode, struct file *filp) return 0; } -static inline int __fscrypt_prepare_link(struct inode *inode, - struct inode *dir) +static inline int __fscrypt_prepare_link(struct inode *inode, struct inode *dir, + struct dentry *dentry) { return -EOPNOTSUPP; } @@ -497,7 +498,7 @@ static inline int fscrypt_prepare_link(struct dentry *old_dentry, struct dentry *dentry) { if (IS_ENCRYPTED(dir)) - return __fscrypt_prepare_link(d_inode(old_dentry), dir); + return __fscrypt_prepare_link(d_inode(old_dentry), dir, dentry); return 0; } From 0bf3d5c1604ecbbd4e49e9f5b3c79152b87adb0d Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 20 Mar 2019 11:39:11 -0700 Subject: [PATCH 06/10] fs, fscrypt: clear DCACHE_ENCRYPTED_NAME when unaliasing directory Make __d_move() clear DCACHE_ENCRYPTED_NAME on the source dentry. This is needed for when d_splice_alias() moves a directory's encrypted alias to its decrypted alias as a result of the encryption key being added. Otherwise, the decrypted alias will incorrectly be invalidated on the next lookup, causing problems such as unmounting a mount the user just mount()ed there. Note that we don't have to support arbitrary moves of this flag because fscrypt doesn't allow dentries with DCACHE_ENCRYPTED_NAME to be the source or target of a rename(). Fixes: 28b4c263961c ("ext4 crypto: revalidate dentry after adding or removing the key") Reported-by: Sarthak Kukreti Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/dcache.c | 2 ++ include/linux/fscrypt.h | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/fs/dcache.c b/fs/dcache.c index aac41adf47433..647e6ed426e20 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -2795,6 +2796,7 @@ static void __d_move(struct dentry *dentry, struct dentry *target, list_move(&dentry->d_child, &dentry->d_parent->d_subdirs); __d_rehash(dentry); fsnotify_update_flags(dentry); + fscrypt_handle_d_move(dentry); write_seqcount_end(&target->d_seq); write_seqcount_end(&dentry->d_seq); diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index 855f743c226e6..76c518f1e4c7a 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -89,6 +89,18 @@ static inline bool fscrypt_dummy_context_enabled(struct inode *inode) inode->i_sb->s_cop->dummy_context(inode); } +/* + * When d_splice_alias() moves a directory's encrypted alias to its decrypted + * alias as a result of the encryption key being added, DCACHE_ENCRYPTED_NAME + * must be cleared. Note that we don't have to support arbitrary moves of this + * flag because fscrypt doesn't allow encrypted aliases to be the source or + * target of a rename(). + */ +static inline void fscrypt_handle_d_move(struct dentry *dentry) +{ + dentry->d_flags &= ~DCACHE_ENCRYPTED_NAME; +} + /* crypto.c */ extern void fscrypt_enqueue_decrypt_work(struct work_struct *); extern struct fscrypt_ctx *fscrypt_get_ctx(gfp_t); @@ -244,6 +256,10 @@ static inline bool fscrypt_dummy_context_enabled(struct inode *inode) return false; } +static inline void fscrypt_handle_d_move(struct dentry *dentry) +{ +} + /* crypto.c */ static inline void fscrypt_enqueue_decrypt_work(struct work_struct *work) { From d456a33f041af4b54f3ce495a86d00c246165032 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 20 Mar 2019 11:39:12 -0700 Subject: [PATCH 07/10] fscrypt: only set dentry_operations on ciphertext dentries Plaintext dentries are always valid, so only set fscrypt_d_ops on ciphertext dentries. Besides marginally improved performance, this allows overlayfs to use an fscrypt-encrypted upperdir, provided that all the following are true: (1) The fscrypt encryption key is placed in the keyring before mounting overlayfs, and remains while the overlayfs is mounted. (2) The overlayfs workdir uses the same encryption policy. (3) No dentries for the ciphertext names of subdirectories have been created in the upperdir or workdir yet. (Since otherwise d_splice_alias() will reuse the old dentry with ->d_op set.) One potential use case is using an ephemeral encryption key to encrypt all files created or changed by a container, so that they can be securely erased ("crypto-shredded") after the container stops. Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/hooks.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c index 2e7498a821a48..9d8910e86ee5d 100644 --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -115,9 +115,8 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry) spin_lock(&dentry->d_lock); dentry->d_flags |= DCACHE_ENCRYPTED_NAME; spin_unlock(&dentry->d_lock); + d_set_d_op(dentry, &fscrypt_d_ops); } - - d_set_d_op(dentry, &fscrypt_d_ops); return 0; } EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup); From b01531db6cec2aa330dbc91bfbfaaef4a0d387a4 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 20 Mar 2019 11:39:13 -0700 Subject: [PATCH 08/10] fscrypt: fix race where ->lookup() marks plaintext dentry as ciphertext ->lookup() in an encrypted directory begins as follows: 1. fscrypt_prepare_lookup(): a. Try to load the directory's encryption key. b. If the key is unavailable, mark the dentry as a ciphertext name via d_flags. 2. fscrypt_setup_filename(): a. Try to load the directory's encryption key. b. If the key is available, encrypt the name (treated as a plaintext name) to get the on-disk name. Otherwise decode the name (treated as a ciphertext name) to get the on-disk name. But if the key is concurrently added, it may be found at (2a) but not at (1a). In this case, the dentry will be wrongly marked as a ciphertext name even though it was actually treated as plaintext. This will cause the dentry to be wrongly invalidated on the next lookup, potentially causing problems. For example, if the racy ->lookup() was part of sys_mount(), then the new mount will be detached when anything tries to access it. This is despite the mountpoint having a plaintext path, which should remain valid now that the key was added. Of course, this is only possible if there's a userspace race. Still, the additional kernel-side race is confusing and unexpected. Close the kernel-side race by changing fscrypt_prepare_lookup() to also set the on-disk filename (step 2b), consistent with the d_flags update. Fixes: 28b4c263961c ("ext4 crypto: revalidate dentry after adding or removing the key") Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/fname.c | 1 + fs/crypto/hooks.c | 11 +++--- fs/ext4/ext4.h | 62 +++++++++++++++++++++++++-------- fs/ext4/namei.c | 76 ++++++++++++++++++++++++++++------------- fs/f2fs/namei.c | 17 +++++---- fs/ubifs/dir.c | 8 ++--- include/linux/fscrypt.h | 30 ++++++++++------ 7 files changed, 139 insertions(+), 66 deletions(-) diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c index 050384c79f40e..eccea3d8f9234 100644 --- a/fs/crypto/fname.c +++ b/fs/crypto/fname.c @@ -356,6 +356,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname, } if (!lookup) return -ENOKEY; + fname->is_ciphertext_name = true; /* * We don't have the key and we are doing a lookup; decode the diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c index 9d8910e86ee5d..042d5b44f4ed9 100644 --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -104,20 +104,21 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry, } EXPORT_SYMBOL_GPL(__fscrypt_prepare_rename); -int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry) +int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry, + struct fscrypt_name *fname) { - int err = fscrypt_get_encryption_info(dir); + int err = fscrypt_setup_filename(dir, &dentry->d_name, 1, fname); - if (err) + if (err && err != -ENOENT) return err; - if (!fscrypt_has_encryption_key(dir)) { + if (fname->is_ciphertext_name) { spin_lock(&dentry->d_lock); dentry->d_flags |= DCACHE_ENCRYPTED_NAME; spin_unlock(&dentry->d_lock); d_set_d_op(dentry, &fscrypt_d_ops); } - return 0; + return err; } EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup); diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 82ffdacdc7fac..e64a4ee96d30a 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -2287,23 +2287,47 @@ extern unsigned ext4_free_clusters_after_init(struct super_block *sb, ext4_fsblk_t ext4_inode_to_goal_block(struct inode *); #ifdef CONFIG_FS_ENCRYPTION +static inline void ext4_fname_from_fscrypt_name(struct ext4_filename *dst, + const struct fscrypt_name *src) +{ + memset(dst, 0, sizeof(*dst)); + + dst->usr_fname = src->usr_fname; + dst->disk_name = src->disk_name; + dst->hinfo.hash = src->hash; + dst->hinfo.minor_hash = src->minor_hash; + dst->crypto_buf = src->crypto_buf; +} + static inline int ext4_fname_setup_filename(struct inode *dir, - const struct qstr *iname, - int lookup, struct ext4_filename *fname) + const struct qstr *iname, + int lookup, + struct ext4_filename *fname) { struct fscrypt_name name; int err; - memset(fname, 0, sizeof(struct ext4_filename)); - err = fscrypt_setup_filename(dir, iname, lookup, &name); + if (err) + return err; - fname->usr_fname = name.usr_fname; - fname->disk_name = name.disk_name; - fname->hinfo.hash = name.hash; - fname->hinfo.minor_hash = name.minor_hash; - fname->crypto_buf = name.crypto_buf; - return err; + ext4_fname_from_fscrypt_name(fname, &name); + return 0; +} + +static inline int ext4_fname_prepare_lookup(struct inode *dir, + struct dentry *dentry, + struct ext4_filename *fname) +{ + struct fscrypt_name name; + int err; + + err = fscrypt_prepare_lookup(dir, dentry, &name); + if (err) + return err; + + ext4_fname_from_fscrypt_name(fname, &name); + return 0; } static inline void ext4_fname_free_filename(struct ext4_filename *fname) @@ -2317,19 +2341,27 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname) fname->usr_fname = NULL; fname->disk_name.name = NULL; } -#else +#else /* !CONFIG_FS_ENCRYPTION */ static inline int ext4_fname_setup_filename(struct inode *dir, - const struct qstr *iname, - int lookup, struct ext4_filename *fname) + const struct qstr *iname, + int lookup, + struct ext4_filename *fname) { fname->usr_fname = iname; fname->disk_name.name = (unsigned char *) iname->name; fname->disk_name.len = iname->len; return 0; } -static inline void ext4_fname_free_filename(struct ext4_filename *fname) { } -#endif +static inline int ext4_fname_prepare_lookup(struct inode *dir, + struct dentry *dentry, + struct ext4_filename *fname) +{ + return ext4_fname_setup_filename(dir, &dentry->d_name, 1, fname); +} + +static inline void ext4_fname_free_filename(struct ext4_filename *fname) { } +#endif /* !CONFIG_FS_ENCRYPTION */ /* dir.c */ extern int __ext4_check_dir_entry(const char *, unsigned int, struct inode *, diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 980166a8122a6..3ba6f30db8d92 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -1327,7 +1327,7 @@ static int is_dx_internal_node(struct inode *dir, ext4_lblk_t block, } /* - * ext4_find_entry() + * __ext4_find_entry() * * finds an entry in the specified directory with the wanted name. It * returns the cache buffer in which the entry was found, and the entry @@ -1337,39 +1337,32 @@ static int is_dx_internal_node(struct inode *dir, ext4_lblk_t block, * The returned buffer_head has ->b_count elevated. The caller is expected * to brelse() it when appropriate. */ -static struct buffer_head * ext4_find_entry (struct inode *dir, - const struct qstr *d_name, - struct ext4_dir_entry_2 **res_dir, - int *inlined) +static struct buffer_head *__ext4_find_entry(struct inode *dir, + struct ext4_filename *fname, + struct ext4_dir_entry_2 **res_dir, + int *inlined) { struct super_block *sb; struct buffer_head *bh_use[NAMEI_RA_SIZE]; struct buffer_head *bh, *ret = NULL; ext4_lblk_t start, block; - const u8 *name = d_name->name; + const u8 *name = fname->usr_fname->name; size_t ra_max = 0; /* Number of bh's in the readahead buffer, bh_use[] */ size_t ra_ptr = 0; /* Current index into readahead buffer */ ext4_lblk_t nblocks; int i, namelen, retval; - struct ext4_filename fname; *res_dir = NULL; sb = dir->i_sb; - namelen = d_name->len; + namelen = fname->usr_fname->len; if (namelen > EXT4_NAME_LEN) return NULL; - retval = ext4_fname_setup_filename(dir, d_name, 1, &fname); - if (retval == -ENOENT) - return NULL; - if (retval) - return ERR_PTR(retval); - if (ext4_has_inline_data(dir)) { int has_inline_data = 1; - ret = ext4_find_inline_entry(dir, &fname, res_dir, + ret = ext4_find_inline_entry(dir, fname, res_dir, &has_inline_data); if (has_inline_data) { if (inlined) @@ -1389,7 +1382,7 @@ static struct buffer_head * ext4_find_entry (struct inode *dir, goto restart; } if (is_dx(dir)) { - ret = ext4_dx_find_entry(dir, &fname, res_dir); + ret = ext4_dx_find_entry(dir, fname, res_dir); /* * On success, or if the error was file not found, * return. Otherwise, fall back to doing a search the @@ -1453,7 +1446,7 @@ static struct buffer_head * ext4_find_entry (struct inode *dir, goto cleanup_and_exit; } set_buffer_verified(bh); - i = search_dirblock(bh, dir, &fname, + i = search_dirblock(bh, dir, fname, block << EXT4_BLOCK_SIZE_BITS(sb), res_dir); if (i == 1) { EXT4_I(dir)->i_dir_start_lookup = block; @@ -1484,10 +1477,50 @@ static struct buffer_head * ext4_find_entry (struct inode *dir, /* Clean up the read-ahead blocks */ for (; ra_ptr < ra_max; ra_ptr++) brelse(bh_use[ra_ptr]); - ext4_fname_free_filename(&fname); return ret; } +static struct buffer_head *ext4_find_entry(struct inode *dir, + const struct qstr *d_name, + struct ext4_dir_entry_2 **res_dir, + int *inlined) +{ + int err; + struct ext4_filename fname; + struct buffer_head *bh; + + err = ext4_fname_setup_filename(dir, d_name, 1, &fname); + if (err == -ENOENT) + return NULL; + if (err) + return ERR_PTR(err); + + bh = __ext4_find_entry(dir, &fname, res_dir, inlined); + + ext4_fname_free_filename(&fname); + return bh; +} + +static struct buffer_head *ext4_lookup_entry(struct inode *dir, + struct dentry *dentry, + struct ext4_dir_entry_2 **res_dir) +{ + int err; + struct ext4_filename fname; + struct buffer_head *bh; + + err = ext4_fname_prepare_lookup(dir, dentry, &fname); + if (err == -ENOENT) + return NULL; + if (err) + return ERR_PTR(err); + + bh = __ext4_find_entry(dir, &fname, res_dir, NULL); + + ext4_fname_free_filename(&fname); + return bh; +} + static struct buffer_head * ext4_dx_find_entry(struct inode *dir, struct ext4_filename *fname, struct ext4_dir_entry_2 **res_dir) @@ -1546,16 +1579,11 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi struct inode *inode; struct ext4_dir_entry_2 *de; struct buffer_head *bh; - int err; - - err = fscrypt_prepare_lookup(dir, dentry, flags); - if (err) - return ERR_PTR(err); if (dentry->d_name.len > EXT4_NAME_LEN) return ERR_PTR(-ENAMETOOLONG); - bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL); + bh = ext4_lookup_entry(dir, dentry, &de); if (IS_ERR(bh)) return ERR_CAST(bh); inode = NULL; diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index f5e34e4670031..c3e8a901d47ac 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -436,19 +436,23 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry, nid_t ino = -1; int err = 0; unsigned int root_ino = F2FS_ROOT_INO(F2FS_I_SB(dir)); + struct fscrypt_name fname; trace_f2fs_lookup_start(dir, dentry, flags); - err = fscrypt_prepare_lookup(dir, dentry, flags); - if (err) - goto out; - if (dentry->d_name.len > F2FS_NAME_LEN) { err = -ENAMETOOLONG; goto out; } - de = f2fs_find_entry(dir, &dentry->d_name, &page); + err = fscrypt_prepare_lookup(dir, dentry, &fname); + if (err == -ENOENT) + goto out_splice; + if (err) + goto out; + de = __f2fs_find_entry(dir, &fname, &page); + fscrypt_free_filename(&fname); + if (!de) { if (IS_ERR(page)) { err = PTR_ERR(page); @@ -488,8 +492,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry, } out_splice: new = d_splice_alias(inode, dentry); - if (IS_ERR(new)) - err = PTR_ERR(new); + err = PTR_ERR_OR_ZERO(new); trace_f2fs_lookup_end(dir, dentry, ino, err); return new; out_iput: diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 5767b373a8ffb..b73de6d04fa3e 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -220,11 +220,9 @@ static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry, dbg_gen("'%pd' in dir ino %lu", dentry, dir->i_ino); - err = fscrypt_prepare_lookup(dir, dentry, flags); - if (err) - return ERR_PTR(err); - - err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm); + err = fscrypt_prepare_lookup(dir, dentry, &nm); + if (err == -ENOENT) + return d_splice_alias(NULL, dentry); if (err) return ERR_PTR(err); diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index 76c518f1e4c7a..abe7081b6b22d 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -33,6 +33,7 @@ struct fscrypt_name { u32 hash; u32 minor_hash; struct fscrypt_str crypto_buf; + bool is_ciphertext_name; }; #define FSTR_INIT(n, l) { .name = n, .len = l } @@ -234,7 +235,8 @@ extern int __fscrypt_prepare_rename(struct inode *old_dir, struct inode *new_dir, struct dentry *new_dentry, unsigned int flags); -extern int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry); +extern int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry, + struct fscrypt_name *fname); extern int __fscrypt_prepare_symlink(struct inode *dir, unsigned int len, unsigned int max_len, struct fscrypt_str *disk_link); @@ -347,7 +349,7 @@ static inline int fscrypt_setup_filename(struct inode *dir, if (IS_ENCRYPTED(dir)) return -EOPNOTSUPP; - memset(fname, 0, sizeof(struct fscrypt_name)); + memset(fname, 0, sizeof(*fname)); fname->usr_fname = iname; fname->disk_name.name = (unsigned char *)iname->name; fname->disk_name.len = iname->len; @@ -434,7 +436,8 @@ static inline int __fscrypt_prepare_rename(struct inode *old_dir, } static inline int __fscrypt_prepare_lookup(struct inode *dir, - struct dentry *dentry) + struct dentry *dentry, + struct fscrypt_name *fname) { return -EOPNOTSUPP; } @@ -555,25 +558,32 @@ static inline int fscrypt_prepare_rename(struct inode *old_dir, * fscrypt_prepare_lookup - prepare to lookup a name in a possibly-encrypted directory * @dir: directory being searched * @dentry: filename being looked up - * @flags: lookup flags + * @fname: (output) the name to use to search the on-disk directory * - * Prepare for ->lookup() in a directory which may be encrypted. Lookups can be - * done with or without the directory's encryption key; without the key, + * Prepare for ->lookup() in a directory which may be encrypted by determining + * the name that will actually be used to search the directory on-disk. Lookups + * can be done with or without the directory's encryption key; without the key, * filenames are presented in encrypted form. Therefore, we'll try to set up * the directory's encryption key, but even without it the lookup can continue. * * This also installs a custom ->d_revalidate() method which will invalidate the * dentry if it was created without the key and the key is later added. * - * Return: 0 on success, -errno if a problem occurred while setting up the - * encryption key + * Return: 0 on success; -ENOENT if key is unavailable but the filename isn't a + * correctly formed encoded ciphertext name, so a negative dentry should be + * created; or another -errno code. */ static inline int fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry, - unsigned int flags) + struct fscrypt_name *fname) { if (IS_ENCRYPTED(dir)) - return __fscrypt_prepare_lookup(dir, dentry); + return __fscrypt_prepare_lookup(dir, dentry, fname); + + memset(fname, 0, sizeof(*fname)); + fname->usr_fname = &dentry->d_name; + fname->disk_name.name = (unsigned char *)dentry->d_name.name; + fname->disk_name.len = dentry->d_name.len; return 0; } From 4c4f7c19b3c721aed418bc97907b411608c5c6a0 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 10 Apr 2019 13:21:14 -0700 Subject: [PATCH 09/10] vfs: use READ_ONCE() to access ->i_link Use 'READ_ONCE(inode->i_link)' to explicitly support filesystems caching the symlink target in ->i_link later if it was unavailable at iget() time, or wasn't easily available. I'll be doing this in fscrypt, to improve the performance of encrypted symlinks on ext4, f2fs, and ubifs. ->i_link will start NULL and may later be set to a non-NULL value by a smp_store_release() or cmpxchg_release(). READ_ONCE() is needed on the read side. smp_load_acquire() is unnecessary because only a data dependency barrier is required. (Thanks to Al for pointing this out.) Acked-by: Al Viro Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/namei.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index dede0147b3f6e..2855de004c1a9 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1066,7 +1066,7 @@ const char *get_link(struct nameidata *nd) return ERR_PTR(error); nd->last_type = LAST_BIND; - res = inode->i_link; + res = READ_ONCE(inode->i_link); if (!res) { const char * (*get)(struct dentry *, struct inode *, struct delayed_call *); @@ -4729,7 +4729,7 @@ int vfs_readlink(struct dentry *dentry, char __user *buffer, int buflen) spin_unlock(&inode->i_lock); } - link = inode->i_link; + link = READ_ONCE(inode->i_link); if (!link) { link = inode->i_op->get_link(dentry, inode, &done); if (IS_ERR(link)) From 2c58d548f5706d085c4b009f6abb945220460632 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 10 Apr 2019 13:21:15 -0700 Subject: [PATCH 10/10] fscrypt: cache decrypted symlink target in ->i_link Path lookups that traverse encrypted symlink(s) are very slow because each encrypted symlink needs to be decrypted each time it's followed. This also involves dropping out of rcu-walk mode. Make encrypted symlinks faster by caching the decrypted symlink target in ->i_link. The first call to fscrypt_get_symlink() sets it. Then, the existing VFS path lookup code uses the non-NULL ->i_link to take the fast path where ->get_link() isn't called, and lookups in rcu-walk mode remain in rcu-walk mode. Also set ->i_link immediately when a new encrypted symlink is created. To safely free the symlink target after an RCU grace period has elapsed, introduce a new function fscrypt_free_inode(), and make the relevant filesystems call it just before actually freeing the inode. Cc: Al Viro Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o --- fs/crypto/hooks.c | 40 +++++++++++++++++++++++++++++++++------- fs/crypto/keyinfo.c | 21 +++++++++++++++++++++ fs/ext4/super.c | 3 +++ fs/f2fs/super.c | 3 +++ fs/ubifs/super.c | 3 +++ include/linux/fscrypt.h | 5 +++++ 6 files changed, 68 insertions(+), 7 deletions(-) diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c index 042d5b44f4ed9..2dc22549d724d 100644 --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -189,11 +189,9 @@ int __fscrypt_encrypt_symlink(struct inode *inode, const char *target, sd->len = cpu_to_le16(ciphertext_len); err = fname_encrypt(inode, &iname, sd->encrypted_path, ciphertext_len); - if (err) { - if (!disk_link->name) - kfree(sd); - return err; - } + if (err) + goto err_free_sd; + /* * Null-terminating the ciphertext doesn't make sense, but we still * count the null terminator in the length, so we might as well @@ -201,9 +199,20 @@ int __fscrypt_encrypt_symlink(struct inode *inode, const char *target, */ sd->encrypted_path[ciphertext_len] = '\0'; + /* Cache the plaintext symlink target for later use by get_link() */ + err = -ENOMEM; + inode->i_link = kmemdup(target, len + 1, GFP_NOFS); + if (!inode->i_link) + goto err_free_sd; + if (!disk_link->name) disk_link->name = (unsigned char *)sd; return 0; + +err_free_sd: + if (!disk_link->name) + kfree(sd); + return err; } EXPORT_SYMBOL_GPL(__fscrypt_encrypt_symlink); @@ -212,7 +221,7 @@ EXPORT_SYMBOL_GPL(__fscrypt_encrypt_symlink); * @inode: the symlink inode * @caddr: the on-disk contents of the symlink * @max_size: size of @caddr buffer - * @done: if successful, will be set up to free the returned target + * @done: if successful, will be set up to free the returned target if needed * * If the symlink's encryption key is available, we decrypt its target. * Otherwise, we encode its target for presentation. @@ -227,12 +236,18 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr, { const struct fscrypt_symlink_data *sd; struct fscrypt_str cstr, pstr; + bool has_key; int err; /* This is for encrypted symlinks only */ if (WARN_ON(!IS_ENCRYPTED(inode))) return ERR_PTR(-EINVAL); + /* If the decrypted target is already cached, just return it. */ + pstr.name = READ_ONCE(inode->i_link); + if (pstr.name) + return pstr.name; + /* * Try to set up the symlink's encryption key, but we can continue * regardless of whether the key is available or not. @@ -240,6 +255,7 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr, err = fscrypt_get_encryption_info(inode); if (err) return ERR_PTR(err); + has_key = fscrypt_has_encryption_key(inode); /* * For historical reasons, encrypted symlink targets are prefixed with @@ -271,7 +287,17 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr, goto err_kfree; pstr.name[pstr.len] = '\0'; - set_delayed_call(done, kfree_link, pstr.name); + + /* + * Cache decrypted symlink targets in i_link for later use. Don't cache + * symlink targets encoded without the key, since those become outdated + * once the key is added. This pairs with the READ_ONCE() above and in + * the VFS path lookup code. + */ + if (!has_key || + cmpxchg_release(&inode->i_link, NULL, pstr.name) != NULL) + set_delayed_call(done, kfree_link, pstr.name); + return pstr.name; err_kfree: diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c index bf291c10c682f..82989098b2fc9 100644 --- a/fs/crypto/keyinfo.c +++ b/fs/crypto/keyinfo.c @@ -584,9 +584,30 @@ int fscrypt_get_encryption_info(struct inode *inode) } EXPORT_SYMBOL(fscrypt_get_encryption_info); +/** + * fscrypt_put_encryption_info - free most of an inode's fscrypt data + * + * Free the inode's fscrypt_info. Filesystems must call this when the inode is + * being evicted. An RCU grace period need not have elapsed yet. + */ void fscrypt_put_encryption_info(struct inode *inode) { put_crypt_info(inode->i_crypt_info); inode->i_crypt_info = NULL; } EXPORT_SYMBOL(fscrypt_put_encryption_info); + +/** + * fscrypt_free_inode - free an inode's fscrypt data requiring RCU delay + * + * Free the inode's cached decrypted symlink target, if any. Filesystems must + * call this after an RCU grace period, just before they free the inode. + */ +void fscrypt_free_inode(struct inode *inode) +{ + if (IS_ENCRYPTED(inode) && S_ISLNK(inode->i_mode)) { + kfree(inode->i_link); + inode->i_link = NULL; + } +} +EXPORT_SYMBOL(fscrypt_free_inode); diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 6ed4eb81e6743..5b92054bf8ea0 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1110,6 +1110,9 @@ static int ext4_drop_inode(struct inode *inode) static void ext4_i_callback(struct rcu_head *head) { struct inode *inode = container_of(head, struct inode, i_rcu); + + fscrypt_free_inode(inode); + kmem_cache_free(ext4_inode_cachep, EXT4_I(inode)); } diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index f2aaa2cc6b3e0..11b3a039a1881 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1003,6 +1003,9 @@ static void f2fs_dirty_inode(struct inode *inode, int flags) static void f2fs_i_callback(struct rcu_head *head) { struct inode *inode = container_of(head, struct inode, i_rcu); + + fscrypt_free_inode(inode); + kmem_cache_free(f2fs_inode_cachep, F2FS_I(inode)); } diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 12628184772c0..19fd210987457 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -276,7 +276,10 @@ static void ubifs_i_callback(struct rcu_head *head) { struct inode *inode = container_of(head, struct inode, i_rcu); struct ubifs_inode *ui = ubifs_inode(inode); + kfree(ui->data); + fscrypt_free_inode(inode); + kmem_cache_free(ubifs_inode_slab, ui); } diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index abe7081b6b22d..28c74e0a72310 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -128,6 +128,7 @@ extern int fscrypt_inherit_context(struct inode *, struct inode *, /* keyinfo.c */ extern int fscrypt_get_encryption_info(struct inode *); extern void fscrypt_put_encryption_info(struct inode *); +extern void fscrypt_free_inode(struct inode *); /* fname.c */ extern int fscrypt_setup_filename(struct inode *, const struct qstr *, @@ -341,6 +342,10 @@ static inline void fscrypt_put_encryption_info(struct inode *inode) return; } +static inline void fscrypt_free_inode(struct inode *inode) +{ +} + /* fname.c */ static inline int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,