linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hin-Tak Leung <hintak.leung@gmail.com>
To: linux-fsdevel@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>
Cc: Anton Altaparmakov <aia21@cam.ac.uk>,
	Hin-Tak Leung <htl10@users.sourceforge.net>,
	Vyacheslav Dubeyko <slava@dubeyko.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>
Subject: [PATCH V3] hfsplus: fixes worst-case unicode to char conversion of file names and attributes
Date: Wed,  9 Apr 2014 22:53:32 +0100	[thread overview]
Message-ID: <1397080412-19127-1-git-send-email-HinTak.Leung@gmail.com> (raw)

From: Hin-Tak Leung <htl10@users.sourceforge.net>

The HFS Plus Volume Format specification (TN1150) states that
file names are stored internally as a maximum of 255 unicode
characters, as defined by The Unicode Standard, Version 2.0
[Unicode, Inc. ISBN 0-201-48345-9]. File names are converted by
the NLS system on Linux before presented to the user.

255 CJK characters converts to UTF-8 with 1 unicode character
to up to 3 bytes, and to GB18030 with 1 unicode character to
up to 4 bytes. Thus, trying in a UTF-8 locale to list files
with names of more than 85 CJK characters results in:

    $ ls /mnt
    ls: reading directory /mnt: File name too long

The receiving buffer to hfsplus_uni2asc() needs to be
255 x NLS_MAX_CHARSET_SIZE bytes, not 255 bytes as the code
has always been.

Similar consideration applies to attributes, which are stored
internally as a maximum of 127 UTF-16BE units. See XNU source for
an up-to-date reference on attributes.

Strictly speaking, the maximum value of NLS_MAX_CHARSET_SIZE = 6
is not attainable in the case of conversion to UTF-8, as going
beyond 3 bytes requires the use of surrogate pairs, i.e. consuming
two input units.

Thanks Anton Altaparmakov for reviewing an earlier version of
this change.

This patch fixes all callers of hfsplus_uni2asc(), and also enables
the use of long non-English file names in HFS+. The getting and
setting, and general usage of long non-English attributes
requires further forthcoming work.

Signed-off-by: Hin-Tak Leung <htl10@users.sourceforge.net>
Reviewed-by: Anton Altaparmakov <anton@tuxera.com>
CC: Vyacheslav Dubeyko <slava@dubeyko.com>
CC: Al Viro <viro@zeniv.linux.org.uk>
CC: Christoph Hellwig <hch@infradead.org>
---
 fs/hfsplus/dir.c   | 11 +++++++++--
 fs/hfsplus/xattr.c | 14 +++++++++++---
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index bdec665..fb07d26 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -12,6 +12,7 @@
 #include <linux/fs.h>
 #include <linux/slab.h>
 #include <linux/random.h>
+#include <linux/nls.h>
 
 #include "hfsplus_fs.h"
 #include "hfsplus_raw.h"
@@ -127,7 +128,7 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
 	struct inode *inode = file_inode(file);
 	struct super_block *sb = inode->i_sb;
 	int len, err;
-	char strbuf[HFSPLUS_MAX_STRLEN + 1];
+	char *strbuf;
 	hfsplus_cat_entry entry;
 	struct hfs_find_data fd;
 	struct hfsplus_readdir_data *rd;
@@ -139,6 +140,11 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
 	err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
 	if (err)
 		return err;
+	strbuf = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_MAX_STRLEN + 1, GFP_KERNEL);
+	if (!strbuf) {
+		err = -ENOMEM;
+		goto out;
+	}
 	hfsplus_cat_build_key(sb, fd.search_key, inode->i_ino, NULL);
 	err = hfs_brec_find(&fd, hfs_find_rec_by_key);
 	if (err)
@@ -193,7 +199,7 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
 		hfs_bnode_read(fd.bnode, &entry, fd.entryoffset,
 			fd.entrylength);
 		type = be16_to_cpu(entry.type);
-		len = HFSPLUS_MAX_STRLEN;
+		len = NLS_MAX_CHARSET_SIZE * HFSPLUS_MAX_STRLEN;
 		err = hfsplus_uni2asc(sb, &fd.key->cat.name, strbuf, &len);
 		if (err)
 			goto out;
@@ -246,6 +252,7 @@ next:
 	}
 	memcpy(&rd->key, fd.key, sizeof(struct hfsplus_cat_key));
 out:
+	kfree(strbuf);
 	hfs_find_exit(&fd);
 	return err;
 }
diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index 4e27edc..3034ce6 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -8,6 +8,7 @@
 
 #include "hfsplus_fs.h"
 #include <linux/posix_acl_xattr.h>
+#include <linux/nls.h>
 #include "xattr.h"
 #include "acl.h"
 
@@ -645,8 +646,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 	struct hfs_find_data fd;
 	u16 key_len = 0;
 	struct hfsplus_attr_key attr_key;
-	char strbuf[HFSPLUS_ATTR_MAX_STRLEN +
-			XATTR_MAC_OSX_PREFIX_LEN + 1] = {0};
+	char *strbuf;
 	int xattr_name_len;
 
 	if ((!S_ISREG(inode->i_mode) &&
@@ -666,6 +666,13 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 		return err;
 	}
 
+	strbuf = kzalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
+			XATTR_MAC_OSX_PREFIX_LEN + 1, GFP_KERNEL);
+	if (!strbuf) {
+		res = -ENOMEM;
+		goto out;
+	}
+
 	err = hfsplus_find_attr(inode->i_sb, inode->i_ino, NULL, &fd);
 	if (err) {
 		if (err == -ENOENT) {
@@ -692,7 +699,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 		if (be32_to_cpu(attr_key.cnid) != inode->i_ino)
 			goto end_listxattr;
 
-		xattr_name_len = HFSPLUS_ATTR_MAX_STRLEN;
+		xattr_name_len = NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN;
 		if (hfsplus_uni2asc(inode->i_sb,
 			(const struct hfsplus_unistr *)&fd.key->attr.key_name,
 					strbuf, &xattr_name_len)) {
@@ -718,6 +725,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 	}
 
 end_listxattr:
+	kfree(strbuf);
 	hfs_find_exit(&fd);
 	return res;
 }
-- 
1.9.0


             reply	other threads:[~2014-04-09 21:53 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-09 21:53 Hin-Tak Leung [this message]
2014-04-10  7:30 ` [PATCH V3] hfsplus: fixes worst-case unicode to char conversion of file names and attributes Vyacheslav Dubeyko
2014-04-10  9:33   ` Anton Altaparmakov
  -- strict thread matches above, loose matches on Subject: below --
2014-04-10 13:39 Hin-Tak Leung
2014-04-10 23:32 Hin-Tak Leung

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=1397080412-19127-1-git-send-email-HinTak.Leung@gmail.com \
    --to=hintak.leung@gmail.com \
    --cc=aia21@cam.ac.uk \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=htl10@users.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=slava@dubeyko.com \
    --cc=viro@zeniv.linux.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).