These patches are on top of the torvalds tree.
F2FS currently supports casefolding and encryption, but not at
the same time. These patches aim to rectify that. In a later follow up,
this will be added for Ext4 as well.
The f2fs-tools changes have already been applied.
Since both fscrypt and casefolding require their own dentry operations,
I've moved the responsibility of setting the dentry operations from fscrypt
to the filesystems and provided helper functions that should work for most
cases.
These are a follow-up to the previously sent patch set
"[PATCH v12 0/4] Prepare for upcoming Casefolding/Encryption patches"
v2:
Simplified generic dentry_op function
Passed through errors in f2fs_match_ci_name
v3:
Split some long lines
Cleaned up some code
Made some comments clearer
Fixed bug in v2 error passing
v4:
Added reviewed bys and acks from Eric
Removed unneeded variable
ifdef consistency
Daniel Rosenberg (3):
libfs: Add generic function for setting dentry_ops
fscrypt: Have filesystems handle their d_ops
f2fs: Handle casefolding with Encryption
fs/crypto/fname.c | 4 --
fs/crypto/fscrypt_private.h | 1 -
fs/crypto/hooks.c | 1 -
fs/ext4/dir.c | 7 ---
fs/ext4/ext4.h | 4 --
fs/ext4/namei.c | 1 +
fs/ext4/super.c | 5 --
fs/f2fs/dir.c | 105 ++++++++++++++++++++++++++----------
fs/f2fs/f2fs.h | 11 ++--
fs/f2fs/hash.c | 11 +++-
fs/f2fs/inline.c | 4 ++
fs/f2fs/namei.c | 1 +
fs/f2fs/recovery.c | 12 ++++-
fs/f2fs/super.c | 7 ---
fs/libfs.c | 70 ++++++++++++++++++++++++
fs/ubifs/dir.c | 1 +
include/linux/fs.h | 1 +
include/linux/fscrypt.h | 7 ++-
18 files changed, 185 insertions(+), 68 deletions(-)
base-commit: 0fa8ee0d9ab95c9350b8b84574824d9a384a9f7d
--
2.29.2.454.gaff20da3a2-goog
Expand f2fs's casefolding support to include encrypted directories. To
index casefolded+encrypted directories, we use the SipHash of the
casefolded name, keyed by a key derived from the directory's fscrypt
master key. This ensures that the dirhash doesn't leak information
about the plaintext filenames.
Encryption keys are unavailable during roll-forward recovery, so we
can't compute the dirhash when recovering a new dentry in an encrypted +
casefolded directory. To avoid having to force a checkpoint when a new
file is fsync'ed, store the dirhash on-disk appended to i_name.
This patch incorporates work by Eric Biggers <[email protected]>
and Jaegeuk Kim <[email protected]>.
Co-developed-by: Eric Biggers <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
Signed-off-by: Daniel Rosenberg <[email protected]>
Reviewed-by: Eric Biggers <[email protected]>
---
fs/f2fs/dir.c | 98 +++++++++++++++++++++++++++++++++++-----------
fs/f2fs/f2fs.h | 8 ++--
fs/f2fs/hash.c | 11 +++++-
fs/f2fs/inline.c | 4 ++
fs/f2fs/recovery.c | 12 +++++-
fs/f2fs/super.c | 6 ---
6 files changed, 106 insertions(+), 33 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 71fdf5076461..82b58d1f80eb 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -5,6 +5,7 @@
* Copyright (c) 2012 Samsung Electronics Co., Ltd.
* http://www.samsung.com/
*/
+#include <asm/unaligned.h>
#include <linux/fs.h>
#include <linux/f2fs_fs.h>
#include <linux/sched/signal.h>
@@ -206,30 +207,55 @@ static struct f2fs_dir_entry *find_in_block(struct inode *dir,
/*
* 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 bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
+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);
- if (res < 0) {
- /*
- * In strict mode, ignore invalid names. In non-strict mode,
- * fall back to treating them as opaque byte sequences.
- */
- if (sb_has_strict_encoding(sb) || name->len != entry.len)
- return false;
- return !memcmp(name->name, entry.name, name->len);
+ /*
+ * 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);
}
- return res == 0;
+out:
+ kfree(decrypted_name.name);
+ return res;
}
#endif /* CONFIG_UNICODE */
-static inline bool f2fs_match_name(const struct inode *dir,
+static inline int f2fs_match_name(const struct inode *dir,
const struct f2fs_filename *fname,
const u8 *de_name, u32 de_name_len)
{
@@ -256,6 +282,7 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
struct f2fs_dir_entry *de;
unsigned long bit_pos = 0;
int max_len = 0;
+ int res = 0;
if (max_slots)
*max_slots = 0;
@@ -273,10 +300,15 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
continue;
}
- if (de->hash_code == fname->hash &&
- f2fs_match_name(d->inode, fname, d->filename[bit_pos],
- le16_to_cpu(de->name_len)))
- goto found;
+ if (de->hash_code == fname->hash) {
+ res = f2fs_match_name(d->inode, fname,
+ d->filename[bit_pos],
+ le16_to_cpu(de->name_len));
+ if (res < 0)
+ return ERR_PTR(res);
+ if (res)
+ goto found;
+ }
if (max_slots && max_len > *max_slots)
*max_slots = max_len;
@@ -326,7 +358,11 @@ static struct f2fs_dir_entry *find_in_level(struct inode *dir,
}
de = find_in_block(dir, dentry_page, fname, &max_slots);
- if (de) {
+ if (IS_ERR(de)) {
+ *res_page = ERR_CAST(de);
+ de = NULL;
+ break;
+ } else if (de) {
*res_page = dentry_page;
break;
}
@@ -448,17 +484,39 @@ void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
f2fs_put_page(page, 1);
}
-static void init_dent_inode(const struct f2fs_filename *fname,
+static void init_dent_inode(struct inode *dir, struct inode *inode,
+ const struct f2fs_filename *fname,
struct page *ipage)
{
struct f2fs_inode *ri;
+ if (!fname) /* tmpfile case? */
+ return;
+
f2fs_wait_on_page_writeback(ipage, NODE, true, true);
/* copy name info. to this inode page */
ri = F2FS_INODE(ipage);
ri->i_namelen = cpu_to_le32(fname->disk_name.len);
memcpy(ri->i_name, fname->disk_name.name, fname->disk_name.len);
+ if (IS_ENCRYPTED(dir)) {
+ file_set_enc_name(inode);
+ /*
+ * Roll-forward recovery doesn't have encryption keys available,
+ * so it can't compute the dirhash for encrypted+casefolded
+ * filenames. Append it to i_name if possible. Else, disable
+ * roll-forward recovery of the dentry (i.e., make fsync'ing the
+ * file force a checkpoint) by setting LOST_PINO.
+ */
+ if (IS_CASEFOLDED(dir)) {
+ if (fname->disk_name.len + sizeof(f2fs_hash_t) <=
+ F2FS_NAME_LEN)
+ put_unaligned(fname->hash, (f2fs_hash_t *)
+ &ri->i_name[fname->disk_name.len]);
+ else
+ file_lost_pino(inode);
+ }
+ }
set_page_dirty(ipage);
}
@@ -541,11 +599,7 @@ struct page *f2fs_init_inode_metadata(struct inode *inode, struct inode *dir,
return page;
}
- if (fname) {
- init_dent_inode(fname, page);
- if (IS_ENCRYPTED(dir))
- file_set_enc_name(inode);
- }
+ init_dent_inode(dir, inode, fname, page);
/*
* This file should be checkpointed during fsync.
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 62b4f31d30e2..878308736761 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -533,9 +533,11 @@ struct f2fs_filename {
#ifdef CONFIG_UNICODE
/*
* For casefolded directories: the casefolded name, but it's left NULL
- * if the original name is not valid Unicode or if the filesystem is
- * doing an internal operation where usr_fname is also NULL. In these
- * cases we fall back to treating the name as an opaque byte sequence.
+ * if the original name is not valid Unicode, if the directory is both
+ * casefolded and encrypted and its encryption key is unavailable, or if
+ * the filesystem is doing an 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;
#endif
diff --git a/fs/f2fs/hash.c b/fs/f2fs/hash.c
index de841aaf3c43..e3beac546c63 100644
--- a/fs/f2fs/hash.c
+++ b/fs/f2fs/hash.c
@@ -111,7 +111,9 @@ void f2fs_hash_filename(const struct inode *dir, struct f2fs_filename *fname)
* If the casefolded name is provided, hash it instead of the
* on-disk name. If the casefolded name is *not* provided, that
* should only be because the name wasn't valid Unicode, so fall
- * back to treating the name as an opaque byte sequence.
+ * back to treating the name as an opaque byte sequence. Note
+ * that to handle encrypted directories, the fallback must use
+ * usr_fname (plaintext) rather than disk_name (ciphertext).
*/
WARN_ON_ONCE(!fname->usr_fname->name);
if (fname->cf_name.name) {
@@ -121,6 +123,13 @@ void f2fs_hash_filename(const struct inode *dir, struct f2fs_filename *fname)
name = fname->usr_fname->name;
len = fname->usr_fname->len;
}
+ if (IS_ENCRYPTED(dir)) {
+ struct qstr tmp = QSTR_INIT(name, len);
+
+ fname->hash =
+ cpu_to_le32(fscrypt_fname_siphash(dir, &tmp));
+ return;
+ }
}
#endif
fname->hash = cpu_to_le32(TEA_hash_name(name, len));
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 70384e31788d..92e9852d316a 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -332,6 +332,10 @@ struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
make_dentry_ptr_inline(dir, &d, inline_dentry);
de = f2fs_find_target_dentry(&d, fname, NULL);
unlock_page(ipage);
+ if (IS_ERR(de)) {
+ *res_page = ERR_CAST(de);
+ de = NULL;
+ }
if (de)
*res_page = ipage;
else
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 4f12ade6410a..0947d36af1a8 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -5,6 +5,7 @@
* Copyright (c) 2012 Samsung Electronics Co., Ltd.
* http://www.samsung.com/
*/
+#include <asm/unaligned.h>
#include <linux/fs.h>
#include <linux/f2fs_fs.h>
#include "f2fs.h"
@@ -128,7 +129,16 @@ static int init_recovered_filename(const struct inode *dir,
}
/* Compute the hash of the filename */
- if (IS_CASEFOLDED(dir)) {
+ if (IS_ENCRYPTED(dir) && IS_CASEFOLDED(dir)) {
+ /*
+ * In this case the hash isn't computable without the key, so it
+ * was saved on-disk.
+ */
+ if (fname->disk_name.len + sizeof(f2fs_hash_t) > F2FS_NAME_LEN)
+ return -EINVAL;
+ fname->hash = get_unaligned((f2fs_hash_t *)
+ &raw_inode->i_name[fname->disk_name.len]);
+ } else if (IS_CASEFOLDED(dir)) {
err = f2fs_init_casefolded_name(dir, fname);
if (err)
return err;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index f51d52591c99..42293b7ceaf2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3399,12 +3399,6 @@ static int f2fs_setup_casefold(struct f2fs_sb_info *sbi)
struct unicode_map *encoding;
__u16 encoding_flags;
- if (f2fs_sb_has_encrypt(sbi)) {
- f2fs_err(sbi,
- "Can't mount with encoding and encryption");
- return -EINVAL;
- }
-
if (f2fs_sb_read_encoding(sbi->raw_super, &encoding_info,
&encoding_flags)) {
f2fs_err(sbi,
--
2.29.2.454.gaff20da3a2-goog
This shifts the responsibility of setting up dentry operations from
fscrypt to the individual filesystems, allowing them to have their own
operations while still setting fscrypt's d_revalidate as appropriate.
Most filesystems can just use generic_set_encrypted_ci_d_ops, unless
they have their own specific dentry operations as well. That operation
will set the minimal d_ops required under the circumstances.
Since the fscrypt d_ops are set later on, we must set all d_ops there,
since we cannot adjust those later on. This should not result in any
change in behavior.
Signed-off-by: Daniel Rosenberg <[email protected]>
Acked-by: Eric Biggers <[email protected]>
---
fs/crypto/fname.c | 4 ----
fs/crypto/fscrypt_private.h | 1 -
fs/crypto/hooks.c | 1 -
fs/ext4/dir.c | 7 -------
fs/ext4/ext4.h | 4 ----
fs/ext4/namei.c | 1 +
fs/ext4/super.c | 5 -----
fs/f2fs/dir.c | 7 -------
fs/f2fs/f2fs.h | 3 ---
fs/f2fs/namei.c | 1 +
fs/f2fs/super.c | 1 -
fs/ubifs/dir.c | 1 +
include/linux/fscrypt.h | 7 +++++--
13 files changed, 8 insertions(+), 35 deletions(-)
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 1fbe6c24d705..cb3cfa6329ba 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -570,7 +570,3 @@ int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
return valid;
}
EXPORT_SYMBOL_GPL(fscrypt_d_revalidate);
-
-const struct dentry_operations fscrypt_d_ops = {
- .d_revalidate = fscrypt_d_revalidate,
-};
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 4f5806a3b73d..df9c48c1fbf7 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -294,7 +294,6 @@ int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
bool fscrypt_fname_encrypted_size(const union fscrypt_policy *policy,
u32 orig_len, u32 max_len,
u32 *encrypted_len_ret);
-extern const struct dentry_operations fscrypt_d_ops;
/* hkdf.c */
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 20b0df47fe6a..9006fa983335 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -117,7 +117,6 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
spin_lock(&dentry->d_lock);
dentry->d_flags |= DCACHE_NOKEY_NAME;
spin_unlock(&dentry->d_lock);
- d_set_d_op(dentry, &fscrypt_d_ops);
}
return err;
}
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index ca50c90adc4c..e757319a4472 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -667,10 +667,3 @@ const struct file_operations ext4_dir_operations = {
.open = ext4_dir_open,
.release = ext4_release_dir,
};
-
-#ifdef CONFIG_UNICODE
-const struct dentry_operations ext4_dentry_ops = {
- .d_hash = generic_ci_d_hash,
- .d_compare = generic_ci_d_compare,
-};
-#endif
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index bf9429484462..ad77f01d9e20 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3380,10 +3380,6 @@ static inline void ext4_unlock_group(struct super_block *sb,
/* dir.c */
extern const struct file_operations ext4_dir_operations;
-#ifdef CONFIG_UNICODE
-extern const struct dentry_operations ext4_dentry_ops;
-#endif
-
/* file.c */
extern const struct inode_operations ext4_file_inode_operations;
extern const struct file_operations ext4_file_operations;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 33509266f5a0..12a417ff5648 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1614,6 +1614,7 @@ static struct buffer_head *ext4_lookup_entry(struct inode *dir,
struct buffer_head *bh;
err = ext4_fname_prepare_lookup(dir, dentry, &fname);
+ generic_set_encrypted_ci_d_ops(dentry);
if (err == -ENOENT)
return NULL;
if (err)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 6633b20224d5..0288bedf46e1 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4968,11 +4968,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
goto failed_mount4;
}
-#ifdef CONFIG_UNICODE
- if (sb->s_encoding)
- sb->s_d_op = &ext4_dentry_ops;
-#endif
-
sb->s_root = d_make_root(root);
if (!sb->s_root) {
ext4_msg(sb, KERN_ERR, "get root dentry failed");
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 4b9ef8bbfa4a..71fdf5076461 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -1099,10 +1099,3 @@ const struct file_operations f2fs_dir_operations = {
.compat_ioctl = f2fs_compat_ioctl,
#endif
};
-
-#ifdef CONFIG_UNICODE
-const struct dentry_operations f2fs_dentry_ops = {
- .d_hash = generic_ci_d_hash,
- .d_compare = generic_ci_d_compare,
-};
-#endif
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index cb700d797296..62b4f31d30e2 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3767,9 +3767,6 @@ static inline void f2fs_update_sit_info(struct f2fs_sb_info *sbi) {}
#endif
extern const struct file_operations f2fs_dir_operations;
-#ifdef CONFIG_UNICODE
-extern const struct dentry_operations f2fs_dentry_ops;
-#endif
extern const struct file_operations f2fs_file_operations;
extern const struct inode_operations f2fs_file_inode_operations;
extern const struct address_space_operations f2fs_dblock_aops;
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 8fa37d1434de..6edb1ab579a1 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -497,6 +497,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
}
err = f2fs_prepare_lookup(dir, dentry, &fname);
+ generic_set_encrypted_ci_d_ops(dentry);
if (err == -ENOENT)
goto out_splice;
if (err)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 00eff2f51807..f51d52591c99 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3427,7 +3427,6 @@ static int f2fs_setup_casefold(struct f2fs_sb_info *sbi)
sbi->sb->s_encoding = encoding;
sbi->sb->s_encoding_flags = encoding_flags;
- sbi->sb->s_d_op = &f2fs_dentry_ops;
}
#else
if (f2fs_sb_has_casefold(sbi)) {
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 155521e51ac5..7a920434d741 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -203,6 +203,7 @@ static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
dbg_gen("'%pd' in dir ino %lu", dentry, dir->i_ino);
err = fscrypt_prepare_lookup(dir, dentry, &nm);
+ generic_set_encrypted_ci_d_ops(dentry);
if (err == -ENOENT)
return d_splice_alias(NULL, dentry);
if (err)
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index a8f7a43f031b..e72f80482671 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -741,8 +741,11 @@ static inline int fscrypt_prepare_rename(struct inode *old_dir,
* directory's encryption key is available, then the lookup is assumed to be by
* plaintext name; otherwise, it is assumed to be by no-key name.
*
- * This also installs a custom ->d_revalidate() method which will invalidate the
- * dentry if it was created without the key and the key is later added.
+ * This will set DCACHE_NOKEY_NAME on the dentry if the lookup is by no-key
+ * name. In this case the filesystem must assign the dentry a dentry_operations
+ * which contains fscrypt_d_revalidate (or contains a d_revalidate method that
+ * calls fscrypt_d_revalidate), so that the dentry will be invalidated if the
+ * directory's encryption key is later added.
*
* Return: 0 on success; -ENOENT if the directory's key is unavailable but the
* filename isn't a valid no-key name, so a negative dentry should be created;
--
2.29.2.454.gaff20da3a2-goog
Daniel Rosenberg <[email protected]> writes:
> This shifts the responsibility of setting up dentry operations from
> fscrypt to the individual filesystems, allowing them to have their own
> operations while still setting fscrypt's d_revalidate as appropriate.
>
> Most filesystems can just use generic_set_encrypted_ci_d_ops, unless
> they have their own specific dentry operations as well. That operation
> will set the minimal d_ops required under the circumstances.
>
> Since the fscrypt d_ops are set later on, we must set all d_ops there,
> since we cannot adjust those later on. This should not result in any
> change in behavior.
>
> Signed-off-by: Daniel Rosenberg <[email protected]>
> Acked-by: Eric Biggers <[email protected]>
> ---
> fs/crypto/fname.c | 4 ----
> fs/crypto/fscrypt_private.h | 1 -
> fs/crypto/hooks.c | 1 -
> fs/ext4/dir.c | 7 -------
> fs/ext4/ext4.h | 4 ----
> fs/ext4/namei.c | 1 +
> fs/ext4/super.c | 5 -----
> fs/f2fs/dir.c | 7 -------
> fs/f2fs/f2fs.h | 3 ---
> fs/f2fs/namei.c | 1 +
> fs/f2fs/super.c | 1 -
> fs/ubifs/dir.c | 1 +
> include/linux/fscrypt.h | 7 +++++--
> 13 files changed, 8 insertions(+), 35 deletions(-)
>
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 1fbe6c24d705..cb3cfa6329ba 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -570,7 +570,3 @@ int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
> return valid;
> }
> EXPORT_SYMBOL_GPL(fscrypt_d_revalidate);
> -
> -const struct dentry_operations fscrypt_d_ops = {
> - .d_revalidate = fscrypt_d_revalidate,
> -};
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 4f5806a3b73d..df9c48c1fbf7 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -294,7 +294,6 @@ int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
> bool fscrypt_fname_encrypted_size(const union fscrypt_policy *policy,
> u32 orig_len, u32 max_len,
> u32 *encrypted_len_ret);
> -extern const struct dentry_operations fscrypt_d_ops;
>
> /* hkdf.c */
>
> diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
> index 20b0df47fe6a..9006fa983335 100644
> --- a/fs/crypto/hooks.c
> +++ b/fs/crypto/hooks.c
> @@ -117,7 +117,6 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
> spin_lock(&dentry->d_lock);
> dentry->d_flags |= DCACHE_NOKEY_NAME;
> spin_unlock(&dentry->d_lock);
> - d_set_d_op(dentry, &fscrypt_d_ops);
> }
> return err;
> }
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index ca50c90adc4c..e757319a4472 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -667,10 +667,3 @@ const struct file_operations ext4_dir_operations = {
> .open = ext4_dir_open,
> .release = ext4_release_dir,
> };
> -
> -#ifdef CONFIG_UNICODE
> -const struct dentry_operations ext4_dentry_ops = {
> - .d_hash = generic_ci_d_hash,
> - .d_compare = generic_ci_d_compare,
> -};
> -#endif
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index bf9429484462..ad77f01d9e20 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3380,10 +3380,6 @@ static inline void ext4_unlock_group(struct super_block *sb,
> /* dir.c */
> extern const struct file_operations ext4_dir_operations;
>
> -#ifdef CONFIG_UNICODE
> -extern const struct dentry_operations ext4_dentry_ops;
> -#endif
> -
> /* file.c */
> extern const struct inode_operations ext4_file_inode_operations;
> extern const struct file_operations ext4_file_operations;
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 33509266f5a0..12a417ff5648 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1614,6 +1614,7 @@ static struct buffer_head *ext4_lookup_entry(struct inode *dir,
> struct buffer_head *bh;
>
> err = ext4_fname_prepare_lookup(dir, dentry, &fname);
> + generic_set_encrypted_ci_d_ops(dentry);
> if (err == -ENOENT)
> return NULL;
> if (err)
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 6633b20224d5..0288bedf46e1 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4968,11 +4968,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> goto failed_mount4;
> }
>
> -#ifdef CONFIG_UNICODE
> - if (sb->s_encoding)
> - sb->s_d_op = &ext4_dentry_ops;
> -#endif
This change has the side-effect of removing the capability of the root
directory from being case-insensitive. It is not a backward
incompatible change because there is no way to make the root directory
CI at the moment (it is never empty). But this restriction seems
artificial. Is there a real reason to prevent the root inode from being
case-insensitive?
> -
> sb->s_root = d_make_root(root);
> if (!sb->s_root) {
> ext4_msg(sb, KERN_ERR, "get root dentry failed");
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 4b9ef8bbfa4a..71fdf5076461 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -1099,10 +1099,3 @@ const struct file_operations f2fs_dir_operations = {
> .compat_ioctl = f2fs_compat_ioctl,
> #endif
> };
> -
> -#ifdef CONFIG_UNICODE
> -const struct dentry_operations f2fs_dentry_ops = {
> - .d_hash = generic_ci_d_hash,
> - .d_compare = generic_ci_d_compare,
> -};
> -#endif
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index cb700d797296..62b4f31d30e2 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3767,9 +3767,6 @@ static inline void f2fs_update_sit_info(struct f2fs_sb_info *sbi) {}
> #endif
>
> extern const struct file_operations f2fs_dir_operations;
> -#ifdef CONFIG_UNICODE
> -extern const struct dentry_operations f2fs_dentry_ops;
> -#endif
> extern const struct file_operations f2fs_file_operations;
> extern const struct inode_operations f2fs_file_inode_operations;
> extern const struct address_space_operations f2fs_dblock_aops;
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 8fa37d1434de..6edb1ab579a1 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -497,6 +497,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
> }
>
> err = f2fs_prepare_lookup(dir, dentry, &fname);
> + generic_set_encrypted_ci_d_ops(dentry);
> if (err == -ENOENT)
> goto out_splice;
> if (err)
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 00eff2f51807..f51d52591c99 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3427,7 +3427,6 @@ static int f2fs_setup_casefold(struct f2fs_sb_info *sbi)
>
> sbi->sb->s_encoding = encoding;
> sbi->sb->s_encoding_flags = encoding_flags;
> - sbi->sb->s_d_op = &f2fs_dentry_ops;
> }
> #else
> if (f2fs_sb_has_casefold(sbi)) {
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 155521e51ac5..7a920434d741 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -203,6 +203,7 @@ static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
> dbg_gen("'%pd' in dir ino %lu", dentry, dir->i_ino);
>
> err = fscrypt_prepare_lookup(dir, dentry, &nm);
> + generic_set_encrypted_ci_d_ops(dentry);
> if (err == -ENOENT)
> return d_splice_alias(NULL, dentry);
> if (err)
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index a8f7a43f031b..e72f80482671 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -741,8 +741,11 @@ static inline int fscrypt_prepare_rename(struct inode *old_dir,
> * directory's encryption key is available, then the lookup is assumed to be by
> * plaintext name; otherwise, it is assumed to be by no-key name.
> *
> - * This also installs a custom ->d_revalidate() method which will invalidate the
> - * dentry if it was created without the key and the key is later added.
> + * This will set DCACHE_NOKEY_NAME on the dentry if the lookup is by no-key
> + * name. In this case the filesystem must assign the dentry a dentry_operations
> + * which contains fscrypt_d_revalidate (or contains a d_revalidate method that
> + * calls fscrypt_d_revalidate), so that the dentry will be invalidated if the
> + * directory's encryption key is later added.
> *
> * Return: 0 on success; -ENOENT if the directory's key is unavailable but the
> * filename isn't a valid no-key name, so a negative dentry should be created;
--
Gabriel Krisman Bertazi
Hi all,
On Thu, Nov 19, 2020 at 06:09:03AM +0000, Daniel Rosenberg wrote:
> This shifts the responsibility of setting up dentry operations from
> fscrypt to the individual filesystems, allowing them to have their own
> operations while still setting fscrypt's d_revalidate as appropriate.
>
> Most filesystems can just use generic_set_encrypted_ci_d_ops, unless
> they have their own specific dentry operations as well. That operation
> will set the minimal d_ops required under the circumstances.
>
> Since the fscrypt d_ops are set later on, we must set all d_ops there,
> since we cannot adjust those later on. This should not result in any
> change in behavior.
>
> Signed-off-by: Daniel Rosenberg <[email protected]>
> Acked-by: Eric Biggers <[email protected]>
> ---
...
> extern const struct file_operations ext4_dir_operations;
>
> -#ifdef CONFIG_UNICODE
> -extern const struct dentry_operations ext4_dentry_ops;
> -#endif
> -
> /* file.c */
> extern const struct inode_operations ext4_file_inode_operations;
> extern const struct file_operations ext4_file_operations;
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 33509266f5a0..12a417ff5648 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1614,6 +1614,7 @@ static struct buffer_head *ext4_lookup_entry(struct inode *dir,
> struct buffer_head *bh;
>
> err = ext4_fname_prepare_lookup(dir, dentry, &fname);
> + generic_set_encrypted_ci_d_ops(dentry);
One thing might be worth noticing is that currently overlayfs might
not work properly when dentry->d_sb->s_encoding is set even only some
subdirs are CI-enabled but the others not, see generic_set_encrypted_ci_d_ops(),
ovl_mount_dir_noesc => ovl_dentry_weird()
For more details, see:
https://android-review.googlesource.com/c/device/linaro/hikey/+/1483316/2#message-2e1f6ab0010a3e35e7d8effea73f60341f84ee4d
Just found it by chance (and not sure if it's vital for now), and
a kind reminder about this.
Thanks,
Gao Xiang
On Sat, Nov 21, 2020 at 11:45:41PM -0500, Gabriel Krisman Bertazi wrote:
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index 6633b20224d5..0288bedf46e1 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -4968,11 +4968,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> > goto failed_mount4;
> > }
> >
> > -#ifdef CONFIG_UNICODE
> > - if (sb->s_encoding)
> > - sb->s_d_op = &ext4_dentry_ops;
> > -#endif
>
> This change has the side-effect of removing the capability of the root
> directory from being case-insensitive. It is not a backward
> incompatible change because there is no way to make the root directory
> CI at the moment (it is never empty). But this restriction seems
> artificial. Is there a real reason to prevent the root inode from being
> case-insensitive?
>
The problem is that the "lost+found" directory is special in that e2fsck needs
to be able to find it.
That's the reason why ext4 doesn't allow the root directory to be encrypted.
(And encrypting the root directory isn't really useful anyway, since if the goal
is to encrypt a whole filesystem with one key, dm-crypt is a better solution.)
Casefolding is a bit less problematic than encryption. But it still doesn't
entirely work, as e.g. if you name the directory "LOST+FOUND" instead (the
directory is casefolded after all...), then e2fsck can't find it.
Unless there's a real use case for the root directory being casefolded and
people are willing to fix e2fsck, I think we should just make ext4 return an
error when setting the casefold flag on the root directory, like it does when
trying to enable encryption on the root directory.
- Eric
On Sun, Nov 22, 2020 at 01:12:18PM +0800, Gao Xiang wrote:
> Hi all,
>
> On Thu, Nov 19, 2020 at 06:09:03AM +0000, Daniel Rosenberg wrote:
> > This shifts the responsibility of setting up dentry operations from
> > fscrypt to the individual filesystems, allowing them to have their own
> > operations while still setting fscrypt's d_revalidate as appropriate.
> >
> > Most filesystems can just use generic_set_encrypted_ci_d_ops, unless
> > they have their own specific dentry operations as well. That operation
> > will set the minimal d_ops required under the circumstances.
> >
> > Since the fscrypt d_ops are set later on, we must set all d_ops there,
> > since we cannot adjust those later on. This should not result in any
> > change in behavior.
> >
> > Signed-off-by: Daniel Rosenberg <[email protected]>
> > Acked-by: Eric Biggers <[email protected]>
> > ---
>
> ...
>
> > extern const struct file_operations ext4_dir_operations;
> >
> > -#ifdef CONFIG_UNICODE
> > -extern const struct dentry_operations ext4_dentry_ops;
> > -#endif
> > -
> > /* file.c */
> > extern const struct inode_operations ext4_file_inode_operations;
> > extern const struct file_operations ext4_file_operations;
> > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> > index 33509266f5a0..12a417ff5648 100644
> > --- a/fs/ext4/namei.c
> > +++ b/fs/ext4/namei.c
> > @@ -1614,6 +1614,7 @@ static struct buffer_head *ext4_lookup_entry(struct inode *dir,
> > struct buffer_head *bh;
> >
> > err = ext4_fname_prepare_lookup(dir, dentry, &fname);
> > + generic_set_encrypted_ci_d_ops(dentry);
>
> One thing might be worth noticing is that currently overlayfs might
> not work properly when dentry->d_sb->s_encoding is set even only some
> subdirs are CI-enabled but the others not, see generic_set_encrypted_ci_d_ops(),
> ovl_mount_dir_noesc => ovl_dentry_weird()
>
> For more details, see:
> https://android-review.googlesource.com/c/device/linaro/hikey/+/1483316/2#message-2e1f6ab0010a3e35e7d8effea73f60341f84ee4d
>
> Just found it by chance (and not sure if it's vital for now), and
> a kind reminder about this.
>
Yes, overlayfs doesn't work on ext4 or f2fs filesystems that have the casefold
feature enabled, regardless of which directories are actually using casefolding.
This is an existing limitation which was previously discussed, e.g. at
https://lkml.kernel.org/linux-ext4/CAOQ4uxgPXBazE-g2v=T_vOvnr_f0ZHyKYZ4wvn7A3ePatZrhnQ@mail.gmail.com/T/#u
and
https://lkml.kernel.org/linux-ext4/[email protected]/T/#u.
Gabriel and Daniel, is one of you still looking into fixing this? IIUC, the
current thinking is that when the casefolding flag is set on a directory, it's
too late to assign dentry_operations at that point. But what if all child
dentries (which must be negative) are invalidated first, and also the filesystem
forbids setting the casefold flag on encrypted directories that are accessed via
a no-key name (so that fscrypt_d_revalidate isn't needed -- i.e. the directory
would only go from "no d_ops" to "generic_ci_dentry_ops", not from
"generic_encrypted_dentry_ops" to "generic_encrypted_ci_dentry_ops")?
- Eric
On Mon, Nov 23, 2020 at 02:51:44PM -0800, Eric Biggers wrote:
> On Sun, Nov 22, 2020 at 01:12:18PM +0800, Gao Xiang wrote:
> > Hi all,
> >
> > On Thu, Nov 19, 2020 at 06:09:03AM +0000, Daniel Rosenberg wrote:
> > > This shifts the responsibility of setting up dentry operations from
> > > fscrypt to the individual filesystems, allowing them to have their own
> > > operations while still setting fscrypt's d_revalidate as appropriate.
> > >
> > > Most filesystems can just use generic_set_encrypted_ci_d_ops, unless
> > > they have their own specific dentry operations as well. That operation
> > > will set the minimal d_ops required under the circumstances.
> > >
> > > Since the fscrypt d_ops are set later on, we must set all d_ops there,
> > > since we cannot adjust those later on. This should not result in any
> > > change in behavior.
> > >
> > > Signed-off-by: Daniel Rosenberg <[email protected]>
> > > Acked-by: Eric Biggers <[email protected]>
> > > ---
> >
> > ...
> >
> > > extern const struct file_operations ext4_dir_operations;
> > >
> > > -#ifdef CONFIG_UNICODE
> > > -extern const struct dentry_operations ext4_dentry_ops;
> > > -#endif
> > > -
> > > /* file.c */
> > > extern const struct inode_operations ext4_file_inode_operations;
> > > extern const struct file_operations ext4_file_operations;
> > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> > > index 33509266f5a0..12a417ff5648 100644
> > > --- a/fs/ext4/namei.c
> > > +++ b/fs/ext4/namei.c
> > > @@ -1614,6 +1614,7 @@ static struct buffer_head *ext4_lookup_entry(struct inode *dir,
> > > struct buffer_head *bh;
> > >
> > > err = ext4_fname_prepare_lookup(dir, dentry, &fname);
> > > + generic_set_encrypted_ci_d_ops(dentry);
> >
> > One thing might be worth noticing is that currently overlayfs might
> > not work properly when dentry->d_sb->s_encoding is set even only some
> > subdirs are CI-enabled but the others not, see generic_set_encrypted_ci_d_ops(),
> > ovl_mount_dir_noesc => ovl_dentry_weird()
> >
> > For more details, see:
> > https://android-review.googlesource.com/c/device/linaro/hikey/+/1483316/2#message-2e1f6ab0010a3e35e7d8effea73f60341f84ee4d
> >
> > Just found it by chance (and not sure if it's vital for now), and
> > a kind reminder about this.
> >
>
> Yes, overlayfs doesn't work on ext4 or f2fs filesystems that have the casefold
> feature enabled, regardless of which directories are actually using casefolding.
> This is an existing limitation which was previously discussed, e.g. at
> https://lkml.kernel.org/linux-ext4/CAOQ4uxgPXBazE-g2v=T_vOvnr_f0ZHyKYZ4wvn7A3ePatZrhnQ@mail.gmail.com/T/#u
> and
> https://lkml.kernel.org/linux-ext4/[email protected]/T/#u.
>
> Gabriel and Daniel, is one of you still looking into fixing this? IIUC, the
> current thinking is that when the casefolding flag is set on a directory, it's
> too late to assign dentry_operations at that point. But what if all child
> dentries (which must be negative) are invalidated first, and also the filesystem
> forbids setting the casefold flag on encrypted directories that are accessed via
> a no-key name (so that fscrypt_d_revalidate isn't needed -- i.e. the directory
> would only go from "no d_ops" to "generic_ci_dentry_ops", not from
> "generic_encrypted_dentry_ops" to "generic_encrypted_ci_dentry_ops")?
From my limited knowledge about VFS, I think that is practical as well, since
we don't have sub-sub-dirs since all sub-dirs are negative dentries for empty dirs.
And if casefold ioctl is "dir inode locked", I think that would be fine (?)
I don't check the code though.
Thanks,
Gao Xiang
>
> - Eric
>
Eric Biggers <[email protected]> writes:
> On Sat, Nov 21, 2020 at 11:45:41PM -0500, Gabriel Krisman Bertazi wrote:
>> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> > index 6633b20224d5..0288bedf46e1 100644
>> > --- a/fs/ext4/super.c
>> > +++ b/fs/ext4/super.c
>> > @@ -4968,11 +4968,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>> > goto failed_mount4;
>> > }
>> >
>> > -#ifdef CONFIG_UNICODE
>> > - if (sb->s_encoding)
>> > - sb->s_d_op = &ext4_dentry_ops;
>> > -#endif
>>
>> This change has the side-effect of removing the capability of the root
>> directory from being case-insensitive. It is not a backward
>> incompatible change because there is no way to make the root directory
>> CI at the moment (it is never empty). But this restriction seems
>> artificial. Is there a real reason to prevent the root inode from being
>> case-insensitive?
>>
>
> The problem is that the "lost+found" directory is special in that e2fsck needs
> to be able to find it.
>
> That's the reason why ext4 doesn't allow the root directory to be encrypted.
> (And encrypting the root directory isn't really useful anyway, since if the goal
> is to encrypt a whole filesystem with one key, dm-crypt is a better solution.)
>
> Casefolding is a bit less problematic than encryption. But it still doesn't
> entirely work, as e.g. if you name the directory "LOST+FOUND" instead (the
> directory is casefolded after all...), then e2fsck can't find it.
>
> Unless there's a real use case for the root directory being casefolded and
> people are willing to fix e2fsck, I think we should just make ext4 return an
> error when setting the casefold flag on the root directory, like it does when
> trying to enable encryption on the root directory.
I don't have a use case where I need a root directory to be CI. In
fact, when I first implemented CI, I did want to block the root directory
from being made CI, just to prevent people from doing "chattr +F /" and
complaining afterwards when /usr/lib breaks.
My concern with the curent patch was whether this side-effect was
considered, but I'm happy with either semantics.
--
Gabriel Krisman Bertazi
Eric Biggers <[email protected]> writes:
> On Sun, Nov 22, 2020 at 01:12:18PM +0800, Gao Xiang wrote:
>> Hi all,
>>
>> On Thu, Nov 19, 2020 at 06:09:03AM +0000, Daniel Rosenberg wrote:
>> > This shifts the responsibility of setting up dentry operations from
>> > fscrypt to the individual filesystems, allowing them to have their own
>> > operations while still setting fscrypt's d_revalidate as appropriate.
>> >
>> > Most filesystems can just use generic_set_encrypted_ci_d_ops, unless
>> > they have their own specific dentry operations as well. That operation
>> > will set the minimal d_ops required under the circumstances.
>> >
>> > Since the fscrypt d_ops are set later on, we must set all d_ops there,
>> > since we cannot adjust those later on. This should not result in any
>> > change in behavior.
>> >
>> > Signed-off-by: Daniel Rosenberg <[email protected]>
>> > Acked-by: Eric Biggers <[email protected]>
>> > ---
>>
>> ...
>>
>> > extern const struct file_operations ext4_dir_operations;
>> >
>> > -#ifdef CONFIG_UNICODE
>> > -extern const struct dentry_operations ext4_dentry_ops;
>> > -#endif
>> > -
>> > /* file.c */
>> > extern const struct inode_operations ext4_file_inode_operations;
>> > extern const struct file_operations ext4_file_operations;
>> > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
>> > index 33509266f5a0..12a417ff5648 100644
>> > --- a/fs/ext4/namei.c
>> > +++ b/fs/ext4/namei.c
>> > @@ -1614,6 +1614,7 @@ static struct buffer_head *ext4_lookup_entry(struct inode *dir,
>> > struct buffer_head *bh;
>> >
>> > err = ext4_fname_prepare_lookup(dir, dentry, &fname);
>> > + generic_set_encrypted_ci_d_ops(dentry);
>>
>> One thing might be worth noticing is that currently overlayfs might
>> not work properly when dentry->d_sb->s_encoding is set even only some
>> subdirs are CI-enabled but the others not, see generic_set_encrypted_ci_d_ops(),
>> ovl_mount_dir_noesc => ovl_dentry_weird()
>>
>> For more details, see:
>> https://android-review.googlesource.com/c/device/linaro/hikey/+/1483316/2#message-2e1f6ab0010a3e35e7d8effea73f60341f84ee4d
>>
>> Just found it by chance (and not sure if it's vital for now), and
>> a kind reminder about this.
>>
>
> Yes, overlayfs doesn't work on ext4 or f2fs filesystems that have the casefold
> feature enabled, regardless of which directories are actually using casefolding.
> This is an existing limitation which was previously discussed, e.g. at
> https://lkml.kernel.org/linux-ext4/CAOQ4uxgPXBazE-g2v=T_vOvnr_f0ZHyKYZ4wvn7A3ePatZrhnQ@mail.gmail.com/T/#u
> and
> https://lkml.kernel.org/linux-ext4/[email protected]/T/#u.
>
> Gabriel and Daniel, is one of you still looking into fixing this?
Eric,
overlayfs+CI has been on my todo list for over a year now, as I have a
customer who wants to mix them, but I haven't been able to get to it.
I'm sure I won't be able to get to it until mid next year, so if anyone
wants to tackle it, feel free to do it.
> IIUC, the current thinking is that when the casefolding flag is set on
> a directory, it's too late to assign dentry_operations at that point.
yes
> But what if all child dentries (which must be negative) are
> invalidated first,
I recall I tried this approach when I quickly looked over this last
year, but my limited vfs knowledge prevented me from getting something
working. But it makes sense.
> and also the filesystem forbids setting the casefold flag on encrypted
> directories that are accessed via a no-key name (so that
> fscrypt_d_revalidate isn't needed -- i.e. the directory would only go
> from "no d_ops" to "generic_ci_dentry_ops", not from
> "generic_encrypted_dentry_ops" to "generic_encrypted_ci_dentry_ops")?
--
Gabriel Krisman Bertazi
>
> This change has the side-effect of removing the capability of the root
> directory from being case-insensitive. It is not a backward
> incompatible change because there is no way to make the root directory
> CI at the moment (it is never empty). But this restriction seems
> artificial. Is there a real reason to prevent the root inode from being
> case-insensitive?
> I don't have a use case where I need a root directory to be CI. In
> fact, when I first implemented CI, I did want to block the root directory
> from being made CI, just to prevent people from doing "chattr +F /" and
> complaining afterwards when /usr/lib breaks.
>
> My concern with the curent patch was whether this side-effect was
> considered, but I'm happy with either semantics.
>
> --
> Gabriel Krisman Bertazi
That's just from the lost+found directory right? If you remove it you
can still change it, and then add the lost+found directory back. Isn't
that how it works currently? I definitely didn't intend to change any
behavior around non-encrypted casefolding there.
I should look at what fsck does if you do that and have a LoSt+fOuNd folder...
-Daniel Rosenberg