Commit b01531db authored by Eric Biggers's avatar Eric Biggers Committed by Theodore Ts'o
Browse files

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: 28b4c263 ("ext4 crypto: revalidate dentry after adding or removing the key")
Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
parent d456a33f

fs/crypto/fname.c

+1 −0
Original line number Original line Diff line number Diff line
@@ -356,6 +356,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname, 
	}

	}

	if (!lookup)

	if (!lookup)

		return -ENOKEY;

		return -ENOKEY;

	fname->is_ciphertext_name = true;





	/*

	/*

	 * We don't have the key and we are doing a lookup; decode the

	 * We don't have the key and we are doing a lookup; decode the

fs/crypto/hooks.c

+6 −5
Original line number Original line Diff line number Diff line
@@ -104,20 +104,21 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry, 
}

}

EXPORT_SYMBOL_GPL(__fscrypt_prepare_rename);

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;

		return err;





	if (!fscrypt_has_encryption_key(dir)) {

	if (fname->is_ciphertext_name) {

		spin_lock(&dentry->d_lock);

		spin_lock(&dentry->d_lock);

		dentry->d_flags |= DCACHE_ENCRYPTED_NAME;

		dentry->d_flags |= DCACHE_ENCRYPTED_NAME;

		spin_unlock(&dentry->d_lock);

		spin_unlock(&dentry->d_lock);

		d_set_d_op(dentry, &fscrypt_d_ops);

		d_set_d_op(dentry, &fscrypt_d_ops);

	}

	}

	return 0;

	return err;

}

}

EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup);

EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup);

fs/ext4/ext4.h

+47 −15
Original line number Original line Diff line number Diff line
@@ -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 *);

ext4_fsblk_t ext4_inode_to_goal_block(struct inode *);





#ifdef CONFIG_FS_ENCRYPTION

#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,

static inline int ext4_fname_setup_filename(struct inode *dir,

					    const struct qstr *iname,

					    const struct qstr *iname,

			int lookup, struct ext4_filename *fname)

					    int lookup,

					    struct ext4_filename *fname)

{

{

	struct fscrypt_name name;

	struct fscrypt_name name;

	int err;

	int err;





	memset(fname, 0, sizeof(struct ext4_filename));



	err = fscrypt_setup_filename(dir, iname, lookup, &name);

	err = fscrypt_setup_filename(dir, iname, lookup, &name);

	if (err)

		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;





	fname->usr_fname = name.usr_fname;

	err = fscrypt_prepare_lookup(dir, dentry, &name);

	fname->disk_name = name.disk_name;

	if (err)

	fname->hinfo.hash = name.hash;

	fname->hinfo.minor_hash = name.minor_hash;

	fname->crypto_buf = name.crypto_buf;

		return err;

		return err;



	ext4_fname_from_fscrypt_name(fname, &name);

	return 0;

}

}





static inline void ext4_fname_free_filename(struct ext4_filename *fname)

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->usr_fname = NULL;

	fname->disk_name.name = NULL;

	fname->disk_name.name = NULL;

}

}

#else

#else /* !CONFIG_FS_ENCRYPTION */

static inline int ext4_fname_setup_filename(struct inode *dir,

static inline int ext4_fname_setup_filename(struct inode *dir,

					    const struct qstr *iname,

					    const struct qstr *iname,

		int lookup, struct ext4_filename *fname)

					    int lookup,

					    struct ext4_filename *fname)

