Re: [PATCH v8 8/8] f2fs: Handle casefolding with Encryption
From: Eric Biggers <ebiggers@kernel.org>
Date: 2020-03-07 05:24:30
Also in:
linux-ext4, linux-f2fs-devel, linux-fscrypt, linux-fsdevel, lkml
On Fri, Mar 06, 2020 at 06:36:11PM -0800, Daniel Rosenberg wrote:
quoted hunk ↗ jump to hunk
int f2fs_add_regular_entry(struct inode *dir, const struct qstr *new_name, const struct qstr *orig_name, + f2fs_hash_t dentry_hash, struct inode *inode, nid_t ino, umode_t mode) { unsigned int bit_pos; unsigned int level; unsigned int current_depth; unsigned long bidx, block; - f2fs_hash_t dentry_hash; unsigned int nbucket, nblock; struct page *dentry_page = NULL; struct f2fs_dentry_block *dentry_blk = NULL;@@ -632,7 +650,6 @@ int f2fs_add_regular_entry(struct inode *dir, const struct qstr *new_name, level = 0; slots = GET_DENTRY_SLOTS(new_name->len); - dentry_hash = f2fs_dentry_hash(dir, new_name, NULL); current_depth = F2FS_I(dir)->i_current_depth; if (F2FS_I(dir)->chash == dentry_hash) {@@ -718,17 +735,19 @@ int f2fs_add_dentry(struct inode *dir, struct fscrypt_name *fname, struct inode *inode, nid_t ino, umode_t mode) { struct qstr new_name; + f2fs_hash_t dentry_hash; int err = -EAGAIN; new_name.name = fname_name(fname); new_name.len = fname_len(fname); if (f2fs_has_inline_dentry(dir)) - err = f2fs_add_inline_entry(dir, &new_name, fname->usr_fname, + err = f2fs_add_inline_entry(dir, &new_name, fname, inode, ino, mode); + dentry_hash = f2fs_dentry_hash(dir, &new_name, fname); if (err == -EAGAIN) err = f2fs_add_regular_entry(dir, &new_name, fname->usr_fname, - inode, ino, mode); + dentry_hash, inode, ino, mode);
Why are the changes to f2fs_add_dentry(), f2fs_add_regular_entry(), and f2fs_add_inline_entry() being made? Directories can't be modified through no-key names, so there's no need to make this part of the code handle grabbing the dentry hash from the struct fscrypt_name. And both the on-disk and original filenames were already passed to these functions. So what else do we need?
quoted hunk ↗ jump to hunk
+static f2fs_hash_t __f2fs_dentry_hash(const struct inode *dir, + const struct qstr *name_info, + const struct fscrypt_name *fname) { __u32 hash; f2fs_hash_t f2fs_hash;@@ -79,12 +80,17 @@ static f2fs_hash_t __f2fs_dentry_hash(const struct qstr *name_info, size_t len = name_info->len; /* encrypted bigname case */ - if (fname && !fname->disk_name.name) + if (fname && !fname->is_ciphertext_name) return cpu_to_le32(fname->hash);
Isn't this check backwards? The hash is valid if fname->is_ciphertext_name, not if !fname->is_ciphertext_name. (Maybe the phrase "ciphertext name" is causing confusion, as we're now calling them "no-key names" instead? We could rename it, if that would help.) - Eric