From: Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
