Re: [PATCH v6 01/10] libfs: Create the helper function generic_ci_validate_strict_name()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em 15/10/2024 12:59, Gabriel Krisman Bertazi escreveu:
André Almeida <andrealmeid@xxxxxxxxxx> writes:

+static inline bool generic_ci_validate_strict_name(struct inode *dir, struct qstr *name)
+{
+	if (!IS_CASEFOLDED(dir) || !sb_has_strict_encoding(dir->i_sb))
+		return true;
+
+	/*
+	 * A casefold dir must have a encoding set, unless the filesystem
+	 * is corrupted
+	 */
+	if (WARN_ON_ONCE(!dir->i_sb->s_encoding))
+		return true;
+
+	return utf8_validate(dir->i_sb->s_encoding, name);

There is something fishy here.  Concerningly, the fstests test doesn't
catch it.

utf8_validate is defined as:

   int utf8_validate(const struct unicode_map *um, const struct qstr *str)

Which returns 0 on success and !0 on error. Thus, when casting to bool,
the return code should be negated.

But generic/556 doesn't fail. That's because we are over cautious, and
also check the string at the end of generic_ci_d_hash.  So we never
really reach utf8_validate in the tested case.

But if you comment the final if in generic_ci_d_hash, you'll see this
patchset regresses the fstests case generic/556 over ext4.

We really need the check in both places, though.  We don't want to rely
on the behavior of generic_ci_d_hash to block invalid filenames, as that
might change.


Thanks Krisman! Nice catch. I fixed this for the next version. Testing with the modified generic_ci_d_hash(), I also realized that the validation was in the wrong place and leaving an inode behind, this fixed:

diff --git a/mm/shmem.c b/mm/shmem.c
index eb1ea1f3b37c..7bd7ca5777af 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3624,13 +3624,13 @@ shmem_mknod(struct mnt_idmap *idmap, struct inode *dir,
        struct inode *inode;
        int error;

+       if (!generic_ci_validate_strict_name(dir, &dentry->d_name))
+               return -EINVAL;
+
inode = shmem_get_inode(idmap, dir->i_sb, dir, mode, dev, VM_NORESERVE);
        if (IS_ERR(inode))
                return PTR_ERR(inode);

-       if (!generic_ci_validate_strict_name(dir, &dentry->d_name))
-               return -EINVAL;
-





[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux