Prepare for upcoming Casefolding/Encryption patches

PATCH Superseded Daniel Rosenberg 4 participants 19 messages Jun 24, 2020 – Jul 7, 2020

Subsystems (7)

Documentation VFS Memory technology devices (MTD) ext4 file system +3 more subsystems

lore

Share & tools

lore

Daniel Rosenberg

author

2020-06-24 04:33 UTC #0 lore raw

v9 cover Prepare for upcoming Casefolding/Encryption patches

This lays the ground work for enabling casefolding and encryption at the same time for ext4 and f2fs. A future set of patches will enable that functionality. These unify the highly similar dentry_operations that ext4 and f2fs both use for casefolding.

Daniel Rosenberg (4):
unicode: Add utf8_casefold_hash
fs: Add standard casefolding support
f2fs: Use generic casefolding support
ext4: Use generic casefolding support

 fs/ext4/dir.c           |  64 +------------------------
 fs/ext4/ext4.h          |  12 -----
 fs/ext4/hash.c          |   2 +-
 fs/ext4/namei.c         |  20 ++++----
 fs/ext4/super.c         |  12 ++---
 fs/f2fs/dir.c           |  84 ++++-----------------------------
 fs/f2fs/f2fs.h          |   4 --
 fs/f2fs/super.c         |  10 ++--
 fs/f2fs/sysfs.c         |  10 ++--
 fs/libfs.c              | 101 ++++++++++++++++++++++++++++++++++++++++
 fs/unicode/utf8-core.c  |  23 ++++++++-
 include/linux/f2fs_fs.h |   3 --
 include/linux/fs.h      |  22 +++++++++
 include/linux/unicode.h |   3 ++
 14 files changed, 186 insertions(+), 184 deletions(-)

Signature · 1 line

2.27.0.111.gc72c7da667-goog

Daniel Rosenberg

author

2020-06-24 04:34 UTC #1 lore raw

PATCH v9 1/4 unicode: Add utf8_casefold_hash

This adds a case insensitive hash function to allow taking the hash without needing to allocate a casefolded copy of the string.

Signed-off-by Daniel Rosenberg <[email protected]>

---
 fs/unicode/utf8-core.c  | 23 ++++++++++++++++++++++-
 include/linux/unicode.h |  3 +++
 2 files changed, 25 insertions(+), 1 deletion(-)

fs/unicode/utf8-core.c +22 -1

index 2a878b739115d..90656b9980720 100644
@@ -6,6 +6,7 @@
 #include <linux/parser.h>
 #include <linux/errno.h>
 #include <linux/unicode.h>
+#include <linux/stringhash.h>
 
 #include "utf8n.h"

@@ -122,9 +123,29 @@ int utf8_casefold(const struct unicode_map *um, const struct qstr *str,
 	}
 	return -EINVAL;
 }
-
 EXPORT_SYMBOL(utf8_casefold);
 
