Hi,
I'm resubmitting this as v9 since I think it has fallen through the
cracks :). It is a collection of trivial fixes for casefold support on
ext4/f2fs. More details below.
It has been sitting on the list for a while and most of it is r-b
already. I'm keeping the tags for this submission, since there is no
modifications from previous submissions, apart from a minor conflict
resolution when merging to linus/master.
Thanks,
v8: https://patchwork.ozlabs.org/project/linux-ext4/cover/[email protected]/
* Original commit letter
The case-insensitive implementations in f2fs and ext4 have quite a bit
of duplicated code. This series simplifies the ext4 version, with the
goal of extracting ext4_ci_compare into a helper library that can be
used by both filesystems. It also reduces the clutter from many
codeguards for CONFIG_UNICODE; as requested by Linus, they are part of
the codeflow now.
While there, I noticed we can leverage the utf8 functions to detect
encoded names that are corrupted in the filesystem. Therefore, it also
adds an ext4 error on that scenario, to mark the filesystem as
corrupted.
This series survived passes of xfstests -g quick.
Gabriel Krisman Bertazi (8):
ext4: Simplify the handling of cached insensitive names
f2fs: Simplify the handling of cached insensitive names
libfs: Introduce case-insensitive string comparison helper
ext4: Reuse generic_ci_match for ci comparisons
f2fs: Reuse generic_ci_match for ci comparisons
ext4: Log error when lookup of encoded dentry fails
ext4: Move CONFIG_UNICODE defguards into the code flow
f2fs: Move CONFIG_UNICODE defguards into the code flow
fs/ext4/crypto.c | 15 ++----
fs/ext4/ext4.h | 34 +++++++-----
fs/ext4/namei.c | 130 ++++++++++++++++-----------------------------
fs/ext4/super.c | 4 +-
fs/f2fs/dir.c | 105 +++++++++++-------------------------
fs/f2fs/f2fs.h | 15 +++++-
fs/f2fs/namei.c | 11 ++--
fs/f2fs/recovery.c | 5 +-
fs/f2fs/super.c | 8 +--
fs/libfs.c | 68 ++++++++++++++++++++++++
include/linux/fs.h | 4 ++
11 files changed, 198 insertions(+), 201 deletions(-)
--
2.37.3
Instead of reimplementing ext4_match_ci, use the new libfs helper.
It also adds a comment explaining why fname->cf_name.name must be
checked prior to the encryption hash optimization, because that tripped
me before.
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
Changes since v6:
- Readd cf_name != NULL on hash verification and comment explaining it (eric)
---
fs/ext4/namei.c | 91 +++++++++++++++----------------------------------
1 file changed, 27 insertions(+), 64 deletions(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 089ea8b42a1e..9c892c58abf2 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1364,58 +1364,6 @@ static void dx_insert_block(struct dx_frame *frame, u32 hash, ext4_lblk_t block)
}
#if IS_ENABLED(CONFIG_UNICODE)
-/*
- * Test whether a case-insensitive directory entry matches the filename
- * being searched for. If quick is set, assume the name being looked up
- * is already in the casefolded form.
- *
- * Returns: 0 if the directory entry matches, more than 0 if it
- * doesn't match or less than zero on error.
- */
-static int ext4_ci_compare(const struct inode *parent, const struct qstr *name,
- u8 *de_name, size_t de_name_len, bool quick)
-{
- const struct super_block *sb = parent->i_sb;
- const struct unicode_map *um = sb->s_encoding;
- struct fscrypt_str decrypted_name = FSTR_INIT(NULL, de_name_len);
- struct qstr entry = QSTR_INIT(de_name, de_name_len);
- int ret;
-
- if (IS_ENCRYPTED(parent)) {
- const struct fscrypt_str encrypted_name =
- FSTR_INIT(de_name, de_name_len);
-
- decrypted_name.name = kmalloc(de_name_len, GFP_KERNEL);
- if (!decrypted_name.name)
- return -ENOMEM;
- ret = fscrypt_fname_disk_to_usr(parent, 0, 0, &encrypted_name,
- &decrypted_name);
- if (ret < 0)
- goto out;
- entry.name = decrypted_name.name;
- entry.len = decrypted_name.len;
- }
-
- if (quick)
- ret = utf8_strncasecmp_folded(um, name, &entry);
- else
- ret = utf8_strncasecmp(um, name, &entry);
- if (ret < 0) {
- /* Handle invalid character sequence as either an error
- * or as an opaque byte sequence.
- */
- if (sb_has_strict_encoding(sb))
- ret = -EINVAL;
- else if (name->len != entry.len)
- ret = 1;
- else
- ret = !!memcmp(name->name, entry.name, entry.len);
- }
-out:
- kfree(decrypted_name.name);
- return ret;
-}
-
int ext4_fname_setup_ci_filename(struct inode *dir, const struct qstr *iname,
struct ext4_filename *name)
{
@@ -1478,20 +1426,35 @@ static bool ext4_match(struct inode *parent,
#if IS_ENABLED(CONFIG_UNICODE)
if (parent->i_sb->s_encoding && IS_CASEFOLDED(parent) &&
(!IS_ENCRYPTED(parent) || fscrypt_has_encryption_key(parent))) {
- if (fname->cf_name.name) {
- if (IS_ENCRYPTED(parent)) {
- if (fname->hinfo.hash != EXT4_DIRENT_HASH(de) ||
- fname->hinfo.minor_hash !=
- EXT4_DIRENT_MINOR_HASH(de)) {
+ int ret;
- return false;
- }
- }
- return !ext4_ci_compare(parent, &fname->cf_name,
- de->name, de->name_len, true);
+ /*
+ * Just checking IS_ENCRYPTED(parent) below is not
+ * sufficient to decide whether one can use the hash for
+ * skipping the string comparison, because the key might
+ * have been added right after
+ * ext4_fname_setup_ci_filename(). In this case, a hash
+ * mismatch will be a false negative. Therefore, make
+ * sure cf_name was properly initialized before
+ * considering the calculated hash.
+ */
+ if (IS_ENCRYPTED(parent) && fname->cf_name.name &&
+ (fname->hinfo.hash != EXT4_DIRENT_HASH(de) ||
+ fname->hinfo.minor_hash != EXT4_DIRENT_MINOR_HASH(de)))
+ return false;
+
+ ret = generic_ci_match(parent, fname->usr_fname,
+ &fname->cf_name, de->name,
+ de->name_len);
+ if (ret < 0) {
+ /*
+ * Treat comparison errors as not a match. The
+ * only case where it happens is on a disk
+ * corruption or ENOMEM.
+ */
+ return false;
}
- return !ext4_ci_compare(parent, fname->usr_fname, de->name,
- de->name_len, false);
+ return ret;
}
#endif
--
2.37.3
If the volume is in strict mode, ext4_ci_compare can report a broken
encoding name. This will not trigger on a bad lookup, which is caught
earlier, only if the actual disk name is bad.
Reviewed-by: Eric Biggers <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
changes since v4:
- Reword error message (Eric)
Changes since v1:
- reword error message "file in directory" -> "filename" (Eric)
---
fs/ext4/namei.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 9c892c58abf2..959346950b3c 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1452,6 +1452,9 @@ static bool ext4_match(struct inode *parent,
* only case where it happens is on a disk
* corruption or ENOMEM.
*/
+ if (ret == -EINVAL)
+ EXT4_ERROR_INODE(parent,
+ "Directory contains filename that is invalid UTF-8");
return false;
}
return ret;
--
2.37.3
Instead of a bunch of ifdefs, make the unicode built checks part of the
code flow where possible, as requested by Torvalds.
Reviewed-by: Eric Biggers <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
Changes since v8:
- Rebase on top of 3030b59c8533 ("ext4: cleanup function defs from ext4.h into crypto.c")
Changes since v5:
- Drop err variable (eric)
Changes since v4:
- Create stub for !CONFIG_UNICODE case (eric)
---
fs/ext4/crypto.c | 15 +++------------
fs/ext4/ext4.h | 32 ++++++++++++++++++++------------
fs/ext4/namei.c | 15 ++++++---------
fs/ext4/super.c | 4 +---
4 files changed, 30 insertions(+), 36 deletions(-)
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index e20ac0654b3f..c65d8a5f8315 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -31,10 +31,7 @@ int ext4_fname_setup_filename(struct inode *dir, const struct qstr *iname,
ext4_fname_from_fscrypt_name(fname, &name);
-#if IS_ENABLED(CONFIG_UNICODE)
- err = ext4_fname_setup_ci_filename(dir, iname, fname);
-#endif
- return err;
+ return ext4_fname_setup_ci_filename(dir, iname, fname);
}
int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry,
@@ -49,10 +46,7 @@ int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry,
ext4_fname_from_fscrypt_name(fname, &name);
-#if IS_ENABLED(CONFIG_UNICODE)
- err = ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname);
-#endif
- return err;
+ return ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname);
}
void ext4_fname_free_filename(struct ext4_filename *fname)
@@ -66,10 +60,7 @@ void ext4_fname_free_filename(struct ext4_filename *fname)
fname->usr_fname = NULL;
fname->disk_name.name = NULL;
-#if IS_ENABLED(CONFIG_UNICODE)
- kfree(fname->cf_name.name);
- fname->cf_name.name = NULL;
-#endif
+ ext4_fname_free_ci_filename(fname);
}
static bool uuid_is_zero(__u8 u[16])
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 9717b9832425..f2a673ac67fe 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2734,8 +2734,24 @@ ext4_fsblk_t ext4_inode_to_goal_block(struct inode *);
#if IS_ENABLED(CONFIG_UNICODE)
extern int ext4_fname_setup_ci_filename(struct inode *dir,
- const struct qstr *iname,
- struct ext4_filename *fname);
+ const struct qstr *iname,
+ struct ext4_filename *fname);
+
+static inline void ext4_fname_free_ci_filename(struct ext4_filename *fname)
+{
+ kfree(fname->cf_name.name);
+ fname->cf_name.name = NULL;
+}
+#else
+static inline int ext4_fname_setup_ci_filename(struct inode *dir,
+ const struct qstr *iname,
+ struct ext4_filename *fname)
+{
+ return 0;
+}
+static inline void ext4_fname_free_ci_filename(struct ext4_filename *fname)
+{
+}
#endif
/* ext4 encryption related stuff goes here crypto.c */
@@ -2758,16 +2774,11 @@ static inline int ext4_fname_setup_filename(struct inode *dir,
int lookup,
struct ext4_filename *fname)
{
- int err = 0;
fname->usr_fname = iname;
fname->disk_name.name = (unsigned char *) iname->name;
fname->disk_name.len = iname->len;
-#if IS_ENABLED(CONFIG_UNICODE)
- err = ext4_fname_setup_ci_filename(dir, iname, fname);
-#endif
-
- return err;
+ return ext4_fname_setup_ci_filename(dir, iname, fname);
}
static inline int ext4_fname_prepare_lookup(struct inode *dir,
@@ -2779,10 +2790,7 @@ static inline int ext4_fname_prepare_lookup(struct inode *dir,
static inline void ext4_fname_free_filename(struct ext4_filename *fname)
{
-#if IS_ENABLED(CONFIG_UNICODE)
- kfree(fname->cf_name.name);
- fname->cf_name.name = NULL;
-#endif
+ ext4_fname_free_ci_filename(fname);
}
static inline int ext4_ioctl_get_encryption_pwsalt(struct file *filp,
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 959346950b3c..100715586473 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1811,8 +1811,7 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi
}
}
-#if IS_ENABLED(CONFIG_UNICODE)
- if (!inode && IS_CASEFOLDED(dir)) {
+ if (IS_ENABLED(CONFIG_UNICODE) && !inode && IS_CASEFOLDED(dir)) {
/* Eventually we want to call d_add_ci(dentry, NULL)
* for negative dentries in the encoding case as
* well. For now, prevent the negative dentry
@@ -1820,7 +1819,7 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi
*/
return NULL;
}
-#endif
+
return d_splice_alias(inode, dentry);
}
@@ -3138,16 +3137,14 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
ext4_fc_track_unlink(handle, dentry);
retval = ext4_mark_inode_dirty(handle, dir);
-#if IS_ENABLED(CONFIG_UNICODE)
/* VFS negative dentries are incompatible with Encoding and
* Case-insensitiveness. Eventually we'll want avoid
* invalidating the dentries here, alongside with returning the
* negative dentries at ext4_lookup(), when it is better
* supported by the VFS for the CI case.
*/
- if (IS_CASEFOLDED(dir))
+ if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir))
d_invalidate(dentry);
-#endif
end_rmdir:
brelse(bh);
@@ -3243,16 +3240,16 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
retval = __ext4_unlink(handle, dir, &dentry->d_name, d_inode(dentry));
if (!retval)
ext4_fc_track_unlink(handle, dentry);
-#if IS_ENABLED(CONFIG_UNICODE)
+
/* VFS negative dentries are incompatible with Encoding and
* Case-insensitiveness. Eventually we'll want avoid
* invalidating the dentries here, alongside with returning the
* negative dentries at ext4_lookup(), when it is better
* supported by the VFS for the CI case.
*/
- if (IS_CASEFOLDED(dir))
+ if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir))
d_invalidate(dentry);
-#endif
+
if (handle)
ext4_journal_stop(handle);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 9a66abcca1a8..6c8f5fa5e455 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3569,14 +3569,12 @@ int ext4_feature_set_ok(struct super_block *sb, int readonly)
return 0;
}
-#if !IS_ENABLED(CONFIG_UNICODE)
- if (ext4_has_feature_casefold(sb)) {
+ if (!IS_ENABLED(CONFIG_UNICODE) && ext4_has_feature_casefold(sb)) {
ext4_msg(sb, KERN_ERR,
"Filesystem with casefold feature cannot be "
"mounted without CONFIG_UNICODE");
return 0;
}
-#endif
if (readonly)
return 1;
--
2.37.3
Now that ci_match is part of libfs, make f2fs reuse it instead of having
a different implementation.
Reviewed-by: Chao Yu <[email protected]>
Reviewed-by: Eric Biggers <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
Changes since v3:
- Drop cast (eric)
- fix unused variable iff !CONFIG_UNICODE (lkp)
---
fs/f2fs/dir.c | 58 ++++-----------------------------------------------
1 file changed, 4 insertions(+), 54 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 98db333a1338..bce3727de3c9 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -218,58 +218,6 @@ static struct f2fs_dir_entry *find_in_block(struct inode *dir,
return f2fs_find_target_dentry(&d, fname, max_slots);
}
-#if IS_ENABLED(CONFIG_UNICODE)
-/*
- * Test whether a case-insensitive directory entry matches the filename
- * being searched for.
- *
- * Returns 1 for a match, 0 for no match, and -errno on an error.
- */
-static int f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
- const u8 *de_name, u32 de_name_len)
-{
- const struct super_block *sb = dir->i_sb;
- const struct unicode_map *um = sb->s_encoding;
- struct fscrypt_str decrypted_name = FSTR_INIT(NULL, de_name_len);
- struct qstr entry = QSTR_INIT(de_name, de_name_len);
- int res;
-
- if (IS_ENCRYPTED(dir)) {
- const struct fscrypt_str encrypted_name =
- FSTR_INIT((u8 *)de_name, de_name_len);
-
- if (WARN_ON_ONCE(!fscrypt_has_encryption_key(dir)))
- return -EINVAL;
-
- decrypted_name.name = kmalloc(de_name_len, GFP_KERNEL);
- if (!decrypted_name.name)
- return -ENOMEM;
- res = fscrypt_fname_disk_to_usr(dir, 0, 0, &encrypted_name,
- &decrypted_name);
- if (res < 0)
- goto out;
- entry.name = decrypted_name.name;
- entry.len = decrypted_name.len;
- }
-
- res = utf8_strncasecmp_folded(um, name, &entry);
- /*
- * In strict mode, ignore invalid names. In non-strict mode,
- * fall back to treating them as opaque byte sequences.
- */
- if (res < 0 && !sb_has_strict_encoding(sb)) {
- res = name->len == entry.len &&
- memcmp(name->name, entry.name, name->len) == 0;
- } else {
- /* utf8_strncasecmp_folded returns 0 on match */
- res = (res == 0);
- }
-out:
- kfree(decrypted_name.name);
- return res;
-}
-#endif /* CONFIG_UNICODE */
-
static inline int f2fs_match_name(const struct inode *dir,
const struct f2fs_filename *fname,
const u8 *de_name, u32 de_name_len)
@@ -278,8 +226,10 @@ static inline int f2fs_match_name(const struct inode *dir,
#if IS_ENABLED(CONFIG_UNICODE)
if (fname->cf_name.name)
- return f2fs_match_ci_name(dir, &fname->cf_name,
- de_name, de_name_len);
+ return generic_ci_match(dir, fname->usr_fname,
+ &fname->cf_name,
+ de_name, de_name_len);
+
#endif
f.usr_fname = fname->usr_fname;
f.disk_name = fname->disk_name;
--
2.37.3
generic_ci_match can be used by case-insensitive filesystems to compare
strings under lookup with dirents in a case-insensitive way. This
function is currently reimplemented by each filesystem supporting
casefolding, so this reduces code duplication in filesystem-specific
code.
Reviewed-by: Eric Biggers <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
Changes since v5:
(eric)
- Rename variable err -> res
- Retype de_name_len from size_t to u32
- Bring WARN_ON_ONCE(!fscrypt_has_encryption_key) from f2fs implementation
---
fs/libfs.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 4 +++
2 files changed, 72 insertions(+)
diff --git a/fs/libfs.c b/fs/libfs.c
index 31b0ddf01c31..f8f95e2f13c1 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1454,6 +1454,74 @@ static const struct dentry_operations generic_ci_dentry_ops = {
.d_hash = generic_ci_d_hash,
.d_compare = generic_ci_d_compare,
};
+
+/**
+ * generic_ci_match() - Match a name (case-insensitively) with a dirent.
+ * @parent: Inode of the parent of the dirent under comparison
+ * @name: name under lookup.
+ * @folded_name: Optional pre-folded name under lookup
+ * @de_name: Dirent name.
+ * @de_name_len: dirent name length.
+ *
+ *
+ * Test whether a case-insensitive directory entry matches the filename
+ * being searched. If @folded_name is provided, it is used instead of
+ * recalculating the casefold of @name.
+ *
+ * Return: > 0 if the directory entry matches, 0 if it doesn't match, or
+ * < 0 on error.
+ */
+int generic_ci_match(const struct inode *parent,
+ const struct qstr *name,
+ const struct qstr *folded_name,
+ const u8 *de_name, u32 de_name_len)
+{
+ const struct super_block *sb = parent->i_sb;
+ const struct unicode_map *um = sb->s_encoding;
+ struct fscrypt_str decrypted_name = FSTR_INIT(NULL, de_name_len);
+ struct qstr dirent = QSTR_INIT(de_name, de_name_len);
+ int res, match = false;
+
+ if (IS_ENCRYPTED(parent)) {
+ const struct fscrypt_str encrypted_name =
+ FSTR_INIT((u8 *) de_name, de_name_len);
+
+ if (WARN_ON_ONCE(!fscrypt_has_encryption_key(parent)))
+ return -EINVAL;
+
+ decrypted_name.name = kmalloc(de_name_len, GFP_KERNEL);
+ if (!decrypted_name.name)
+ return -ENOMEM;
+ res = fscrypt_fname_disk_to_usr(parent, 0, 0, &encrypted_name,
+ &decrypted_name);
+ if (res < 0)
+ goto out;
+ dirent.name = decrypted_name.name;
+ dirent.len = decrypted_name.len;
+ }
+
+ if (folded_name->name)
+ res = utf8_strncasecmp_folded(um, folded_name, &dirent);
+ else
+ res = utf8_strncasecmp(um, name, &dirent);
+
+ if (!res)
+ match = true;
+ else if (res < 0 && !sb_has_strict_encoding(sb)) {
+ /*
+ * In non-strict mode, fallback to a byte comparison if
+ * the names have invalid characters.
+ */
+ res = 0;
+ match = ((name->len == dirent.len) &&
+ !memcmp(name->name, dirent.name, dirent.len));
+ }
+
+out:
+ kfree(decrypted_name.name);
+ return (res >= 0) ? match : res;
+}
+EXPORT_SYMBOL(generic_ci_match);
#endif
#ifdef CONFIG_FS_ENCRYPTION
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9eced4cc286e..d43178edb3d5 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3352,6 +3352,10 @@ extern int generic_file_fsync(struct file *, loff_t, loff_t, int);
extern int generic_check_addressable(unsigned, u64);
extern void generic_set_encrypted_ci_d_ops(struct dentry *dentry);
+extern int generic_ci_match(const struct inode *parent,
+ const struct qstr *name,
+ const struct qstr *folded_name,
+ const u8 *de_name, u32 de_name_len);
int may_setattr(struct user_namespace *mnt_userns, struct inode *inode,
unsigned int ia_valid);
--
2.37.3
Instead of a bunch of ifdefs, make the unicode built checks part of the
code flow where possible, as requested by Torvalds.
Reviewed-by: Eric Biggers <[email protected]>
Reviewed-by: Chao Yu <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
Changes since v4:
- Drop stub removal for !CONFIG_UNICODE case (eric)
---
fs/f2fs/namei.c | 11 +++++------
fs/f2fs/super.c | 8 ++++----
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index bf00d5057abb..46325cac4fb6 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -571,8 +571,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
goto out_iput;
}
out_splice:
-#if IS_ENABLED(CONFIG_UNICODE)
- if (!inode && IS_CASEFOLDED(dir)) {
+ if (IS_ENABLED(CONFIG_UNICODE) && !inode && IS_CASEFOLDED(dir)) {
/* Eventually we want to call d_add_ci(dentry, NULL)
* for negative dentries in the encoding case as
* well. For now, prevent the negative dentry
@@ -581,7 +580,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
trace_f2fs_lookup_end(dir, dentry, ino, err);
return NULL;
}
-#endif
+
new = d_splice_alias(inode, dentry);
err = PTR_ERR_OR_ZERO(new);
trace_f2fs_lookup_end(dir, dentry, ino, !new ? -ENOENT : err);
@@ -632,16 +631,16 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
goto fail;
}
f2fs_delete_entry(de, page, dir, inode);
-#if IS_ENABLED(CONFIG_UNICODE)
+
/* VFS negative dentries are incompatible with Encoding and
* Case-insensitiveness. Eventually we'll want avoid
* invalidating the dentries here, alongside with returning the
* negative dentries at f2fs_lookup(), when it is better
* supported by the VFS for the CI case.
*/
- if (IS_CASEFOLDED(dir))
+ if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir))
d_invalidate(dentry);
-#endif
+
f2fs_unlock_op(sbi);
if (IS_DIRSYNC(dir))
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 2451623c05a7..636488d65342 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -284,7 +284,7 @@ struct kmem_cache *f2fs_cf_name_slab;
static int __init f2fs_create_casefold_cache(void)
{
f2fs_cf_name_slab = f2fs_kmem_cache_create("f2fs_casefolded_name",
- F2FS_NAME_LEN);
+ F2FS_NAME_LEN);
if (!f2fs_cf_name_slab)
return -ENOMEM;
return 0;
@@ -1273,13 +1273,13 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
return -EINVAL;
}
#endif
-#if !IS_ENABLED(CONFIG_UNICODE)
- if (f2fs_sb_has_casefold(sbi)) {
+
+ if (!IS_ENABLED(CONFIG_UNICODE) && f2fs_sb_has_casefold(sbi)) {
f2fs_err(sbi,
"Filesystem with casefold feature cannot be mounted without CONFIG_UNICODE");
return -EINVAL;
}
-#endif
+
/*
* The BLKZONED feature indicates that the drive was formatted with
* zone alignment optimization. This is optional for host-aware
--
2.37.3
Keeping it as qstr avoids the unnecessary conversion in ext4_match
Reviewed-by: Eric Biggers <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
Changes since v1:
- Simplify hunk (eric)
---
fs/ext4/ext4.h | 2 +-
fs/ext4/namei.c | 23 +++++++++++------------
2 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 9bca5565547b..9717b9832425 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2497,7 +2497,7 @@ struct ext4_filename {
struct fscrypt_str crypto_buf;
#endif
#if IS_ENABLED(CONFIG_UNICODE)
- struct fscrypt_str cf_name;
+ struct qstr cf_name;
#endif
};
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 3a31b662f661..089ea8b42a1e 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1419,7 +1419,8 @@ static int ext4_ci_compare(const struct inode *parent, const struct qstr *name,
int ext4_fname_setup_ci_filename(struct inode *dir, const struct qstr *iname,
struct ext4_filename *name)
{
- struct fscrypt_str *cf_name = &name->cf_name;
+ struct qstr *cf_name = &name->cf_name;
+ unsigned char *buf;
struct dx_hash_info *hinfo = &name->hinfo;
int len;
@@ -1429,18 +1430,18 @@ int ext4_fname_setup_ci_filename(struct inode *dir, const struct qstr *iname,
return 0;
}
- cf_name->name = kmalloc(EXT4_NAME_LEN, GFP_NOFS);
- if (!cf_name->name)
+ buf = kmalloc(EXT4_NAME_LEN, GFP_NOFS);
+ if (!buf)
return -ENOMEM;
- len = utf8_casefold(dir->i_sb->s_encoding,
- iname, cf_name->name,
- EXT4_NAME_LEN);
+ len = utf8_casefold(dir->i_sb->s_encoding, iname, buf, EXT4_NAME_LEN);
if (len <= 0) {
- kfree(cf_name->name);
- cf_name->name = NULL;
+ kfree(buf);
+ buf = NULL;
}
+ cf_name->name = buf;
cf_name->len = (unsigned) len;
+
if (!IS_ENCRYPTED(dir))
return 0;
@@ -1478,8 +1479,6 @@ static bool ext4_match(struct inode *parent,
if (parent->i_sb->s_encoding && IS_CASEFOLDED(parent) &&
(!IS_ENCRYPTED(parent) || fscrypt_has_encryption_key(parent))) {
if (fname->cf_name.name) {
- struct qstr cf = {.name = fname->cf_name.name,
- .len = fname->cf_name.len};
if (IS_ENCRYPTED(parent)) {
if (fname->hinfo.hash != EXT4_DIRENT_HASH(de) ||
fname->hinfo.minor_hash !=
@@ -1488,8 +1487,8 @@ static bool ext4_match(struct inode *parent,
return false;
}
}
- return !ext4_ci_compare(parent, &cf, de->name,
- de->name_len, true);
+ return !ext4_ci_compare(parent, &fname->cf_name,
+ de->name, de->name_len, true);
}
return !ext4_ci_compare(parent, fname->usr_fname, de->name,
de->name_len, false);
--
2.37.3
Keeping it as qstr avoids the unnecessary conversion in f2fs_match
Reviewed-by: Eric Biggers <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
Changes since v4:
- Fix inconsistent return on error (eric)
---
fs/f2fs/dir.c | 53 ++++++++++++++++++++++++++--------------------
fs/f2fs/f2fs.h | 15 ++++++++++++-
fs/f2fs/recovery.c | 5 +----
3 files changed, 45 insertions(+), 28 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index d5bd7932fb64..98db333a1338 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -75,35 +75,49 @@ unsigned char f2fs_get_de_type(struct f2fs_dir_entry *de)
return DT_UNKNOWN;
}
+#if IS_ENABLED(CONFIG_UNICODE)
/* If @dir is casefolded, initialize @fname->cf_name from @fname->usr_fname. */
int f2fs_init_casefolded_name(const struct inode *dir,
struct f2fs_filename *fname)
{
-#if IS_ENABLED(CONFIG_UNICODE)
struct super_block *sb = dir->i_sb;
+ unsigned char *buf;
+ int len;
if (IS_CASEFOLDED(dir) &&
!is_dot_dotdot(fname->usr_fname->name, fname->usr_fname->len)) {
- fname->cf_name.name = f2fs_kmem_cache_alloc(f2fs_cf_name_slab,
- GFP_NOFS, false, F2FS_SB(sb));
- if (!fname->cf_name.name)
+ buf = f2fs_kmem_cache_alloc(f2fs_cf_name_slab,
+ GFP_NOFS, false, F2FS_SB(sb));
+ if (!buf)
return -ENOMEM;
- fname->cf_name.len = utf8_casefold(sb->s_encoding,
- fname->usr_fname,
- fname->cf_name.name,
- F2FS_NAME_LEN);
- if ((int)fname->cf_name.len <= 0) {
- kmem_cache_free(f2fs_cf_name_slab, fname->cf_name.name);
- fname->cf_name.name = NULL;
+
+ len = utf8_casefold(sb->s_encoding, fname->usr_fname,
+ buf, F2FS_NAME_LEN);
+ if (len <= 0) {
+ kmem_cache_free(f2fs_cf_name_slab, buf);
if (sb_has_strict_encoding(sb))
return -EINVAL;
/* fall back to treating name as opaque byte sequence */
+ return 0;
}
+ fname->cf_name.name = buf;
+ fname->cf_name.len = len;
}
-#endif
+
return 0;
}
+void f2fs_free_casefolded_name(struct f2fs_filename *fname)
+{
+ unsigned char *buf = (unsigned char *)fname->cf_name.name;
+
+ if (buf) {
+ kmem_cache_free(f2fs_cf_name_slab, buf);
+ fname->cf_name.name = NULL;
+ }
+}
+#endif /* CONFIG_UNICODE */
+
static int __f2fs_setup_filename(const struct inode *dir,
const struct fscrypt_name *crypt_name,
struct f2fs_filename *fname)
@@ -175,12 +189,7 @@ void f2fs_free_filename(struct f2fs_filename *fname)
kfree(fname->crypto_buf.name);
fname->crypto_buf.name = NULL;
#endif
-#if IS_ENABLED(CONFIG_UNICODE)
- if (fname->cf_name.name) {
- kmem_cache_free(f2fs_cf_name_slab, fname->cf_name.name);
- fname->cf_name.name = NULL;
- }
-#endif
+ f2fs_free_casefolded_name(fname);
}
static unsigned long dir_block_index(unsigned int level,
@@ -268,11 +277,9 @@ static inline int f2fs_match_name(const struct inode *dir,
struct fscrypt_name f;
#if IS_ENABLED(CONFIG_UNICODE)
- if (fname->cf_name.name) {
- struct qstr cf = FSTR_TO_QSTR(&fname->cf_name);
-
- return f2fs_match_ci_name(dir, &cf, de_name, de_name_len);
- }
+ if (fname->cf_name.name)
+ return f2fs_match_ci_name(dir, &fname->cf_name,
+ de_name, de_name_len);
#endif
f.usr_fname = fname->usr_fname;
f.disk_name = fname->disk_name;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 3c7cdb70fe2e..2a0cc75665d6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -515,7 +515,7 @@ struct f2fs_filename {
* internal operation where usr_fname is also NULL. In all these cases
* we fall back to treating the name as an opaque byte sequence.
*/
- struct fscrypt_str cf_name;
+ struct qstr cf_name;
#endif
};
@@ -3484,8 +3484,21 @@ int f2fs_get_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
* dir.c
*/
unsigned char f2fs_get_de_type(struct f2fs_dir_entry *de);
+#if IS_ENABLED(CONFIG_UNICODE)
int f2fs_init_casefolded_name(const struct inode *dir,
struct f2fs_filename *fname);
+void f2fs_free_casefolded_name(struct f2fs_filename *fname);
+#else
+static inline int f2fs_init_casefolded_name(const struct inode *dir,
+ struct f2fs_filename *fname)
+{
+ return 0;
+}
+static inline void f2fs_free_casefolded_name(struct f2fs_filename *fname)
+{
+}
+#endif /* CONFIG_UNICODE */
+
int f2fs_setup_filename(struct inode *dir, const struct qstr *iname,
int lookup, struct f2fs_filename *fname);
int f2fs_prepare_lookup(struct inode *dir, struct dentry *dentry,
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index dcd0a1e35095..ead49ea25ae8 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -153,11 +153,8 @@ static int init_recovered_filename(const struct inode *dir,
if (err)
return err;
f2fs_hash_filename(dir, fname);
-#if IS_ENABLED(CONFIG_UNICODE)
/* Case-sensitive match is fine for recovery */
- kmem_cache_free(f2fs_cf_name_slab, fname->cf_name.name);
- fname->cf_name.name = NULL;
-#endif
+ f2fs_free_casefolded_name(fname);
} else {
f2fs_hash_filename(dir, fname);
}
--
2.37.3
On Tue, Sep 13, 2022 at 07:41:42PM -0400, Gabriel Krisman Bertazi wrote:
> Hi,
>
> I'm resubmitting this as v9 since I think it has fallen through the
> cracks :). It is a collection of trivial fixes for casefold support on
> ext4/f2fs. More details below.
>
> It has been sitting on the list for a while and most of it is r-b
> already. I'm keeping the tags for this submission, since there is no
> modifications from previous submissions, apart from a minor conflict
> resolution when merging to linus/master.
Who are you expecting to apply this?
- Eric
On Tue, Sep 13, 2022 at 07:41:46PM -0400, Gabriel Krisman Bertazi wrote:
> Instead of reimplementing ext4_match_ci, use the new libfs helper.
>
> It also adds a comment explaining why fname->cf_name.name must be
> checked prior to the encryption hash optimization, because that tripped
> me before.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
Reviewed-by: Eric Biggers <[email protected]>
- Eric
Eric Biggers <[email protected]> writes:
> On Tue, Sep 13, 2022 at 07:41:42PM -0400, Gabriel Krisman Bertazi wrote:
>> Hi,
>>
>> I'm resubmitting this as v9 since I think it has fallen through the
>> cracks :). It is a collection of trivial fixes for casefold support on
>> ext4/f2fs. More details below.
>>
>> It has been sitting on the list for a while and most of it is r-b
>> already. I'm keeping the tags for this submission, since there is no
>> modifications from previous submissions, apart from a minor conflict
>> resolution when merging to linus/master.
>
> Who are you expecting to apply this?
Hi Eric,
There are three groups of changes here: libfs, ext4 and f2fs. Since the
changes in libfs are self-contained and only affect these two
filesystems, I think it should be fine for them to go through a fs tree.
The bulk of changes are ext4, and Ted mentioned on an earlier version
that he could pick the first patches of this series, so I'm thinking it
should all go through the ext4 tree. If Jaegeuk acks, the f2fs changes
are safe to go with the rest, or I can send them afterwards as a
separate series once the libfs code is merged.
Thanks,
--
Gabriel Krisman Bertazi
Gabriel Krisman Bertazi <[email protected]> writes:
> Eric Biggers <[email protected]> writes:
>
>> On Tue, Sep 13, 2022 at 07:41:42PM -0400, Gabriel Krisman Bertazi wrote:
>>> Hi,
>>>
>>> I'm resubmitting this as v9 since I think it has fallen through the
>>> cracks :). It is a collection of trivial fixes for casefold support on
>>> ext4/f2fs. More details below.
>>>
>>> It has been sitting on the list for a while and most of it is r-b
>>> already. I'm keeping the tags for this submission, since there is no
>>> modifications from previous submissions, apart from a minor conflict
>>> resolution when merging to linus/master.
>>
>> Who are you expecting to apply this?
>
> Hi Eric,
>
> There are three groups of changes here: libfs, ext4 and f2fs. Since the
> changes in libfs are self-contained and only affect these two
> filesystems, I think it should be fine for them to go through a fs tree.
>
> The bulk of changes are ext4, and Ted mentioned on an earlier version
> that he could pick the first patches of this series, so I'm thinking it
> should all go through the ext4 tree. If Jaegeuk acks, the f2fs changes
> are safe to go with the rest, or I can send them afterwards as a
> separate series once the libfs code is merged.
Ted,
Does the above plan work for you? Do you intend to pick this up for the
next merge window?
--
Gabriel Krisman Bertazi
On 10/14/22 4:45 AM, Gabriel Krisman Bertazi wrote:
> Gabriel Krisman Bertazi <[email protected]> writes:
>
>> Eric Biggers <[email protected]> writes:
>>
>>> On Tue, Sep 13, 2022 at 07:41:42PM -0400, Gabriel Krisman Bertazi wrote:
>>>> Hi,
>>>>
>>>> I'm resubmitting this as v9 since I think it has fallen through the
>>>> cracks :). It is a collection of trivial fixes for casefold support on
>>>> ext4/f2fs. More details below.
>>>>
>>>> It has been sitting on the list for a while and most of it is r-b
>>>> already. I'm keeping the tags for this submission, since there is no
>>>> modifications from previous submissions, apart from a minor conflict
>>>> resolution when merging to linus/master.
>>>
>>> Who are you expecting to apply this?
>>
>> Hi Eric,
>>
>> There are three groups of changes here: libfs, ext4 and f2fs. Since the
>> changes in libfs are self-contained and only affect these two
>> filesystems, I think it should be fine for them to go through a fs tree.
>>
>> The bulk of changes are ext4, and Ted mentioned on an earlier version
>> that he could pick the first patches of this series, so I'm thinking it
>> should all go through the ext4 tree. If Jaegeuk acks, the f2fs changes
>> are safe to go with the rest, or I can send them afterwards as a
>> separate series once the libfs code is merged.
>
> Ted,
>
> Does the above plan work for you? Do you intend to pick this up for the
> next merge window?
It seems like this series hasn't been picked up. Any ideas on what can be done?
--
BR,
Muhammad Usama Anjum
Muhammad Usama Anjum <[email protected]> writes:
> On 10/14/22 4:45 AM, Gabriel Krisman Bertazi wrote:
>> Gabriel Krisman Bertazi <[email protected]> writes:
>>
>>> Eric Biggers <[email protected]> writes:
>>>
>>>> On Tue, Sep 13, 2022 at 07:41:42PM -0400, Gabriel Krisman Bertazi wrote:
>>>>> Hi,
>>>>>
>>>>> I'm resubmitting this as v9 since I think it has fallen through the
>>>>> cracks :). It is a collection of trivial fixes for casefold support on
>>>>> ext4/f2fs. More details below.
>>>>>
>>>>> It has been sitting on the list for a while and most of it is r-b
>>>>> already. I'm keeping the tags for this submission, since there is no
>>>>> modifications from previous submissions, apart from a minor conflict
>>>>> resolution when merging to linus/master.
>>>>
>>>> Who are you expecting to apply this?
>>>
>>> Hi Eric,
>>>
>>> There are three groups of changes here: libfs, ext4 and f2fs. Since the
>>> changes in libfs are self-contained and only affect these two
>>> filesystems, I think it should be fine for them to go through a fs tree.
>>>
>>> The bulk of changes are ext4, and Ted mentioned on an earlier version
>>> that he could pick the first patches of this series, so I'm thinking it
>>> should all go through the ext4 tree. If Jaegeuk acks, the f2fs changes
>>> are safe to go with the rest, or I can send them afterwards as a
>>> separate series once the libfs code is merged.
>>
>> Ted,
>>
>> Does the above plan work for you? Do you intend to pick this up for the
>> next merge window?
> It seems like this series hasn't been picked up. Any ideas on what can
> be done?
I got tired of the radio silence and gave up on it. If there is interest,
feel free to respin it once more.
--
Gabriel Krisman Bertazi