From: Hin-Tak Leung <hintak.leung@gmail.com>
To: linux-fsdevel@vger.kernel.org
Cc: Hin-Tak Leung <htl10@users.sourceforge.net>
Subject: [RFC V0] Exminating the use of HFSPLUS_ATTR_MAX_STRLEN c.f. non-English attributes
Date: Thu, 10 Apr 2014 02:04:46 +0100 [thread overview]
Message-ID: <1397091886-29661-1-git-send-email-HinTak.Leung@gmail.com> (raw)
From: Hin-Tak Leung <htl10@users.sourceforge.net>
This isn't a real patch but just an RFC of examination of all usages of
HFSPLUS_ATTR_MAX_STRLEN .
Actually it turns out to be rather simple - except for
one use calling hfsplus_asc2uni (which should stay the same) and
one use calling hfsplus_uni2asc (which was changed in the just submitted
patch), all the other uses are of the forms:
1. char buffer[size]
2. bound check: "if (input_length > size) return failure;"
The buffer size change is simple enough - conversion between on-disk and char
(in whichever direction) always needs to be in the worst, so any char buffer
with that size almost all needs to have a NLS_MAX_CHARSET_SIZE x . They also
all need to switch to dynamic allocation (not done here yet, just for
simplicity ATM), and also possibly need to do kzalloc(?or null terminated
on first use?) since some of them are involved in strcpy.
(unlike the case of file names, which goes around with a length and
used as is).
The bound checks are all wrong. since they are all of comparing
nls_length to unicode_length_limit . changing them to comparing
nls_length to (unicode_length_limit * NLS_MAX) is rather stupid and useless.
So really, we want a cheap function to calculate the unicode length
of a given nls string, and do that instead of calling strlen() as many of these
functions do; and distinguish the two lengths where it matters.
Also, we have two other complications - sometimes the nls length is added to
XATTR_TRUSTED_PREFIX_LEN,XATTR_SECURITY_PREFIX_LEN,XATTR_MAC_OSX_PREFIX_LEN .
Can we assume that XATTR_TRUSTED_PREFIX, XATTR_SECURITY_PREFIX and
XATTR_MAC_OSX_PREFIX will forever stay Engish (i.e. their ascii length
is the same as their unicode length)?
Also with the switch to dynamic allocation, many of the routines
which formerly does
return hfsplus_{get,set}xattr(dentry, xattr_name ...)
needs to be turned into:
int res;
...
res = hfsplus_{get,set}xattr(dentry, xattr_name ...)
kfree(xattr_name);
return res;
As outlined above, all the changes are relatively mundane and tedious,
other than that of calculating the unicode length of an nls string.
Writing a wrapper around hfsplus_asc2uni is one possibility,
except it either returns a len or -ENAMETOOLONG, so we possibly
should convert all the length-based bound check to tests
for failure of conversion to unicode of length of
say HFSPLUS_ATTR_MAX_STRLEN - XATTR_MAC_OSX_PREFIX, and just
test for -ENAMETOOLONG or other failure instead.
Signed-off-by: Hin-Tak Leung <htl10@users.sourceforge.net>
---
fs/hfsplus/attributes.c | 2 +-
fs/hfsplus/xattr.c | 8 ++++----
fs/hfsplus/xattr_security.c | 12 ++++++------
fs/hfsplus/xattr_trusted.c | 8 ++++----
fs/hfsplus/xattr_user.c | 8 ++++----
5 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/fs/hfsplus/attributes.c b/fs/hfsplus/attributes.c
index caf89a7..86422bf 100644
--- a/fs/hfsplus/attributes.c
+++ b/fs/hfsplus/attributes.c
@@ -55,7 +55,7 @@ int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
key->attr.cnid = cpu_to_be32(cnid);
if (name) {
len = strlen(name);
- if (len > HFSPLUS_ATTR_MAX_STRLEN) {
+ if (len > NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN) {
pr_err("invalid xattr name's length\n");
return -EINVAL;
}
diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index 3034ce6..04b8f5b 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -805,14 +805,14 @@ end_removexattr:
static int hfsplus_osx_getxattr(struct dentry *dentry, const char *name,
void *buffer, size_t size, int type)
{
- char xattr_name[HFSPLUS_ATTR_MAX_STRLEN +
+ char xattr_name[NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
XATTR_MAC_OSX_PREFIX_LEN + 1] = {0};
size_t len = strlen(name);
if (!strcmp(name, ""))
return -EINVAL;
- if (len > HFSPLUS_ATTR_MAX_STRLEN)
+ if (len > NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN)
return -EOPNOTSUPP;
/*
@@ -828,14 +828,14 @@ static int hfsplus_osx_getxattr(struct dentry *dentry, const char *name,
static int hfsplus_osx_setxattr(struct dentry *dentry, const char *name,
const void *buffer, size_t size, int flags, int type)
{
- char xattr_name[HFSPLUS_ATTR_MAX_STRLEN +
+ char xattr_name[NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
XATTR_MAC_OSX_PREFIX_LEN + 1] = {0};
size_t len = strlen(name);
if (!strcmp(name, ""))
return -EINVAL;
- if (len > HFSPLUS_ATTR_MAX_STRLEN)
+ if (len > NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN)
return -EOPNOTSUPP;
/*
diff --git a/fs/hfsplus/xattr_security.c b/fs/hfsplus/xattr_security.c
index 0072276..4dfa15e 100644
--- a/fs/hfsplus/xattr_security.c
+++ b/fs/hfsplus/xattr_security.c
@@ -14,13 +14,13 @@
static int hfsplus_security_getxattr(struct dentry *dentry, const char *name,
void *buffer, size_t size, int type)
{
- char xattr_name[HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
+ char xattr_name[NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
size_t len = strlen(name);
if (!strcmp(name, ""))
return -EINVAL;
- if (len + XATTR_SECURITY_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN)
+ if (len + XATTR_SECURITY_PREFIX_LEN > NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN)
return -EOPNOTSUPP;
strcpy(xattr_name, XATTR_SECURITY_PREFIX);
@@ -32,13 +32,13 @@ static int hfsplus_security_getxattr(struct dentry *dentry, const char *name,
static int hfsplus_security_setxattr(struct dentry *dentry, const char *name,
const void *buffer, size_t size, int flags, int type)
{
- char xattr_name[HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
+ char xattr_name[NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
size_t len = strlen(name);
if (!strcmp(name, ""))
return -EINVAL;
- if (len + XATTR_SECURITY_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN)
+ if (len + XATTR_SECURITY_PREFIX_LEN > NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN)
return -EOPNOTSUPP;
strcpy(xattr_name, XATTR_SECURITY_PREFIX);
@@ -62,7 +62,7 @@ static int hfsplus_initxattrs(struct inode *inode,
void *fs_info)
{
const struct xattr *xattr;
- char xattr_name[HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
+ char xattr_name[NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
size_t xattr_name_len;
int err = 0;
@@ -73,7 +73,7 @@ static int hfsplus_initxattrs(struct inode *inode,
continue;
if (xattr_name_len + XATTR_SECURITY_PREFIX_LEN >
- HFSPLUS_ATTR_MAX_STRLEN)
+ NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN)
return -EOPNOTSUPP;
strcpy(xattr_name, XATTR_SECURITY_PREFIX);
diff --git a/fs/hfsplus/xattr_trusted.c b/fs/hfsplus/xattr_trusted.c
index 426cee2..965593b 100644
--- a/fs/hfsplus/xattr_trusted.c
+++ b/fs/hfsplus/xattr_trusted.c
@@ -12,13 +12,13 @@
static int hfsplus_trusted_getxattr(struct dentry *dentry, const char *name,
void *buffer, size_t size, int type)
{
- char xattr_name[HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
+ char xattr_name[NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
size_t len = strlen(name);
if (!strcmp(name, ""))
return -EINVAL;
- if (len + XATTR_TRUSTED_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN)
+ if (len + XATTR_TRUSTED_PREFIX_LEN > NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN)
return -EOPNOTSUPP;
strcpy(xattr_name, XATTR_TRUSTED_PREFIX);
@@ -30,13 +30,13 @@ static int hfsplus_trusted_getxattr(struct dentry *dentry, const char *name,
static int hfsplus_trusted_setxattr(struct dentry *dentry, const char *name,
const void *buffer, size_t size, int flags, int type)
{
- char xattr_name[HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
+ char xattr_name[NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
size_t len = strlen(name);
if (!strcmp(name, ""))
return -EINVAL;
- if (len + XATTR_TRUSTED_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN)
+ if (len + XATTR_TRUSTED_PREFIX_LEN > NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN)
return -EOPNOTSUPP;
strcpy(xattr_name, XATTR_TRUSTED_PREFIX);
diff --git a/fs/hfsplus/xattr_user.c b/fs/hfsplus/xattr_user.c
index e340165..8ead0640 100644
--- a/fs/hfsplus/xattr_user.c
+++ b/fs/hfsplus/xattr_user.c
@@ -12,13 +12,13 @@
static int hfsplus_user_getxattr(struct dentry *dentry, const char *name,
void *buffer, size_t size, int type)
{
- char xattr_name[HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
+ char xattr_name[NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
size_t len = strlen(name);
if (!strcmp(name, ""))
return -EINVAL;
- if (len + XATTR_USER_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN)
+ if (len + XATTR_USER_PREFIX_LEN > NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN)
return -EOPNOTSUPP;
strcpy(xattr_name, XATTR_USER_PREFIX);
@@ -30,13 +30,13 @@ static int hfsplus_user_getxattr(struct dentry *dentry, const char *name,
static int hfsplus_user_setxattr(struct dentry *dentry, const char *name,
const void *buffer, size_t size, int flags, int type)
{
- char xattr_name[HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
+ char xattr_name[NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1] = {0};
size_t len = strlen(name);
if (!strcmp(name, ""))
return -EINVAL;
- if (len + XATTR_USER_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN)
+ if (len + XATTR_USER_PREFIX_LEN > NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN)
return -EOPNOTSUPP;
strcpy(xattr_name, XATTR_USER_PREFIX);
--
1.9.0
next reply other threads:[~2014-04-10 1:04 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-10 1:04 Hin-Tak Leung [this message]
2014-04-10 7:34 ` [RFC V0] Exminating the use of HFSPLUS_ATTR_MAX_STRLEN c.f. non-English attributes Vyacheslav Dubeyko
2014-04-10 9:35 ` Anton Altaparmakov
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=1397091886-29661-1-git-send-email-HinTak.Leung@gmail.com \
--to=hintak.leung@gmail.com \
--cc=htl10@users.sourceforge.net \
--cc=linux-fsdevel@vger.kernel.org \
/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).