+int utf8_casefold_hash(const struct unicode_map *um, const void *salt,
+		       struct qstr *str)
+{
+	const struct utf8data *data = utf8nfdicf(um->version);
+	struct utf8cursor cur;
+	int c;
+	unsigned long hash = init_name_hash(salt);
+
+	if (utf8ncursor(&cur, data, str->name, str->len) < 0)
+		return -EINVAL;
+
+	while ((c = utf8byte(&cur))) {
+		if (c < 0)
+			return c;
+		hash = partial_name_hash((unsigned char)c, hash);
+	}
+	str->hash = end_name_hash(hash);
+	return 0;
+}
+EXPORT_SYMBOL(utf8_casefold_hash);
+
 int utf8_normalize(const struct unicode_map *um, const struct qstr *str,
 		   unsigned char *dest, size_t dlen)
 {

include/linux/unicode.h +3 -0

index 990aa97d80496..74484d44c7554 100644
@@ -27,6 +27,9 @@ int utf8_normalize(const struct unicode_map *um, const struct qstr *str,
 int utf8_casefold(const struct unicode_map *um, const struct qstr *str,
 		  unsigned char *dest, size_t dlen);
 
+int utf8_casefold_hash(const struct unicode_map *um, const void *salt,
+		       struct qstr *str);
+
 struct unicode_map *utf8_load(const char *version);
 void utf8_unload(struct unicode_map *um);

Signature · 1 line

2.27.0.111.gc72c7da667-goog

Gabriel Krisman Bertazi

Reviewed-by on v9 1/4

2020-06-24 05:13 UTC #2 lore raw

Re: [PATCH v9 1/4] unicode: Add utf8_casefold_hash

Quoted 32 lines · Daniel Rosenberg

This adds a case insensitive hash function to allow taking the hash without needing to allocate a casefolded copy of the string.

Signed-off-by Daniel Rosenberg <[email protected]>

---
 fs/unicode/utf8-core.c  | 23 ++++++++++++++++++++++-
 include/linux/unicode.h |  3 +++
 2 files changed, 25 insertions(+), 1 deletion(-)

fs/unicode/utf8-core.c +5 -1

index 2a878b739115d..90656b9980720 100644
@@ -6,6 +6,7 @@
 #include <linux/parser.h>
 #include <linux/errno.h>
 #include <linux/unicode.h>
+#include <linux/stringhash.h>
 
 #include "utf8n.h"

@@ -122,9 +123,29 @@ int utf8_casefold(const struct unicode_map *um, const struct qstr *str,
 	}
 	return -EINVAL;
 }
-
 EXPORT_SYMBOL(utf8_casefold);
 
+int utf8_casefold_hash(const struct unicode_map *um, const void *salt,
+		       struct qstr *str)
+{
+	const struct utf8data *data = utf8nfdicf(um->version);

fs/unicode/utf8-core.c int utf8_casefold(const struct unicode_map *um, const struct qstr *str, · line 141

Quoted 10 lines

fs/unicode/utf8-core.c +10 -0

+	struct utf8cursor cur;
+	int c;
+	unsigned long hash = init_name_hash(salt);
+
+	if (utf8ncursor(&cur, data, str->name, str->len) < 0)
+		return -EINVAL;
+
+	while ((c = utf8byte(&cur))) {
+		if (c < 0)
+			return c;

Return -EINVAL here to match other unicode functions, since utf8byte will return -1 on a binary blob, which doesn't make sense for this.

Other than that, looks good to me.

Reviewed-by Gabriel Krisman Bertazi <[email protected]>

Quoted 23 lines

fs/unicode/utf8-core.c +7 -0

+		hash = partial_name_hash((unsigned char)c, hash);
+	}
+	str->hash = end_name_hash(hash);
+	return 0;
+}
+EXPORT_SYMBOL(utf8_casefold_hash);
+
 int utf8_normalize(const struct unicode_map *um, const struct qstr *str,
 		   unsigned char *dest, size_t dlen)
 {

include/linux/unicode.h +3 -0

index 990aa97d80496..74484d44c7554 100644
@@ -27,6 +27,9 @@ int utf8_normalize(const struct unicode_map *um, const struct qstr *str,
 int utf8_casefold(const struct unicode_map *um, const struct qstr *str,
 		  unsigned char *dest, size_t dlen);
 
+int utf8_casefold_hash(const struct unicode_map *um, const void *salt,
+		       struct qstr *str);
+
 struct unicode_map *utf8_load(const char *version);
 void utf8_unload(struct unicode_map *um);

Signature · 1 line

Gabriel Krisman Bertazi

Eric Biggers

maintainer

2020-06-24 05:37 UTC #3 lore raw

Re: [PATCH v9 1/4] unicode: Add utf8_casefold_hash

In reply to Daniel Rosenberg

Quoted 2 lines · Daniel Rosenberg

This adds a case insensitive hash function to allow taking the hash without needing to allocate a casefolded copy of the string.

It would be helpful to add a few more details in this commit message. Somewhat along the lines of: ->d_hash() for casefolding currently allocates memory, it needs to use GFP_ATOMIC due to ->d_hash() being called in rcu-walk mode, this is unreliable and inefficient, and this patch allows solving that problem by removing the need to allocate memory.

- Eric

Daniel Rosenberg

author

2020-06-24 04:34 UTC #4 lore raw

PATCH v9 2/4 fs: Add standard casefolding support

In reply to Daniel Rosenberg

This adds general supporting functions for filesystems that use utf8 casefolding. It provides standard dentry_operations and adds the necessary structures in struct super_block to allow this standardization.

Ext4 and F2fs will switch to these common implementations.

Signed-off-by Daniel Rosenberg <[email protected]>

---
 fs/libfs.c         | 101 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |  22 ++++++++++
 2 files changed, 123 insertions(+)

fs/libfs.c +101 -0

index 4d08edf19c782..f7345a5ed562f 100644
@@ -20,6 +20,8 @@
 #include <linux/fs_context.h>
 #include <linux/pseudo_fs.h>
 #include <linux/fsnotify.h>
+#include <linux/unicode.h>
+#include <linux/fscrypt.h>
 
 #include <linux/uaccess.h>

@@ -1363,3 +1365,102 @@ bool is_empty_dir_inode(struct inode *inode)
 	return (inode->i_fop == &empty_dir_operations) &&
 		(inode->i_op == &empty_dir_inode_operations);
 }
+
+#ifdef CONFIG_UNICODE
+/**
+ * needs_casefold - generic helper to determine if a filename should be casefolded
+ * @dir: Parent directory
+ *
+ * Generic helper for filesystems to use to determine if the name of a dentry
+ * should be casefolded. It does not make sense to casefold the no-key token of
+ * an encrypted filename.
+ *
+ * Return: if names will need casefolding
+ */
+bool needs_casefold(const struct inode *dir)
+{
+	return IS_CASEFOLDED(dir) && dir->i_sb->s_encoding &&
+			(!IS_ENCRYPTED(dir) || fscrypt_has_encryption_key(dir));
+}
+EXPORT_SYMBOL(needs_casefold);
+
+/**
+ * generic_ci_d_compare - generic d_compare implementation for casefolding filesystems
+ * @dentry:	dentry whose name we are checking against
+ * @len:	len of name of dentry
+ * @str:	str pointer to name of dentry
+ * @name:	Name to compare against
+ *
+ * Return: 0 if names match, 1 if mismatch, or -ERRNO
+ */
+int generic_ci_d_compare(const struct dentry *dentry, unsigned int len,
+			  const char *str, const struct qstr *name)
+{
+	const struct dentry *parent = READ_ONCE(dentry->d_parent);
+	const struct inode *inode = READ_ONCE(parent->d_inode);
+	const struct super_block *sb = dentry->d_sb;
+	const struct unicode_map *um = sb->s_encoding;
+	struct qstr qstr = QSTR_INIT(str, len);
+	char strbuf[DNAME_INLINE_LEN];
+	int ret;
+
+	if (!inode || !needs_casefold(inode))
+		goto fallback;
+	/*
+	 * If the dentry name is stored in-line, then it may be concurrently
+	 * modified by a rename.  If this happens, the VFS will eventually retry
+	 * the lookup, so it doesn't matter what ->d_compare() returns.
+	 * However, it's unsafe to call utf8_strncasecmp() with an unstable
+	 * string.  Therefore, we have to copy the name into a temporary buffer.
+	 */
+	if (len <= DNAME_INLINE_LEN - 1) {
+		memcpy(strbuf, str, len);
+		strbuf[len] = 0;
+		qstr.name = strbuf;
+		/* prevent compiler from optimizing out the temporary buffer */
+		barrier();
+	}
+	ret = utf8_strncasecmp(um, name, &qstr);
+	if (ret >= 0)
+		return ret;
+
+	if (sb_has_enc_strict_mode(sb))
+		return -EINVAL;
+fallback:
+	if (len != name->len)
+		return 1;
+	return !!memcmp(str, name->name, len);
+}
+EXPORT_SYMBOL(generic_ci_d_compare);
+
+/**
+ * generic_ci_d_hash - generic d_hash implementation for casefolding filesystems
+ * @dentry:	dentry whose name we are hashing
+ * @str:	qstr of name whose hash we should fill in
+ *
+ * Return: 0 if hash was successful, or -ERRNO
+ */
+int generic_ci_d_hash(const struct dentry *dentry, struct qstr *str)
+{
+	const struct inode *inode = READ_ONCE(dentry->d_inode);
+	struct super_block *sb = dentry->d_sb;
+	const struct unicode_map *um = sb->s_encoding;
+	int ret = 0;
+
+	if (!inode || !needs_casefold(inode))
+		return 0;
+
+	ret = utf8_casefold_hash(um, dentry, str);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+err:
+	if (sb_has_enc_strict_mode(sb))
+		ret = -EINVAL;
+	else
+		ret = 0;
+	return ret;
+}
+EXPORT_SYMBOL(generic_ci_d_hash);
+#endif

include/linux/fs.h +22 -0

index 3f881a892ea74..261904e06873b 100644
@@ -1392,6 +1392,12 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_ACTIVE	(1<<30)
 #define SB_NOUSER	(1<<31)
 
+/* These flags relate to encoding and casefolding */
+#define SB_ENC_STRICT_MODE_FL	(1 << 0)
+
+#define sb_has_enc_strict_mode(sb) \
+	(sb->s_encoding_flags & SB_ENC_STRICT_MODE_FL)
+
 /*
  *	Umount options
  */

@@ -1461,6 +1467,10 @@ struct super_block {
 #endif
 #ifdef CONFIG_FS_VERITY
 	const struct fsverity_operations *s_vop;
+#endif
+#ifdef CONFIG_UNICODE
+	struct unicode_map *s_encoding;
+	__u16 s_encoding_flags;
 #endif
 	struct hlist_bl_head	s_roots;	/* alternate root dentries for NFS */
 	struct list_head	s_mounts;	/* list of mounts; _not_ for fs use */

@@ -3385,6 +3395,18 @@ extern int generic_file_fsync(struct file *, loff_t, loff_t, int);
 
 extern int generic_check_addressable(unsigned, u64);
 
+#ifdef CONFIG_UNICODE
+extern int generic_ci_d_hash(const struct dentry *dentry, struct qstr *str);
+extern int generic_ci_d_compare(const struct dentry *dentry, unsigned int len,
+				const char *str, const struct qstr *name);
+extern bool needs_casefold(const struct inode *dir);
+#else
+static inline bool needs_casefold(const struct inode *dir)
+{
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_MIGRATION
 extern int buffer_migrate_page(struct address_space *,
 				struct page *, struct page *,

Signature · 1 line

2.27.0.111.gc72c7da667-goog

Gabriel Krisman Bertazi

2020-06-24 05:33 UTC #5 lore raw

Re: [PATCH v9 2/4] fs: Add standard casefolding support

Quoted 116 lines · Daniel Rosenberg

Ext4 and F2fs will switch to these common implementations.

Signed-off-by Daniel Rosenberg <[email protected]>

---
 fs/libfs.c         | 101 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |  22 ++++++++++
 2 files changed, 123 insertions(+)

fs/libfs.c +89 -0

index 4d08edf19c782..f7345a5ed562f 100644
@@ -20,6 +20,8 @@
 #include <linux/fs_context.h>
 #include <linux/pseudo_fs.h>
 #include <linux/fsnotify.h>
+#include <linux/unicode.h>
+#include <linux/fscrypt.h>
 
 #include <linux/uaccess.h>

@@ -1363,3 +1365,102 @@ bool is_empty_dir_inode(struct inode *inode)
 	return (inode->i_fop == &empty_dir_operations) &&
 		(inode->i_op == &empty_dir_inode_operations);
 }
+
+#ifdef CONFIG_UNICODE
+/**
+ * needs_casefold - generic helper to determine if a filename should be casefolded
+ * @dir: Parent directory
+ *
+ * Generic helper for filesystems to use to determine if the name of a dentry
+ * should be casefolded. It does not make sense to casefold the no-key token of
+ * an encrypted filename.
+ *
+ * Return: if names will need casefolding
+ */
+bool needs_casefold(const struct inode *dir)
+{
+	return IS_CASEFOLDED(dir) && dir->i_sb->s_encoding &&
+			(!IS_ENCRYPTED(dir) || fscrypt_has_encryption_key(dir));
+}
+EXPORT_SYMBOL(needs_casefold);
+
+/**
+ * generic_ci_d_compare - generic d_compare implementation for casefolding filesystems
+ * @dentry:	dentry whose name we are checking against
+ * @len:	len of name of dentry
+ * @str:	str pointer to name of dentry
+ * @name:	Name to compare against
+ *
+ * Return: 0 if names match, 1 if mismatch, or -ERRNO
+ */
+int generic_ci_d_compare(const struct dentry *dentry, unsigned int len,
+			  const char *str, const struct qstr *name)
+{
+	const struct dentry *parent = READ_ONCE(dentry->d_parent);
+	const struct inode *inode = READ_ONCE(parent->d_inode);
+	const struct super_block *sb = dentry->d_sb;
+	const struct unicode_map *um = sb->s_encoding;
+	struct qstr qstr = QSTR_INIT(str, len);
+	char strbuf[DNAME_INLINE_LEN];
+	int ret;
+
+	if (!inode || !needs_casefold(inode))
+		goto fallback;
+	/*
+	 * If the dentry name is stored in-line, then it may be concurrently
+	 * modified by a rename.  If this happens, the VFS will eventually retry
+	 * the lookup, so it doesn't matter what ->d_compare() returns.
+	 * However, it's unsafe to call utf8_strncasecmp() with an unstable
+	 * string.  Therefore, we have to copy the name into a temporary buffer.
+	 */
+	if (len <= DNAME_INLINE_LEN - 1) {
+		memcpy(strbuf, str, len);
+		strbuf[len] = 0;
+		qstr.name = strbuf;
+		/* prevent compiler from optimizing out the temporary buffer */
+		barrier();
+	}
+	ret = utf8_strncasecmp(um, name, &qstr);
+	if (ret >= 0)
+		return ret;
+
+	if (sb_has_enc_strict_mode(sb))
+		return -EINVAL;
+fallback:
+	if (len != name->len)
+		return 1;
+	return !!memcmp(str, name->name, len);
+}
+EXPORT_SYMBOL(generic_ci_d_compare);
+
+/**
+ * generic_ci_d_hash - generic d_hash implementation for casefolding filesystems
+ * @dentry:	dentry whose name we are hashing
+ * @str:	qstr of name whose hash we should fill in
+ *
+ * Return: 0 if hash was successful, or -ERRNO
+ */
+int generic_ci_d_hash(const struct dentry *dentry, struct qstr *str)
+{
+	const struct inode *inode = READ_ONCE(dentry->d_inode);
+	struct super_block *sb = dentry->d_sb;
+	const struct unicode_map *um = sb->s_encoding;
+	int ret = 0;
+
+	if (!inode || !needs_casefold(inode))
+		return 0;
+
+	ret = utf8_casefold_hash(um, dentry, str);
+	if (ret < 0)

fs/libfs.c bool is_empty_dir_inode(struct inode *inode) · line 1464

Quoted 10 lines

fs/libfs.c +10 -0

+		goto err;
+
+	return 0;
+err:
+	if (sb_has_enc_strict_mode(sb))
+		ret = -EINVAL;
+	else
+		ret = 0;
+	return ret;
+}

Maybe drop the err label and simplify:

  ret = utf8_casefold_hash(um, dentry, str);
  if (ret < 0 && sb_has_enc_strict_mode(sb))
     return -EINVAL;
  return 0;

Quoted 49 lines

fs/libfs.c +2 -0

+EXPORT_SYMBOL(generic_ci_d_hash);
+#endif

include/linux/fs.h +22 -0

index 3f881a892ea74..261904e06873b 100644
@@ -1392,6 +1392,12 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_ACTIVE	(1<<30)
 #define SB_NOUSER	(1<<31)
 
+/* These flags relate to encoding and casefolding */
+#define SB_ENC_STRICT_MODE_FL	(1 << 0)
+
+#define sb_has_enc_strict_mode(sb) \
+	(sb->s_encoding_flags & SB_ENC_STRICT_MODE_FL)
+
 /*
  *	Umount options
  */

@@ -1461,6 +1467,10 @@ struct super_block {
 #endif
 #ifdef CONFIG_FS_VERITY
 	const struct fsverity_operations *s_vop;
+#endif
+#ifdef CONFIG_UNICODE
+	struct unicode_map *s_encoding;
+	__u16 s_encoding_flags;
 #endif
 	struct hlist_bl_head	s_roots;	/* alternate root dentries for NFS */
 	struct list_head	s_mounts;	/* list of mounts; _not_ for fs use */

@@ -3385,6 +3395,18 @@ extern int generic_file_fsync(struct file *, loff_t, loff_t, int);
 
 extern int generic_check_addressable(unsigned, u64);
 
+#ifdef CONFIG_UNICODE
+extern int generic_ci_d_hash(const struct dentry *dentry, struct qstr *str);
+extern int generic_ci_d_compare(const struct dentry *dentry, unsigned int len,
+				const char *str, const struct qstr *name);
+extern bool needs_casefold(const struct inode *dir);
+#else
+static inline bool needs_casefold(const struct inode *dir)
+{
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_MIGRATION
 extern int buffer_migrate_page(struct address_space *,
 				struct page *, struct page *,

Signature · 1 line

Gabriel Krisman Bertazi

Eric Biggers

maintainer

2020-06-24 05:42 UTC #6 lore raw

Re: [PATCH v9 2/4] fs: Add standard casefolding support

In reply to Daniel Rosenberg

Quoted 5 lines · Daniel Rosenberg

This adds general supporting functions for filesystems that use utf8 casefolding. It provides standard dentry_operations and adds the necessary structures in struct super_block to allow this standardization.
Ext4 and F2fs will switch to these common implementations.

It would be helpful to explicitly call out anything that's "new" in this commit, i.e. anything that isn't simply moving code into the libfs with no behavior changes. There's the change of ->d_hash() to use utf8_casefold_hash() instead of allocating memory; that's not present in the ext4 and f2fs versions.

There's also the change of needs_casefold() to be aware of encrypt+casefold. (Maybe that small change would better belong in a later patchset that actually introduces encrypt+casefold support?)

Anything else?

Quoted 7 lines

+/**
+ * generic_ci_d_hash - generic d_hash implementation for casefolding filesystems
+ * @dentry: dentry whose name we are hashing
+ * @str: qstr of name whose hash we should fill in
+ *
+ * Return: 0 if hash was successful, or -ERRNO
+ */

It also returns 0 if the hashing was not done because it wants to fallback to the standard hashing.

include/linux/fs.h extern int generic_file_fsync(struct file *, loff_t, loff_t, int); · line 3407

Quoted 4 lines

include/linux/fs.h +4 -0

+static inline bool needs_casefold(const struct inode *dir)
+{
+	return 0;
+}

The return type is bool, so it should 'return false', not 'return 0'.

- Eric

Eric Biggers

maintainer

2020-06-24 05:57 UTC #7 lore raw

Re: [PATCH v9 2/4] fs: Add standard casefolding support

In reply to Daniel Rosenberg

Quoted 37 lines · Daniel Rosenberg

Ext4 and F2fs will switch to these common implementations.

Signed-off-by Daniel Rosenberg <[email protected]>

---
 fs/libfs.c         | 101 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |  22 ++++++++++
 2 files changed, 123 insertions(+)

fs/libfs.c +10 -0

index 4d08edf19c782..f7345a5ed562f 100644
@@ -20,6 +20,8 @@
 #include <linux/fs_context.h>
 #include <linux/pseudo_fs.h>
 #include <linux/fsnotify.h>
+#include <linux/unicode.h>
+#include <linux/fscrypt.h>
 
 #include <linux/uaccess.h>

@@ -1363,3 +1365,102 @@ bool is_empty_dir_inode(struct inode *inode)
 	return (inode->i_fop == &empty_dir_operations) &&
 		(inode->i_op == &empty_dir_inode_operations);
 }
+
+#ifdef CONFIG_UNICODE
+/**
+ * needs_casefold - generic helper to determine if a filename should be casefolded
+ * @dir: Parent directory
+ *
+ * Generic helper for filesystems to use to determine if the name of a dentry
+ * should be casefolded. It does not make sense to casefold the no-key token of

fs/libfs.c bool is_empty_dir_inode(struct inode *inode) · line 1385

Quoted 10 lines

fs/libfs.c +10 -0

+ * an encrypted filename.
+ *
+ * Return: if names will need casefolding
+ */
+bool needs_casefold(const struct inode *dir)
+{
+	return IS_CASEFOLDED(dir) && dir->i_sb->s_encoding &&
+			(!IS_ENCRYPTED(dir) || fscrypt_has_encryption_key(dir));
+}
+EXPORT_SYMBOL(needs_casefold);

Note that the '!IS_ENCRYPTED(dir) || fscrypt_has_encryption_key(dir)' check can be racy, because a process can be looking up a no-key token in a directory while concurrently another process initializes the directory's ->i_crypt_info, causing fscrypt_has_encryption_key(dir) to suddenly start returning true.

In my rework of filename handling in f2fs, I actually ended up removing all calls to needs_casefold(), thus avoiding this race. f2fs now decides whether the name is going to need casefolding early on, in __f2fs_setup_filename(), where it knows in a race-free way whether the filename is a no-key token or not.

Perhaps ext4 should work the same way? It did look like there would be some extra complexity due to how the ext4 directory hashing works in comparison to f2fs's, but I haven't had a chance to properly investigate it.

- Eric

Daniel Rosenberg

author

2020-07-03 01:01 UTC #8 lore raw

Re: [PATCH v9 2/4] fs: Add standard casefolding support

Quoted 11 lines · Eric Biggers

Note that the '!IS_ENCRYPTED(dir) || fscrypt_has_encryption_key(dir)' check can be racy, because a process can be looking up a no-key token in a directory while concurrently another process initializes the directory's ->i_crypt_info, causing fscrypt_has_encryption_key(dir) to suddenly start returning true.
In my rework of filename handling in f2fs, I actually ended up removing all calls to needs_casefold(), thus avoiding this race. f2fs now decides whether the name is going to need casefolding early on, in __f2fs_setup_filename(), where it knows in a race-free way whether the filename is a no-key token or not.

Quoted 5 lines

Perhaps ext4 should work the same way? It did look like there would be some extra complexity due to how the ext4 directory hashing works in comparison to f2fs's, but I haven't had a chance to properly investigate it.
- Eric

Hm. I think I should be able to just check for DCACHE_ENCRYPTED_NAME in the dentry here, right? I'm just trying to avoid casefolding the no-key token, and that flag should indicate that.
I'll see if I can rework the ext4 patches to not need needs_casefold as well, since then there'd be no need to export it. -Daniel

Eric Biggers

maintainer

2020-07-03 19:20 UTC #9 lore raw

Re: [PATCH v9 2/4] fs: Add standard casefolding support

Quoted 16 lines · Daniel Rosenberg

Quoted 15 lines · Eric Biggers
Note that the '!IS_ENCRYPTED(dir) || fscrypt_has_encryption_key(dir)' check can be racy, because a process can be looking up a no-key token in a directory while concurrently another process initializes the directory's ->i_crypt_info, causing fscrypt_has_encryption_key(dir) to suddenly start returning true.
In my rework of filename handling in f2fs, I actually ended up removing all calls to needs_casefold(), thus avoiding this race. f2fs now decides whether the name is going to need casefolding early on, in __f2fs_setup_filename(), where it knows in a race-free way whether the filename is a no-key token or not.
Perhaps ext4 should work the same way? It did look like there would be some extra complexity due to how the ext4 directory hashing works in comparison to f2fs's, but I haven't had a chance to properly investigate it.

Quoted 5 lines

Quoted 1 line
- Eric
Hm. I think I should be able to just check for DCACHE_ENCRYPTED_NAME in the dentry here, right? I'm just trying to avoid casefolding the no-key token, and that flag should indicate that.

Ideally yes, but currently the 'struct dentry' isn't always available. See how fscrypt_setup_filename(), f2fs_setup_filename(), f2fs_find_entry(), ext4_find_entry(), etc. take a 'struct qstr', not a 'struct dentry'.

At some point we should fix that by passing down the dentry whenever it's available, so that we reliably know whether the name is a no-key name or not.

So even my new f2fs code is still racy. But it at least handles each filename in a consistent way within each directory operation. In comparison, your proposed ext4 code can treat a filename as a no-key name while matching one dir_entry and then as a regular filename while matching the next. I think the f2fs way is more on the right track, both correctness-wise and efficiency-wise.

- Eric

Daniel Rosenberg

author

2020-06-24 04:34 UTC #10 lore raw

PATCH v9 3/4 f2fs: Use generic casefolding support

In reply to Daniel Rosenberg

This switches f2fs over to the generic support provided in commit 5f829feca774 ("fs: Add standard casefolding support")

Signed-off-by Daniel Rosenberg <[email protected]>

---
 fs/f2fs/dir.c           | 84 +++++------------------------------------
 fs/f2fs/f2fs.h          |  4 --
 fs/f2fs/super.c         | 10 ++---
 fs/f2fs/sysfs.c         | 10 +++--
 include/linux/f2fs_fs.h |  3 --
 5 files changed, 21 insertions(+), 90 deletions(-)

fs/f2fs/dir.c +10 ~~-74~~

index d35976785e8c5..b59c2673daa09 100644
@@ -75,21 +75,22 @@ int f2fs_init_casefolded_name(const struct inode *dir,
 			      struct f2fs_filename *fname)
 {
 #ifdef CONFIG_UNICODE
-	struct f2fs_sb_info *sbi = F2FS_SB(dir->i_sb);
+	struct super_block *sb = dir->i_sb;
+	struct f2fs_sb_info *sbi = F2FS_SB(sb);
 
 	if (IS_CASEFOLDED(dir)) {
 		fname->cf_name.name = f2fs_kmalloc(sbi, F2FS_NAME_LEN,
 						   GFP_NOFS);
 		if (!fname->cf_name.name)
 			return -ENOMEM;
-		fname->cf_name.len = utf8_casefold(sbi->s_encoding,
+		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) {
 			kfree(fname->cf_name.name);
 			fname->cf_name.name = NULL;
-			if (f2fs_has_strict_mode(sbi))
+			if (sb_has_enc_strict_mode(sb))
 				return -EINVAL;
 			/* fall back to treating name as opaque byte sequence */
 		}

@@ -215,8 +216,9 @@ static struct f2fs_dir_entry *find_in_block(struct inode *dir,
 static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
 			       const u8 *de_name, u32 de_name_len)
 {
-	const struct f2fs_sb_info *sbi = F2FS_SB(dir->i_sb);
-	const struct unicode_map *um = sbi->s_encoding;
+	const struct super_block *sb = dir->i_sb;
+	const struct f2fs_sb_info *sbi = F2FS_SB(sb);
+	const struct unicode_map *um = sb->s_encoding;
 	struct qstr entry = QSTR_INIT(de_name, de_name_len);
 	int res;

@@ -226,7 +228,7 @@ static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
 		 * In strict mode, ignore invalid names.  In non-strict mode,
 		 * fall back to treating them as opaque byte sequences.
 		 */
-		if (f2fs_has_strict_mode(sbi) || name->len != entry.len)
+		if (sb_has_enc_strict_mode(sb) || name->len != entry.len)
 			return false;
 		return !memcmp(name->name, entry.name, name->len);
 	}

@@ -1107,75 +1109,9 @@ const struct file_operations f2fs_dir_operations = {
 };
 
 #ifdef CONFIG_UNICODE
-static int f2fs_d_compare(const struct dentry *dentry, unsigned int len,
-			  const char *str, const struct qstr *name)
-{
-	const struct dentry *parent = READ_ONCE(dentry->d_parent);
-	const struct inode *dir = READ_ONCE(parent->d_inode);
-	const struct f2fs_sb_info *sbi = F2FS_SB(dentry->d_sb);
-	struct qstr entry = QSTR_INIT(str, len);
-	char strbuf[DNAME_INLINE_LEN];
-	int res;
-
-	if (!dir || !IS_CASEFOLDED(dir))
-		goto fallback;
-
-	/*
-	 * If the dentry name is stored in-line, then it may be concurrently
-	 * modified by a rename.  If this happens, the VFS will eventually retry
-	 * the lookup, so it doesn't matter what ->d_compare() returns.
-	 * However, it's unsafe to call utf8_strncasecmp() with an unstable
-	 * string.  Therefore, we have to copy the name into a temporary buffer.
-	 */
-	if (len <= DNAME_INLINE_LEN - 1) {
-		memcpy(strbuf, str, len);
-		strbuf[len] = 0;
-		entry.name = strbuf;
-		/* prevent compiler from optimizing out the temporary buffer */
-		barrier();
-	}
-
-	res = utf8_strncasecmp(sbi->s_encoding, name, &entry);
-	if (res >= 0)
-		return res;
-
-	if (f2fs_has_strict_mode(sbi))
-		return -EINVAL;
-fallback:
-	if (len != name->len)
-		return 1;
-	return !!memcmp(str, name->name, len);
-}
-
-static int f2fs_d_hash(const struct dentry *dentry, struct qstr *str)
-{
-	struct f2fs_sb_info *sbi = F2FS_SB(dentry->d_sb);
-	const struct unicode_map *um = sbi->s_encoding;
-	const struct inode *inode = READ_ONCE(dentry->d_inode);
-	unsigned char *norm;
-	int len, ret = 0;
-
-	if (!inode || !IS_CASEFOLDED(inode))
-		return 0;
-
-	norm = f2fs_kmalloc(sbi, PATH_MAX, GFP_ATOMIC);
-	if (!norm)
-		return -ENOMEM;
-
-	len = utf8_casefold(um, str, norm, PATH_MAX);
-	if (len < 0) {
-		if (f2fs_has_strict_mode(sbi))
-			ret = -EINVAL;
-		goto out;
-	}
-	str->hash = full_name_hash(dentry, norm, len);
-out:
-	kvfree(norm);
-	return ret;
-}
 
 const struct dentry_operations f2fs_dentry_ops = {
-	.d_hash = f2fs_d_hash,
-	.d_compare = f2fs_d_compare,
+	.d_hash = generic_ci_d_hash,
+	.d_compare = generic_ci_d_compare,
 };
 #endif

fs/f2fs/f2fs.h +0 -4

index b35a50f4953c5..d11ffe26bfded 100644
@@ -1402,10 +1402,6 @@ struct f2fs_sb_info {
 	int valid_super_block;			/* valid super block no */
 	unsigned long s_flag;				/* flags for sbi */
 	struct mutex writepages;		/* mutex for writepages() */
-#ifdef CONFIG_UNICODE
-	struct unicode_map *s_encoding;
-	__u16 s_encoding_flags;
-#endif
 
 #ifdef CONFIG_BLK_DEV_ZONED
 	unsigned int blocks_per_blkz;		/* F2FS blocks per zone */

fs/f2fs/super.c +5 -5

index 20e56b0fa46a9..cca7a83ffa089 100644
@@ -1257,7 +1257,7 @@ static void f2fs_put_super(struct super_block *sb)
 	for (i = 0; i < NR_PAGE_TYPE; i++)
 		kvfree(sbi->write_io[i]);
 #ifdef CONFIG_UNICODE
-	utf8_unload(sbi->s_encoding);
+	utf8_unload(sb->s_encoding);
 #endif
 	kvfree(sbi);
 }

@@ -3278,7 +3278,7 @@ static int f2fs_scan_devices(struct f2fs_sb_info *sbi)
 static int f2fs_setup_casefold(struct f2fs_sb_info *sbi)
 {
 #ifdef CONFIG_UNICODE
-	if (f2fs_sb_has_casefold(sbi) && !sbi->s_encoding) {
+	if (f2fs_sb_has_casefold(sbi) && !sbi->sb->s_encoding) {
 		const struct f2fs_sb_encodings *encoding_info;
 		struct unicode_map *encoding;
 		__u16 encoding_flags;

@@ -3309,8 +3309,8 @@ static int f2fs_setup_casefold(struct f2fs_sb_info *sbi)
 			 "%s-%s with flags 0x%hx", encoding_info->name,
 			 encoding_info->version?:"\b", encoding_flags);
 
-		sbi->s_encoding = encoding;
-		sbi->s_encoding_flags = encoding_flags;
+		sbi->sb->s_encoding = encoding;
+		sbi->sb->s_encoding_flags = encoding_flags;
 		sbi->sb->s_d_op = &f2fs_dentry_ops;
 	}
 #else

@@ -3806,7 +3806,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		kvfree(sbi->write_io[i]);
 
 #ifdef CONFIG_UNICODE
-	utf8_unload(sbi->s_encoding);
+	utf8_unload(sb->s_encoding);
 #endif
 free_options:
 #ifdef CONFIG_QUOTA

fs/f2fs/sysfs.c +6 -4

index e877c59b9fdb4..8bee99ab3978e 100644
@@ -176,12 +176,14 @@ static ssize_t encoding_show(struct f2fs_attr *a,
 		struct f2fs_sb_info *sbi, char *buf)
 {
 #ifdef CONFIG_UNICODE
+	struct super_block *sb = sbi->sb;
+
 	if (f2fs_sb_has_casefold(sbi))
 		return snprintf(buf, PAGE_SIZE, "%s (%d.%d.%d)\n",
-			sbi->s_encoding->charset,
-			(sbi->s_encoding->version >> 16) & 0xff,
-			(sbi->s_encoding->version >> 8) & 0xff,
-			sbi->s_encoding->version & 0xff);
+			sb->s_encoding->charset,
+			(sb->s_encoding->version >> 16) & 0xff,
+			(sb->s_encoding->version >> 8) & 0xff,
+			sb->s_encoding->version & 0xff);
 #endif
 	return sprintf(buf, "(none)");
 }

include/linux/f2fs_fs.h +0 -3

index 3c383ddd92ddd..a5dbb57a687fb 100644
@@ -38,9 +38,6 @@
 #define F2FS_MAX_QUOTAS		3
 
 #define F2FS_ENC_UTF8_12_1	1
-#define F2FS_ENC_STRICT_MODE_FL	(1 << 0)
-#define f2fs_has_strict_mode(sbi) \
-	(sbi->s_encoding_flags & F2FS_ENC_STRICT_MODE_FL)
 
 #define F2FS_IO_SIZE(sbi)	(1 << F2FS_OPTION(sbi).write_io_size_bits) /* Blocks */
 #define F2FS_IO_SIZE_KB(sbi)	(1 << (F2FS_OPTION(sbi).write_io_size_bits + 2)) /* KB */

Signature · 1 line

2.27.0.111.gc72c7da667-goog

kernel test robot

build bot

2020-06-24 06:34 UTC #11 lore raw

Re: [PATCH v9 3/4] f2fs: Use generic casefolding support

Thank you for the patch! Perhaps something to improve:

Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.8-rc2 next-20200623]
[cannot apply to ext4/dev f2fs/dev-test tytso-fscrypt/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Rosenberg/Prepare-for-upcoming-Casefolding-Encryption-patches/20200624-123657
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3e08a95294a4fb3702bb3d35ed08028433c37fe6
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

fs/f2fs/dir.c: In function 'f2fs_match_ci_name':

>> fs/f2fs/dir.c:220:43: warning: passing argument 1 of 'F2FS_SB' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     220 |  const struct f2fs_sb_info *sbi = F2FS_SB(sb);
         |                                           ^~
   In file included from fs/f2fs/dir.c:12:
   fs/f2fs/f2fs.h:1734:64: note: expected 'struct super_block *' but argument is of type 'const struct super_block *'
    1734 | static inline struct f2fs_sb_info *F2FS_SB(struct super_block *sb)
         |                                            ~~~~~~~~~~~~~~~~~~~~^~
   fs/f2fs/dir.c:220:29: warning: unused variable 'sbi' [-Wunused-variable]
     220 |  const struct f2fs_sb_info *sbi = F2FS_SB(sb);
         |                             ^~~

vim +220 fs/f2fs/dir.c

210
211 #ifdef CONFIG_UNICODE
212 /*
213 * Test whether a case-insensitive directory entry matches the filename
214 * being searched for.
215 */
216 static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
217 const u8 *de_name, u32 de_name_len)
218 {
219 const struct super_block *sb = dir->i_sb;

 > 220		const struct f2fs_sb_info *sbi = F2FS_SB(sb);

221 const struct unicode_map *um = sb->s_encoding;
222 struct qstr entry = QSTR_INIT(de_name, de_name_len);
223 int res;
224
225 res = utf8_strncasecmp_folded(um, name, &entry);
226 if (res < 0) {
227 /*
228 * In strict mode, ignore invalid names. In non-strict mode,
229 * fall back to treating them as opaque byte sequences.
230 */
231 if (sb_has_enc_strict_mode(sb) || name->len != entry.len)
232 return false;
233 return !memcmp(name->name, entry.name, name->len);
234 }
235 return res == 0;
236 }
237 #endif /* CONFIG_UNICODE */
238

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

kernel test robot

build bot

2020-06-24 09:55 UTC #12 lore raw

Re: [PATCH v9 3/4] f2fs: Use generic casefolding support

In reply to Daniel Rosenberg

Thank you for the patch! Yet something to improve:

Hi Daniel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.8-rc2 next-20200624]
[cannot apply to ext4/dev f2fs/dev-test]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Rosenberg/Prepare-for-upcoming-Casefolding-Encryption-patches/20200624-123657
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3e08a95294a4fb3702bb3d35ed08028433c37fe6
config: x86_64-randconfig-a013-20200624 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1d4c87335d5236ea1f35937e1014980ba961ae34)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> fs/f2fs/dir.c:220:43: error: passing 'const struct super_block *' to parameter of type 'struct super_block *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
           const struct f2fs_sb_info *sbi = F2FS_SB(sb);
                                                    ^~
   fs/f2fs/f2fs.h:1734:64: note: passing argument to parameter 'sb' here
   static inline struct f2fs_sb_info *F2FS_SB(struct super_block *sb)
                                                                  ^
   fs/f2fs/dir.c:220:29: warning: unused variable 'sbi' [-Wunused-variable]
           const struct f2fs_sb_info *sbi = F2FS_SB(sb);
                                      ^

1 warning and 1 error generated.

vim +220 fs/f2fs/dir.c

 > 220		const struct f2fs_sb_info *sbi = F2FS_SB(sb);

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

kernel test robot

build bot

2020-06-24 10:01 UTC #13 lore raw

Re: [PATCH v9 3/4] f2fs: Use generic casefolding support

In reply to Daniel Rosenberg

Thank you for the patch! Perhaps something to improve:

Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.8-rc2 next-20200624]
[cannot apply to ext4/dev f2fs/dev-test]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Rosenberg/Prepare-for-upcoming-Casefolding-Encryption-patches/20200624-123657
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3e08a95294a4fb3702bb3d35ed08028433c37fe6
config: x86_64-randconfig-s021-20200624 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=x86_64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

sparse warnings: (new ones prefixed by >>)

>> fs/f2fs/dir.c:220:50: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected struct super_block *sb @@     got struct super_block const *sb @@
>> fs/f2fs/dir.c:220:50: sparse:     expected struct super_block *sb
>> fs/f2fs/dir.c:220:50: sparse:     got struct super_block const *sb

vim +220 fs/f2fs/dir.c

 > 220		const struct f2fs_sb_info *sbi = F2FS_SB(sb);

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Daniel Rosenberg

author

2020-06-24 04:34 UTC #14 lore raw

PATCH v9 4/4 ext4: Use generic casefolding support

In reply to Daniel Rosenberg

This switches ext4 over to the generic support provided in commit 5f829feca774 ("fs: Add standard casefolding support")

Signed-off-by Daniel Rosenberg <[email protected]>

---
 fs/ext4/dir.c   | 64 ++-----------------------------------------------
 fs/ext4/ext4.h  | 12 ----------
 fs/ext4/hash.c  |  2 +-
 fs/ext4/namei.c | 20 +++++++---------
 fs/ext4/super.c | 12 +++++-----
 5 files changed, 17 insertions(+), 93 deletions(-)

fs/ext4/dir.c +2 ~~-62~~

index 1d82336b1cd45..b437120f0b3f5 100644
@@ -669,68 +669,8 @@ const struct file_operations ext4_dir_operations = {
 };
 
 #ifdef CONFIG_UNICODE
-static int ext4_d_compare(const struct dentry *dentry, unsigned int len,
-			  const char *str, const struct qstr *name)
-{
-	struct qstr qstr = {.name = str, .len = len };
-	const struct dentry *parent = READ_ONCE(dentry->d_parent);
-	const struct inode *inode = READ_ONCE(parent->d_inode);
-	char strbuf[DNAME_INLINE_LEN];
-
-	if (!inode || !IS_CASEFOLDED(inode) ||
-	    !EXT4_SB(inode->i_sb)->s_encoding) {
-		if (len != name->len)
-			return -1;
-		return memcmp(str, name->name, len);
-	}
-
-	/*
-	 * If the dentry name is stored in-line, then it may be concurrently
-	 * modified by a rename.  If this happens, the VFS will eventually retry
-	 * the lookup, so it doesn't matter what ->d_compare() returns.
-	 * However, it's unsafe to call utf8_strncasecmp() with an unstable
-	 * string.  Therefore, we have to copy the name into a temporary buffer.
-	 */
-	if (len <= DNAME_INLINE_LEN - 1) {
-		memcpy(strbuf, str, len);
-		strbuf[len] = 0;
-		qstr.name = strbuf;
-		/* prevent compiler from optimizing out the temporary buffer */
-		barrier();
-	}
-
-	return ext4_ci_compare(inode, name, &qstr, false);
-}
-
-static int ext4_d_hash(const struct dentry *dentry, struct qstr *str)
-{
-	const struct ext4_sb_info *sbi = EXT4_SB(dentry->d_sb);
-	const struct unicode_map *um = sbi->s_encoding;
-	const struct inode *inode = READ_ONCE(dentry->d_inode);
-	unsigned char *norm;
-	int len, ret = 0;
-
-	if (!inode || !IS_CASEFOLDED(inode) || !um)
-		return 0;
-
-	norm = kmalloc(PATH_MAX, GFP_ATOMIC);
-	if (!norm)
-		return -ENOMEM;
-
-	len = utf8_casefold(um, str, norm, PATH_MAX);
-	if (len < 0) {
-		if (ext4_has_strict_mode(sbi))
-			ret = -EINVAL;
-		goto out;
-	}
-	str->hash = full_name_hash(dentry, norm, len);
-out:
-	kfree(norm);
-	return ret;
-}
-
 const struct dentry_operations ext4_dentry_ops = {
-	.d_hash = ext4_d_hash,
-	.d_compare = ext4_d_compare,
+	.d_hash = generic_ci_d_hash,
+	.d_compare = generic_ci_d_compare,
 };
 #endif

fs/ext4/ext4.h +0 ~~-12~~

index 42f5060f3cdf1..5cd8be24a4fd9 100644
@@ -1393,14 +1393,6 @@ struct ext4_super_block {
 
 #define EXT4_ENC_UTF8_12_1	1
 
-/*
- * Flags for ext4_sb_info.s_encoding_flags.
- */
-#define EXT4_ENC_STRICT_MODE_FL	(1 << 0)
-
-#define ext4_has_strict_mode(sbi) \
-	(sbi->s_encoding_flags & EXT4_ENC_STRICT_MODE_FL)
-
 /*
  * fourth extended-fs super-block data in memory
  */

@@ -1450,10 +1442,6 @@ struct ext4_sb_info {
 	struct kobject s_kobj;
 	struct completion s_kobj_unregister;
 	struct super_block *s_sb;
-#ifdef CONFIG_UNICODE
-	struct unicode_map *s_encoding;
-	__u16 s_encoding_flags;
-#endif
 
 	/* Journaling */
 	struct journal_s *s_journal;

fs/ext4/hash.c +1 -1

index 3e133793a5a34..143b0073b3f46 100644
@@ -275,7 +275,7 @@ int ext4fs_dirhash(const struct inode *dir, const char *name, int len,
 		   struct dx_hash_info *hinfo)
 {
 #ifdef CONFIG_UNICODE
-	const struct unicode_map *um = EXT4_SB(dir->i_sb)->s_encoding;
+	const struct unicode_map *um = dir->i_sb->s_encoding;
 	int r, dlen;
 	unsigned char *buff;
 	struct qstr qstr = {.name = name, .len = len };

fs/ext4/namei.c +8 ~~-12~~

index 56738b538ddf4..7e9fb77fd2cc7 100644
@@ -1286,8 +1286,8 @@ static void dx_insert_block(struct dx_frame *frame, u32 hash, ext4_lblk_t block)
 int ext4_ci_compare(const struct inode *parent, const struct qstr *name,
 		    const struct qstr *entry, bool quick)
 {
-	const struct ext4_sb_info *sbi = EXT4_SB(parent->i_sb);
-	const struct unicode_map *um = sbi->s_encoding;
+	const struct super_block *sb = parent->i_sb;
+	const struct unicode_map *um = sb->s_encoding;
 	int ret;
 
 	if (quick)

@@ -1299,7 +1299,7 @@ int ext4_ci_compare(const struct inode *parent, const struct qstr *name,
 		/* Handle invalid character sequence as either an error
 		 * or as an opaque byte sequence.
 		 */
-		if (ext4_has_strict_mode(sbi))
+		if (sb_has_enc_strict_mode(sb))
 			return -EINVAL;
 
 		if (name->len != entry->len)

@@ -1316,7 +1316,7 @@ void ext4_fname_setup_ci_filename(struct inode *dir, const struct qstr *iname,
 {
 	int len;
 
-	if (!IS_CASEFOLDED(dir) || !EXT4_SB(dir->i_sb)->s_encoding) {
+	if (!needs_casefold(dir)) {
 		cf_name->name = NULL;
 		return;
 	}

@@ -1325,7 +1325,7 @@ void ext4_fname_setup_ci_filename(struct inode *dir, const struct qstr *iname,
 	if (!cf_name->name)
 		return;
 
-	len = utf8_casefold(EXT4_SB(dir->i_sb)->s_encoding,
+	len = utf8_casefold(dir->i_sb->s_encoding,
 			    iname, cf_name->name,
 			    EXT4_NAME_LEN);
 	if (len <= 0) {

@@ -1362,7 +1362,7 @@ static inline bool ext4_match(const struct inode *parent,
 #endif
 
 #ifdef CONFIG_UNICODE
-	if (EXT4_SB(parent->i_sb)->s_encoding && IS_CASEFOLDED(parent)) {
+	if (needs_casefold(parent)) {
 		if (fname->cf_name.name) {
 			struct qstr cf = {.name = fname->cf_name.name,
 					  .len = fname->cf_name.len};

@@ -2171,9 +2171,6 @@ static int ext4_add_entry(handle_t *handle, struct dentry *dentry,
 	struct buffer_head *bh = NULL;
 	struct ext4_dir_entry_2 *de;
 	struct super_block *sb;
-#ifdef CONFIG_UNICODE
-	struct ext4_sb_info *sbi;
-#endif
 	struct ext4_filename fname;
 	int	retval;
 	int	dx_fallback=0;

@@ -2190,9 +2187,8 @@ static int ext4_add_entry(handle_t *handle, struct dentry *dentry,
 		return -EINVAL;
 
 #ifdef CONFIG_UNICODE
-	sbi = EXT4_SB(sb);
-	if (ext4_has_strict_mode(sbi) && IS_CASEFOLDED(dir) &&
-	    sbi->s_encoding && utf8_validate(sbi->s_encoding, &dentry->d_name))
+	if (sb_has_enc_strict_mode(sb) && IS_CASEFOLDED(dir) &&
+	    sb->s_encoding && utf8_validate(sb->s_encoding, &dentry->d_name))
 		return -EINVAL;
 #endif

fs/ext4/super.c +6 -6

index 330957ed1f05c..d097771a374f8 100644
@@ -1102,7 +1102,7 @@ static void ext4_put_super(struct super_block *sb)
 	fs_put_dax(sbi->s_daxdev);
 	fscrypt_free_dummy_context(&sbi->s_dummy_enc_ctx);
 #ifdef CONFIG_UNICODE
-	utf8_unload(sbi->s_encoding);
+	utf8_unload(sb->s_encoding);
 #endif
 	kfree(sbi);
 }

@@ -4035,7 +4035,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		goto failed_mount;
 
 #ifdef CONFIG_UNICODE
-	if (ext4_has_feature_casefold(sb) && !sbi->s_encoding) {
+	if (ext4_has_feature_casefold(sb) && !sb->s_encoding) {
 		const struct ext4_sb_encodings *encoding_info;
 		struct unicode_map *encoding;
 		__u16 encoding_flags;

@@ -4066,8 +4066,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 			 "%s-%s with flags 0x%hx", encoding_info->name,
 			 encoding_info->version?:"\b", encoding_flags);
 
-		sbi->s_encoding = encoding;
-		sbi->s_encoding_flags = encoding_flags;
+		sb->s_encoding = encoding;
+		sb->s_encoding_flags = encoding_flags;
 	}
 #endif

@@ -4678,7 +4678,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	}
 
 #ifdef CONFIG_UNICODE
-	if (sbi->s_encoding)
+	if (sb->s_encoding)
 		sb->s_d_op = &ext4_dentry_ops;
 #endif

@@ -4873,7 +4873,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		crypto_free_shash(sbi->s_chksum_driver);
 
 #ifdef CONFIG_UNICODE
-	utf8_unload(sbi->s_encoding);
+	utf8_unload(sb->s_encoding);
 #endif
 
 #ifdef CONFIG_QUOTA

Signature · 1 line

2.27.0.111.gc72c7da667-goog

Gabriel Krisman Bertazi

Reviewed-by on v9 4/4

2020-06-24 05:43 UTC #15 lore raw

Re: [PATCH v9 4/4] ext4: Use generic casefolding support

fs/ext4/dir.c const struct file_operations ext4_dir_operations = { · line 676

Quoted 8 lines · Daniel Rosenberg

fs/ext4/dir.c +2 -3

-
 const struct dentry_operations ext4_dentry_ops = {
-	.d_hash = ext4_d_hash,
-	.d_compare = ext4_d_compare,
+	.d_hash = generic_ci_d_hash,
+	.d_compare = generic_ci_d_compare,
 };
 #endif

Can you make the structure generic since it is the same for f2fs and ext4, which let you drop the code guards? Unless that becomes a problem for d_revalidate with fscrypt, it is fine like this.

fs/ext4/namei.c static int ext4_add_entry(handle_t *handle, struct dentry *dentry, · line 2190

Quoted 5 lines

#ifdef CONFIG_UNICODE

fs/ext4/namei.c +1 -3

-	sbi = EXT4_SB(sb);
-	if (ext4_has_strict_mode(sbi) && IS_CASEFOLDED(dir) &&
-	    sbi->s_encoding && utf8_validate(sbi->s_encoding, &dentry->d_name))
+	if (sb_has_enc_strict_mode(sb) && IS_CASEFOLDED(dir) &&

I keep reading the 'enc' in sb_has_enc_strict_mode() as 'encryption'. What do you think about renaming it to sb_has_strict_encoding()?

These comments apply equally to patches 3 and 4. Other than that,

Reviewed-by Gabriel Krisman Bertazi <[email protected]>

Signature · 1 line

Gabriel Krisman Bertazi

Daniel Rosenberg

author

2020-07-07 10:44 UTC #16 lore raw

Re: [PATCH v9 4/4] ext4: Use generic casefolding support

Quoted 12 lines · Gabriel Krisman Bertazi

fs/ext4/dir.c const struct file_operations ext4_dir_operations = { · line 676
Quoted 8 lines · Daniel Rosenberg
fs/ext4/dir.c +2 -3
-
 const struct dentry_operations ext4_dentry_ops = {
-     .d_hash = ext4_d_hash,
-     .d_compare = ext4_d_compare,
+     .d_hash = generic_ci_d_hash,
+     .d_compare = generic_ci_d_compare,
 };
 #endif

Quoted 6 lines

Can you make the structure generic since it is the same for f2fs and ext4, which let you drop the code guards? Unless that becomes a problem for d_revalidate with fscrypt, it is fine like this.
--
Gabriel Krisman Bertazi

I unify them in a later patch, since I end up having to deal with fscrypt's d_revalidate. With that patch I'd end up undoing the export I'd add for this, so I'll skip that for the moment.

-Daniel

Eric Biggers

maintainer

2020-06-24 06:04 UTC #17 lore raw

Re: [PATCH v9 4/4] ext4: Use generic casefolding support

In reply to Daniel Rosenberg

Quoted 2 lines · Daniel Rosenberg

This switches ext4 over to the generic support provided in commit 5f829feca774 ("fs: Add standard casefolding support")

Commit IDs aren't determined until the patches are applied. It's possible for the person applying the patches to fix them, but in general people will forget, so it's better not to include non-stable commit IDs.

Also, a sentence explaining *why* this change is good would be helpful. Commit messages should always have a *why* unless it's obvious.

Likewise for the f2fs commit.

- Eric

Eric Biggers

maintainer

2020-06-24 05:35 UTC #18 lore raw

In reply to Daniel Rosenberg

Quoted 4 lines · Daniel Rosenberg

This lays the ground work for enabling casefolding and encryption at the same time for ext4 and f2fs. A future set of patches will enable that functionality. These unify the highly similar dentry_operations that ext4 and f2fs both use for casefolding.

I think this undersells this patchset a bit. This patchset makes ext4 and f2fs share the casefolded ->d_compare() and ->d_hash() implementations, which eliminates duplicated code. That's a good thing regardless of whether we're going to add encrypt+casefold support or not.

It also changes the casefolded ->d_hash() implementation to not have to allocate memory (with GFP_ATOMIC, no less), which was a big problem with the old implementation as it's unreliable and inefficient.

So yes, this prepares for supporting encrypt+casefold. But these changes make sense on their own too as an improvement of the casefold feature. Except for the one line of code in needs_casefold() that is specific to encrypt+casefold; maybe that should be left out for now.

(Side note: I think you could drop linux-doc and linux-mtd from Cc, as this patchset isn't really relevant to those mailing lists.)

- Eric