f2fs: don't use casefolded comparison for "." and ".."
authorEric Biggers <ebiggers@google.com>
Sat, 14 May 2022 17:59:29 +0000 (10:59 -0700)
committerJaegeuk Kim <jaegeuk@kernel.org>
Tue, 17 May 2022 18:19:23 +0000 (11:19 -0700)
Tryng to rename a directory that has all following properties fails with
EINVAL and triggers the 'WARN_ON_ONCE(!fscrypt_has_encryption_key(dir))'
in f2fs_match_ci_name():

    - The directory is casefolded
    - The directory is encrypted
    - The directory's encryption key is not yet set up
    - The parent directory is *not* encrypted

The problem is incorrect handling of the lookup of ".." to get the
parent reference to update.  fscrypt_setup_filename() treats ".." (and
".") specially, as it's never encrypted.  It's passed through as-is, and
setting up the directory's key is not attempted.  As the name isn't a
no-key name, f2fs treats it as a "normal" name and attempts a casefolded
comparison.  That breaks the assumption of the WARN_ON_ONCE() in
f2fs_match_ci_name() which assumes that for encrypted directories,
casefolded comparisons only happen when the directory's key is set up.

We could just remove this WARN_ON_ONCE().  However, since casefolding is
always a no-op on "." and ".." anyway, let's instead just not casefold
these names.  This results in the standard bytewise comparison.

Fixes: 7ad08a58bf67 ("f2fs: Handle casefolding with Encryption")
Cc: <stable@vger.kernel.org> # v5.11+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
fs/f2fs/dir.c
fs/f2fs/f2fs.h
fs/f2fs/hash.c

index a0e5193..d5bd793 100644 (file)
@@ -82,7 +82,8 @@ int f2fs_init_casefolded_name(const struct inode *dir,
 #if IS_ENABLED(CONFIG_UNICODE)
        struct super_block *sb = dir->i_sb;
 
-       if (IS_CASEFOLDED(dir)) {
+       if (IS_CASEFOLDED(dir) &&
+           !is_dot_dotdot(fname->usr_fname->name, fname->usr_fname->len)) {
                fname->cf_name.name = f2fs_kmem_cache_alloc(f2fs_cf_name_slab,
                                        GFP_NOFS, false, F2FS_SB(sb));
                if (!fname->cf_name.name)
index 492af5b..e9e32bc 100644 (file)
@@ -508,11 +508,11 @@ struct f2fs_filename {
 #if IS_ENABLED(CONFIG_UNICODE)
        /*
         * For casefolded directories: the casefolded name, but it's left NULL
-        * 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.
+        * if the original name is not valid Unicode, if the original name is
+        * "." or "..", 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
index 3cb1e7a..049ce50 100644 (file)
@@ -91,7 +91,7 @@ static u32 TEA_hash_name(const u8 *p, size_t len)
 /*
  * Compute @fname->hash.  For all directories, @fname->disk_name must be set.
  * For casefolded directories, @fname->usr_fname must be set, and also
- * @fname->cf_name if the filename is valid Unicode.
+ * @fname->cf_name if the filename is valid Unicode and is not "." or "..".
  */
 void f2fs_hash_filename(const struct inode *dir, struct f2fs_filename *fname)
 {
@@ -110,10 +110,11 @@ 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.  Note
-                * that to handle encrypted directories, the fallback must use
-                * usr_fname (plaintext) rather than disk_name (ciphertext).
+                * should only be because the name wasn't valid Unicode or was
+                * "." or "..", so fall 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) {