{

{

	fname->usr_fname = iname;

	fname->usr_fname = iname;

	fname->disk_name.name = (unsigned char *) iname->name;

	fname->disk_name.name = (unsigned char *) iname->name;

	fname->disk_name.len = iname->len;

	fname->disk_name.len = iname->len;

	return 0;

	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 */

/* dir.c */

extern int __ext4_check_dir_entry(const char *, unsigned int, struct inode *,

extern int __ext4_check_dir_entry(const char *, unsigned int, struct inode *,

fs/ext4/namei.c

+52 −24
Original line number Original line Diff line number Diff line
@@ -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

 * 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

 * returns the cache buffer in which the entry was found, and the entry
@@ -1337,8 +1337,8 @@ 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

 * The returned buffer_head has ->b_count elevated.  The caller is expected

 * to brelse() it when appropriate.

 * to brelse() it when appropriate.

 */

 */

static struct buffer_head * ext4_find_entry (struct inode *dir,

static struct buffer_head *__ext4_find_entry(struct inode *dir,

					const struct qstr *d_name,

					     struct ext4_filename *fname,

					     struct ext4_dir_entry_2 **res_dir,

					     struct ext4_dir_entry_2 **res_dir,

					     int *inlined)

					     int *inlined)

{

{
@@ -1346,30 +1346,23 @@ static struct buffer_head * ext4_find_entry (struct inode *dir, 
	struct buffer_head *bh_use[NAMEI_RA_SIZE];

	struct buffer_head *bh_use[NAMEI_RA_SIZE];

	struct buffer_head *bh, *ret = NULL;

	struct buffer_head *bh, *ret = NULL;

	ext4_lblk_t start, block;

	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

	size_t ra_max = 0;	/* Number of bh's in the readahead

				   buffer, bh_use[] */

				   buffer, bh_use[] */

	size_t ra_ptr = 0;	/* Current index into readahead

	size_t ra_ptr = 0;	/* Current index into readahead

				   buffer */

				   buffer */

	ext4_lblk_t  nblocks;

	ext4_lblk_t  nblocks;

	int i, namelen, retval;

	int i, namelen, retval;

	struct ext4_filename fname;





	*res_dir = NULL;

	*res_dir = NULL;

	sb = dir->i_sb;

	sb = dir->i_sb;

	namelen = d_name->len;

	namelen = fname->usr_fname->len;

	if (namelen > EXT4_NAME_LEN)

	if (namelen > EXT4_NAME_LEN)

		return NULL;

		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)) {

	if (ext4_has_inline_data(dir)) {

		int has_inline_data = 1;

		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);

					     &has_inline_data);

		if (has_inline_data) {

		if (has_inline_data) {

			if (inlined)

			if (inlined)
@@ -1389,7 +1382,7 @@ static struct buffer_head * ext4_find_entry (struct inode *dir, 
		goto restart;

		goto restart;

	}

	}

	if (is_dx(dir)) {

	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,

		 * On success, or if the error was file not found,

		 * return.  Otherwise, fall back to doing a search the

		 * 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;

			goto cleanup_and_exit;

		}

		}

		set_buffer_verified(bh);

		set_buffer_verified(bh);

		i = search_dirblock(bh, dir, &fname,

		i = search_dirblock(bh, dir, fname,

			    block << EXT4_BLOCK_SIZE_BITS(sb), res_dir);

			    block << EXT4_BLOCK_SIZE_BITS(sb), res_dir);

		if (i == 1) {

		if (i == 1) {

			EXT4_I(dir)->i_dir_start_lookup = block;

			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 */

	/* Clean up the read-ahead blocks */

	for (; ra_ptr < ra_max; ra_ptr++)

	for (; ra_ptr < ra_max; ra_ptr++)

		brelse(bh_use[ra_ptr]);

		brelse(bh_use[ra_ptr]);

	ext4_fname_free_filename(&fname);

	return ret;

	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,

static struct buffer_head * ext4_dx_find_entry(struct inode *dir,

			struct ext4_filename *fname,

			struct ext4_filename *fname,

			struct ext4_dir_entry_2 **res_dir)

			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 inode *inode;

	struct ext4_dir_entry_2 *de;

	struct ext4_dir_entry_2 *de;

	struct buffer_head *bh;

	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)

	if (dentry->d_name.len > EXT4_NAME_LEN)

		return ERR_PTR(-ENAMETOOLONG);

		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))

	if (IS_ERR(bh))

		return ERR_CAST(bh);

		return ERR_CAST(bh);

	inode = NULL;

	inode = NULL;

fs/f2fs/namei.c

+10 −7
Original line number Original line Diff line number Diff line
@@ -436,19 +436,23 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry, 
	nid_t ino = -1;

	nid_t ino = -1;

	int err = 0;

	int err = 0;

	unsigned int root_ino = F2FS_ROOT_INO(F2FS_I_SB(dir));

	unsigned int root_ino = F2FS_ROOT_INO(F2FS_I_SB(dir));

	struct fscrypt_name fname;





	trace_f2fs_lookup_start(dir, dentry, flags);

	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) {

	if (dentry->d_name.len > F2FS_NAME_LEN) {

		err = -ENAMETOOLONG;

		err = -ENAMETOOLONG;

		goto out;

		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 (!de) {

		if (IS_ERR(page)) {

		if (IS_ERR(page)) {

			err = PTR_ERR(page);

			err = PTR_ERR(page);
@@ -488,8 +492,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry, 
	}

	}

out_splice:

out_splice:

	new = d_splice_alias(inode, dentry);

	new = d_splice_alias(inode, dentry);

	if (IS_ERR(new))

	err = PTR_ERR_OR_ZERO(new);

		err = PTR_ERR(new);

	trace_f2fs_lookup_end(dir, dentry, ino, err);

	trace_f2fs_lookup_end(dir, dentry, ino, err);

	return new;

	return new;

out_iput:

out_iput: