Skip to content

Commit

Permalink
fscrypt: fix keyring memory leak on mount failure
Browse files Browse the repository at this point in the history
commit ccd30a4 upstream.

Commit d7e7b9a ("fscrypt: stop using keyrings subsystem for
fscrypt_master_key") moved the keyring destruction from __put_super() to
generic_shutdown_super() so that the filesystem's block device(s) are
still available.  Unfortunately, this causes a memory leak in the case
where a mount is attempted with the test_dummy_encryption mount option,
but the mount fails after the option has already been processed.

To fix this, attempt the keyring destruction in both places.

Reported-by: syzbot+104c2a89561289cec13e@syzkaller.appspotmail.com
Fixes: d7e7b9a ("fscrypt: stop using keyrings subsystem for fscrypt_master_key")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Link: https://lore.kernel.org/r/20221011213838.209879-1-ebiggers@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
ebiggers authored and gregkh committed Nov 10, 2022
1 parent e6f4fd8 commit cff805b
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 9 deletions.
17 changes: 11 additions & 6 deletions fs/crypto/keyring.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,19 @@ static int allocate_filesystem_keyring(struct super_block *sb)
}

/*
* This is called at unmount time to release all encryption keys that have been
* added to the filesystem, along with the keyring that contains them.
* Release all encryption keys that have been added to the filesystem, along
* with the keyring that contains them.
*
* Note that besides clearing and freeing memory, this might need to evict keys
* from the keyslots of an inline crypto engine. Therefore, this must be called
* while the filesystem's underlying block device(s) are still available.
* This is called at unmount time. The filesystem's underlying block device(s)
* are still available at this time; this is important because after user file
* accesses have been allowed, this function may need to evict keys from the
* keyslots of an inline crypto engine, which requires the block device(s).
*
* This is also called when the super_block is being freed. This is needed to
* avoid a memory leak if mounting fails after the "test_dummy_encryption"
* option was processed, as in that case the unmount-time call isn't made.
*/
void fscrypt_sb_delete(struct super_block *sb)
void fscrypt_destroy_keyring(struct super_block *sb)
{
struct fscrypt_keyring *keyring = sb->s_master_keys;
size_t i;
Expand Down
3 changes: 2 additions & 1 deletion fs/super.c
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ static void __put_super(struct super_block *s)
WARN_ON(s->s_inode_lru.node);
WARN_ON(!list_empty(&s->s_mounts));
security_sb_free(s);
fscrypt_destroy_keyring(s);
put_user_ns(s->s_user_ns);
kfree(s->s_subtype);
call_rcu(&s->rcu, destroy_super_rcu);
Expand Down Expand Up @@ -453,7 +454,7 @@ void generic_shutdown_super(struct super_block *sb)
evict_inodes(sb);
/* only nonzero refcount inodes can have marks */
fsnotify_sb_delete(sb);
fscrypt_sb_delete(sb);
fscrypt_destroy_keyring(sb);
security_sb_delete(sb);

if (sb->s_dio_done_wq) {
Expand Down
4 changes: 2 additions & 2 deletions include/linux/fscrypt.h
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ fscrypt_free_dummy_policy(struct fscrypt_dummy_policy *dummy_policy)
}

/* keyring.c */
void fscrypt_sb_delete(struct super_block *sb);
void fscrypt_destroy_keyring(struct super_block *sb);
int fscrypt_ioctl_add_key(struct file *filp, void __user *arg);
int fscrypt_ioctl_remove_key(struct file *filp, void __user *arg);
int fscrypt_ioctl_remove_key_all_users(struct file *filp, void __user *arg);
Expand Down Expand Up @@ -482,7 +482,7 @@ fscrypt_free_dummy_policy(struct fscrypt_dummy_policy *dummy_policy)
}

/* keyring.c */
static inline void fscrypt_sb_delete(struct super_block *sb)
static inline void fscrypt_destroy_keyring(struct super_block *sb)
{
}

Expand Down

0 comments on commit cff805b

Please sign in to comment.