mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Viacheslav Dubeyko <slava@dubeyko.com>
To: glaubitz@physik.fu-berlin.de, frank.li@vivo.com
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	vdubeyko@coreweave.com, Viacheslav Dubeyko <slava@dubeyko.com>
Subject: [PATCH 1/2] hfsplus: make hidden directory name independent from mount's nls
Date: Tue,  3 Nov 2026 19:46:04 -0800	[thread overview]
Message-ID: <20261104034605.1582293-2-slava@dubeyko.com> (raw)
In-Reply-To: <20261104034605.1582293-1-slava@dubeyko.com>

The catalog key of the hidden directory is built from
HFSP_HIDDENDIR_NAME, that is UTF-8 string, by means of the nls table
of the mount. hfsplus_fill_super() works around it by temporarily
replacing sbi->nls with utf8 table. However, hfsplus_reconfigure()
creates the hidden directory on remount read-write with the mount's
nls table. As a result, for example, with nls=cp437 the hidden
directory is created with garbled name that neither macOS nor
the next mount recognizes, and yet another hidden directory is
created on the next read-write mount. Temporary replacement of
sbi->nls is not an option in hfsplus_reconfigure() because lookups
are running concurrently and use sbi->nls.

Define the hidden directory name as constant Unicode string and
introduce hfsplus_create_cat_uni() that creates catalog records for
already converted name. Use hfsplus_cat_build_key_uni() for hidden
directory lookup and remove the temporary nls replacement from
hfsplus_fill_super(). The rollback in hfsplus_create_hidden_dir()
calls hfsplus_delete_cat() with NULL name, so the name is taken
from the thread record.

Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
---
 fs/hfsplus/catalog.c       | 83 ++++++++++++++++++++++++++++++--------
 fs/hfsplus/hfsplus_fs.h    |  5 +++
 fs/hfsplus/super.c         | 66 ++++++++++++++++--------------
 include/linux/hfs_common.h | 20 ++++++++-
 4 files changed, 124 insertions(+), 50 deletions(-)

diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index fe11c4b6dd99..5e53a978d420 100644
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -40,8 +40,8 @@ int hfsplus_cat_bin_cmp_key(const hfsplus_btree_key *k1,
 }
 
 /* Generates key for catalog file/folders record. */
-int hfsplus_cat_build_key(struct super_block *sb,
-		hfsplus_btree_key *key, u32 parent, const struct qstr *str)
+int hfsplus_cat_build_key(struct super_block *sb, hfsplus_btree_key *key,
+			  u32 parent, const struct qstr *str)
 {
 	int len, err;
 
@@ -58,15 +58,15 @@ int hfsplus_cat_build_key(struct super_block *sb,
 
 /* Generates key for catalog thread record. */
 void hfsplus_cat_build_key_with_cnid(struct super_block *sb,
-			hfsplus_btree_key *key, u32 parent)
+				     hfsplus_btree_key *key, u32 parent)
 {
 	key->cat.parent = cpu_to_be32(parent);
 	key->cat.name.length = 0;
 	key->key_len = cpu_to_be16(6);
 }
 
-static void hfsplus_cat_build_key_uni(hfsplus_btree_key *key, u32 parent,
-				      struct hfsplus_unistr *name)
+void hfsplus_cat_build_key_uni(hfsplus_btree_key *key, u32 parent,
+			       const struct hfsplus_unistr *name)
 {
 	int ustrlen;
 
@@ -103,7 +103,7 @@ void hfsplus_cat_set_perms(struct inode *inode, struct hfsplus_perm *perms)
 }
 
 static int hfsplus_cat_build_record(hfsplus_cat_entry *entry,
-		u32 cnid, struct inode *inode)
+				    u32 cnid, struct inode *inode)
 {
 	struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
 
@@ -187,7 +187,23 @@ static int hfsplus_fill_cat_thread(struct super_block *sb,
 	if (unlikely(err < 0))
 		return err;
 
-	return 10 + be16_to_cpu(entry->thread.nodeName.length) * 2;
+	return hfsplus_cat_thread_size(&entry->thread);
+}
+
+static int hfsplus_fill_cat_thread_uni(hfsplus_cat_entry *entry, int type,
+				       u32 parentid,
+				       const struct hfsplus_unistr *name)
+{
+	u16 len = be16_to_cpu(name->length);
+
+	entry->type = cpu_to_be16(type);
+	entry->thread.reserved = 0;
+	entry->thread.parentID = cpu_to_be32(parentid);
+	entry->thread.nodeName.length = name->length;
+	memcpy(entry->thread.nodeName.unicode, name->unicode,
+	       len * sizeof(hfsplus_unichr));
+
+	return hfsplus_cat_thread_size(&entry->thread);
 }
 
 /* Try to get a catalog entry for given catalog id */
@@ -250,17 +266,25 @@ static void hfsplus_subfolders_dec(struct inode *dir)
 	}
 }
 
