linux-kernel - [RESEND PATCH v2] hfsplus: prevent negative dentries when casefolded

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210716073635.1613671-1-cccheng@synology.com>
Date:   Fri, 16 Jul 2021 15:36:35 +0800
From:   Chung-Chiang Cheng <cccheng@...ology.com>
To:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        viro@...iv.linux.org.uk, akpm@...ux-foundation.org,
        slava@...eyko.com, gustavoars@...nel.org,
        gregkh@...uxfoundation.org, keescook@...omium.org,
        mszeredi@...hat.com
Cc:     Chung-Chiang Cheng <cccheng@...ology.com>
Subject: [RESEND PATCH v2] hfsplus: prevent negative dentries when casefolded

hfsplus uses the case-insensitive filenames by default, but VFS negative
dentries are incompatible with case-insensitive. For example, the
following instructions will get a cached filename 'aaa' which isn't
expected. There is no such problem in macOS.

  touch aaa
  rm aaa
  touch AAA

This patch takes the same approach to drop negative dentires as vfat does.
The dentry is revalidated without blocking and storing to the dentry,
and should be safe in rcu-walk.

Signed-off-by: Chung-Chiang Cheng <cccheng@...ology.com>
---
 fs/hfsplus/hfsplus_fs.h |  1 +
 fs/hfsplus/inode.c      |  1 +
 fs/hfsplus/unicode.c    | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 1798949f269b..4ae7f1ca1584 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -520,6 +520,7 @@ int hfsplus_asc2uni(struct super_block *sb, struct hfsplus_unistr *ustr,
 int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str);
 int hfsplus_compare_dentry(const struct dentry *dentry, unsigned int len,
 			   const char *str, const struct qstr *name);
+int hfsplus_revalidate_dentry(struct dentry *dentry, unsigned int flags);
 
 /* wrapper.c */
 int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 6fef67c2a9f0..4188a0760118 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -179,6 +179,7 @@ const struct address_space_operations hfsplus_aops = {
 const struct dentry_operations hfsplus_dentry_operations = {
 	.d_hash       = hfsplus_hash_dentry,
 	.d_compare    = hfsplus_compare_dentry,
+	.d_revalidate = hfsplus_revalidate_dentry,
 };
 
 static void hfsplus_get_perms(struct inode *inode,
diff --git a/fs/hfsplus/unicode.c b/fs/hfsplus/unicode.c
index 73342c925a4b..e336631334eb 100644
--- a/fs/hfsplus/unicode.c
+++ b/fs/hfsplus/unicode.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/types.h>
+#include <linux/namei.h>
 #include <linux/nls.h>
 #include "hfsplus_fs.h"
 #include "hfsplus_raw.h"
@@ -518,3 +519,34 @@ int hfsplus_compare_dentry(const struct dentry *dentry,
 		return 1;
 	return 0;
 }
+
+int hfsplus_revalidate_dentry(struct dentry *dentry, unsigned int flags)
+{
+	/*
+	 * dentries are always valid when disabling casefold.
+	 */
+	if (!test_bit(HFSPLUS_SB_CASEFOLD, &HFSPLUS_SB(dentry->d_sb)->flags))
+		return 1;
+
+	/*
+	 * Positive dentries are valid when enabling casefold.
+	 *
+	 * Note, rename() to existing directory entry will have ->d_inode, and
+	 * will use existing name which isn't specified name by user.
+	 *
+	 * We may be able to drop this positive dentry here. But dropping
+	 * positive dentry isn't good idea. So it's unsupported like
+	 * rename("filename", "FILENAME") for now.
+	 */
+	if (d_really_is_positive(dentry))
+		return 1;
+
+	/*
+	 * Drop the negative dentry, in order to make sure to use the case
+	 * sensitive name which is specified by user if this is for creation.
+	 */
+	if (flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET))
+		return 0;
+
+	return 1;
+}
-- 
2.25.1