-int hfsplus_create_cat(u32 cnid, struct inode *dir,
-		const struct qstr *str, struct inode *inode)
+/*
+ * Create catalog records for the name given either as @str (it is converted
+ * by means of the mount's nls table) or as already converted @name.
+ */
+static int __hfsplus_create_cat(u32 cnid, struct inode *dir,
+				const struct qstr *str,
+				const struct hfsplus_unistr *name,
+				struct inode *inode)
 {
 	struct super_block *sb = dir->i_sb;
 	struct hfs_find_data fd;
 	hfsplus_cat_entry entry;
+	int type;
 	int entry_size;
 	int err;
 
 	hfs_dbg("name %s, cnid %u, i_nlink %d\n",
-		str->name, cnid, inode->i_nlink);
+		str ? (const char *)str->name : "<unicode>",
+		cnid, inode->i_nlink);
 	err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
 	if (err)
 		return err;
@@ -274,10 +298,14 @@ int hfsplus_create_cat(u32 cnid, struct inode *dir,
 		goto err2;
 
 	hfsplus_cat_build_key_with_cnid(sb, fd.search_key, cnid);
-	entry_size = hfsplus_fill_cat_thread(sb, &entry,
-		S_ISDIR(inode->i_mode) ?
-			HFSPLUS_FOLDER_THREAD : HFSPLUS_FILE_THREAD,
-		dir->i_ino, str);
+	type = S_ISDIR(inode->i_mode) ?
+			HFSPLUS_FOLDER_THREAD : HFSPLUS_FILE_THREAD;
+	if (name)
+		entry_size = hfsplus_fill_cat_thread_uni(&entry, type,
+							 dir->i_ino, name);
+	else
+		entry_size = hfsplus_fill_cat_thread(sb, &entry, type,
+						     dir->i_ino, str);
 	if (unlikely(entry_size < 0)) {
 		err = entry_size;
 		goto err2;
@@ -293,9 +321,13 @@ int hfsplus_create_cat(u32 cnid, struct inode *dir,
 	if (err)
 		goto err2;
 
-	err = hfsplus_cat_build_key(sb, fd.search_key, dir->i_ino, str);
-	if (unlikely(err))
-		goto err1;
+	if (name)
+		hfsplus_cat_build_key_uni(fd.search_key, dir->i_ino, name);
+	else {
+		err = hfsplus_cat_build_key(sb, fd.search_key, dir->i_ino, str);
+		if (unlikely(err))
+			goto err1;
+	}
 
 	entry_size = hfsplus_cat_build_record(&entry, cnid, inode);
 	err = hfs_brec_find(&fd, hfs_find_rec_by_key);
@@ -328,6 +360,23 @@ int hfsplus_create_cat(u32 cnid, struct inode *dir,
 	return err;
 }
 
+int hfsplus_create_cat(u32 cnid, struct inode *dir,
+			const struct qstr *str, struct inode *inode)
+{
+	return __hfsplus_create_cat(cnid, dir, str, NULL, inode);
+}
+
+/*
+ * Create catalog records for the name that is independent from
+ * the mount's nls table (for example, the hidden directory name).
+ */
+int hfsplus_create_cat_uni(u32 cnid, struct inode *dir,
+			   const struct hfsplus_unistr *name,
+			   struct inode *inode)
+{
+	return __hfsplus_create_cat(cnid, dir, NULL, name, inode);
+}
+
 int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str)
 {
 	struct super_block *sb = dir->i_sb;
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 6f240f87f8ff..e2d3f488142b 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -418,11 +418,16 @@ int hfsplus_cat_build_key(struct super_block *sb, hfsplus_btree_key *key,
 			   u32 parent, const struct qstr *str);
 void hfsplus_cat_build_key_with_cnid(struct super_block *sb,
 				     hfsplus_btree_key *key, u32 parent);
+void hfsplus_cat_build_key_uni(hfsplus_btree_key *key, u32 parent,
+			       const struct hfsplus_unistr *name);
 void hfsplus_cat_set_perms(struct inode *inode, struct hfsplus_perm *perms);
 int hfsplus_find_cat(struct super_block *sb, u32 cnid,
 		     struct hfs_find_data *fd);
 int hfsplus_create_cat(u32 cnid, struct inode *dir, const struct qstr *str,
 		       struct inode *inode);
+int hfsplus_create_cat_uni(u32 cnid, struct inode *dir,
+			   const struct hfsplus_unistr *name,
+			   struct inode *inode);
 int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str);
 int hfsplus_rename_cat(u32 cnid, struct inode *src_dir, const struct qstr *src_name,
 		       struct inode *dst_dir, const struct qstr *dst_name);
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 8c65741f2cf2..57858d8685ff 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -399,6 +399,28 @@ static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
+/*
+ * On-disk UTF-16 name of the hidden directory: HFSP_HIDDENDIR_NUL_COUNT
+ * U+0000 (NUL) followed by HFSP_HIDDENDIR_SUFFIX. HFSP_HIDDENDIR_NAME_UTF8
+ * is the Linux representation of this name, where every NUL is replaced
+ * by U+2400 (SYMBOL FOR NULL). The catalog key of the hidden directory
+ * must not depend on the nls table of the mount.
+ */
+static const struct hfsplus_unistr hfsplus_hidden_dir_name_utf16 = {
+	.length = cpu_to_be16(HFSP_HIDDENDIR_NAME_UTF16_LEN),
+	.unicode = {
+		/* unicode[0 .. HFSP_HIDDENDIR_NUL_COUNT - 1] are U+0000 */
+		[HFSP_HIDDENDIR_NUL_COUNT] =
+		cpu_to_be16('H'), cpu_to_be16('F'), cpu_to_be16('S'),
+		cpu_to_be16('+'), cpu_to_be16(' '),
+		cpu_to_be16('P'), cpu_to_be16('r'), cpu_to_be16('i'),
+		cpu_to_be16('v'), cpu_to_be16('a'), cpu_to_be16('t'),
+		cpu_to_be16('e'), cpu_to_be16(' '),
+		cpu_to_be16('D'), cpu_to_be16('a'), cpu_to_be16('t'),
+		cpu_to_be16('a'),
+	},
+};
+
 /*
  * Create the hidden directory that keeps the indirect node files
  * of hard links. The caller is responsible for cancelling the
@@ -406,11 +428,12 @@ static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf)
  * set to true if the catalog file could be left in inconsistent state.
  */
 static int hfsplus_create_hidden_dir(struct super_block *sb,
-				     const struct qstr *str,
 				     bool *cat_damaged)
 {
 	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
 	struct inode *root = d_inode(sb->s_root);
+	const struct qstr str = QSTR_INIT(HFSP_HIDDENDIR_NAME_UTF8,
+					  HFSP_HIDDENDIR_NAME_UTF8_LEN);
 	struct inode *inode;
 	int err;
 
@@ -423,15 +446,17 @@ static int hfsplus_create_hidden_dir(struct super_block *sb,
 		goto out_unlock;
 	}
 
-	err = hfsplus_create_cat(inode->i_ino, root, str, inode);
+	err = hfsplus_create_cat_uni(inode->i_ino, root,
+				     &hfsplus_hidden_dir_name_utf16, inode);
 	if (err)
 		goto out_put_inode;
 
-	err = hfsplus_init_security(inode, root, str);
+	err = hfsplus_init_security(inode, root, &str);
 	if (err == -EOPNOTSUPP)
 		err = 0; /* Operation is not supported. */
 	else if (err) {
-		if (hfsplus_delete_cat(inode->i_ino, root, str)) {
+		/* The name is taken from the thread record. */
+		if (hfsplus_delete_cat(inode->i_ino, root, NULL)) {
 			pr_err("unable to remove hidden directory record\n");
 			*cat_damaged = true;
 		}
@@ -520,11 +545,9 @@ static int hfsplus_reconfigure(struct fs_context *fc)
 	 * It could be absent if the volume was mounted read-only.
 	 */
 	if (!sbi->hidden_dir) {
-		struct qstr str = QSTR_INIT(HFSP_HIDDENDIR_NAME,
-					    sizeof(HFSP_HIDDENDIR_NAME) - 1);
 		bool cat_damaged;
 
-		err = hfsplus_create_hidden_dir(sb, &str, &cat_damaged);
+		err = hfsplus_create_hidden_dir(sb, &cat_damaged);
 		if (err) {
 			pr_err("unable to create hidden directory: err %d\n",
 			       err);
@@ -562,7 +585,6 @@ void hfsplus_prepare_volume_header_for_commit(struct hfsplus_vh *vhdr)
 }
 
 static inline int hfsplus_get_hidden_dir_entry(struct super_block *sb,
-					       const struct qstr *str,
 					       hfsplus_cat_entry *entry)
 {
 	struct hfs_find_data fd;
@@ -572,15 +594,13 @@ static inline int hfsplus_get_hidden_dir_entry(struct super_block *sb,
 	if (unlikely(err))
 		return err;
 
-	err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, str);
-	if (unlikely(err))
-		goto free_fd;
+	hfsplus_cat_build_key_uni(fd.search_key, HFSPLUS_ROOT_CNID,
+				  &hfsplus_hidden_dir_name_utf16);
 
 	err = hfsplus_brec_read_cat(&fd, entry);
 	if (err)
 		err = -ENOENT;
 
-free_fd:
 	hfs_find_exit(&fd);
 	return err;
 }
@@ -591,8 +611,6 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
 	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
 	hfsplus_cat_entry entry;
 	struct inode *root, *inode;
-	struct qstr str;
-	struct nls_table *nls;
 	u64 last_fs_block, last_fs_page;
 	int silent = fc->sb_flags & SB_SILENT;
 	int err;
@@ -610,19 +628,11 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
 			sbi->nls = load_nls_default();
 	}
 
-	/* temporarily use utf8 to correctly find the hidden dir below */
-	nls = sbi->nls;
-	sbi->nls = load_nls("utf8");
-	if (!sbi->nls) {
-		pr_err("unable to load nls for utf8\n");
-		goto out_unload_nls;
-	}
-
 	/* Grab the volume header */
 	if (hfsplus_read_wrapper(sb)) {
 		if (!silent)
 			pr_warn("unable to find HFS+ superblock\n");
-		goto out_unload_nls;
+		return err;
 	}
 	vhdr = sbi->s_vhdr;
 
@@ -723,9 +733,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
 		goto out_put_alloc_file;
 	}
 
-	str = (struct qstr)QSTR_INIT(HFSP_HIDDENDIR_NAME,
-				     sizeof(HFSP_HIDDENDIR_NAME) - 1);
-	err = hfsplus_get_hidden_dir_entry(sb, &str, &entry);
+	err = hfsplus_get_hidden_dir_entry(sb, &entry);
 	if (err == -ENOENT) {
 		/*
 		 * Hidden directory is absent or it cannot be read.
@@ -756,14 +764,12 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
 		if (!sbi->hidden_dir) {
 			bool cat_damaged;
 
-			err = hfsplus_create_hidden_dir(sb, &str, &cat_damaged);
+			err = hfsplus_create_hidden_dir(sb, &cat_damaged);
 			if (err)
 				goto out_cancel_sync_work;
 		}
 	}
 
-	unload_nls(sbi->nls);
-	sbi->nls = nls;
 	return 0;
 
 out_cancel_sync_work:
@@ -782,8 +788,6 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
 out_free_vhdr:
 	kfree(sbi->s_vhdr_buf);
 	kfree(sbi->s_backup_vhdr_buf);
-out_unload_nls:
-	unload_nls(nls);
 	return err;
 }
 
diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
index d6a615e74b26..8a7bc9763eb9 100644
--- a/include/linux/hfs_common.h
+++ b/include/linux/hfs_common.h
@@ -57,8 +57,24 @@
 
 #define HFSP_MOUNT_VERSION		0x482b4c78	/* 'H+Lx' */
 
-#define HFSP_HIDDENDIR_NAME \
-	"\xe2\x90\x80\xe2\x90\x80\xe2\x90\x80\xe2\x90\x80HFS+ Private Data"
+/*
+ * The name of the hidden directory is HFSP_HIDDENDIR_NUL_COUNT U+0000
+ * characters followed by HFSP_HIDDENDIR_SUFFIX. On disk, it is stored
+ * in UTF-16 and has HFSP_HIDDENDIR_NAME_UTF16_LEN characters. In Linux,
+ * it is HFSP_HIDDENDIR_NAME_UTF8, where every U+0000 is represented by
+ * U+2400 (SYMBOL FOR NULL); it has HFSP_HIDDENDIR_NAME_UTF8_LEN bytes.
+ */
+#define HFSP_HIDDENDIR_NUL_COUNT	4
+#define HFSP_HIDDENDIR_SUFFIX		"HFS+ Private Data"
+#define HFSP_HIDDENDIR_SUFFIX_LEN	(sizeof(HFSP_HIDDENDIR_SUFFIX) - 1)
+#define HFSP_HIDDENDIR_NAME_UTF16_LEN \
+	(HFSP_HIDDENDIR_NUL_COUNT + HFSP_HIDDENDIR_SUFFIX_LEN)
+#define HFSP_SYMBOL_FOR_NULL_UTF8	"\xe2\x90\x80"
+#define HFSP_HIDDENDIR_NAME_UTF8 \
+	(HFSP_SYMBOL_FOR_NULL_UTF8 HFSP_SYMBOL_FOR_NULL_UTF8 \
+	 HFSP_SYMBOL_FOR_NULL_UTF8 HFSP_SYMBOL_FOR_NULL_UTF8 \
+	 HFSP_HIDDENDIR_SUFFIX)
+#define HFSP_HIDDENDIR_NAME_UTF8_LEN	(sizeof(HFSP_HIDDENDIR_NAME_UTF8) - 1)
 
 /* various FIXED size parameters */
 #define HFS_SECTOR_SIZE			512	/* size of an HFS sector */
-- 
2.43.0


  reply	other threads:[~2026-11-04  3:46 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04  3:46 [PATCH 0/2] hfsplus: make hidden directory name independent from nls Viacheslav Dubeyko
2026-11-04  3:46 ` Viacheslav Dubeyko [this message]
2026-11-04  3:46 ` [PATCH 2/2] hfsplus: add KUnit check for hidden directory name Viacheslav Dubeyko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261104034605.1582293-2-slava@dubeyko.com \
    --to=slava@dubeyko.com \
    --cc=frank.li@vivo.com \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vdubeyko@coreweave.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®