Hi Anton and Vyacheslav, I think I have reached the end of this unicode fix journey. It turned out that all the bound checks (and all the try_asc2uni() code) can be removed. Because there is only one usage asc2uni() as far as attributes are concerned, and that's the one in hfsplus_attr_build_key() which Anton mentioned and which I had corrected. build_key() is used by find_attr() and also create_attr(), which, as far as I can see, is used by everything else. And it does make sense that there are only two uses: find an existing attribute (possibly as a precursor to removal) or create a new one. However, I also uncovered a whole bunch of potential problems with where error from asc2uni() in the catalog code is uncaught. So I like to ask you two to help critically check these ideas: - Is it true that all usages of attribute length check eventually lead to the single one in hfsplus_attr_build_key(), via find/create? - I like to know how to provoke the error in the unpatched catalog code, for testing purposes. - In general, the error recovery for that whole bunch of potential problems with asc2uni in the catalog code seems obvious - I just make it goto wherever neighbouring errors goto, even when I don't completely understand how to provoke the error and/or what the error might look like in real usage. There is one particular possible error in the catalog code I am not sure about. at the end of hfsplus_create_cat() in catalog.c, after the "err1:" label. If it fails, I just make it by-pass the next statement to go to err2. I think it makes sense that way but I'd like another pair of eyes. So it is going to be a series of 4 patches: - hfsplus: fixes worst-case unicode to char conversion of file names and attributes (about uni2asc) - Correct usage of HFSPLUS_ATTR_MAX_STRLEN for non-English attributes - remove unused hfsplus_attr_build_key_uni() - fixes error propagation of hfsplus_asc2uni It seems to have come full circle - from checking problems with uni2asc, to checking problems with hfsplus_asc2uni. The 3rd patch is just unused attribute code I think should be removed since I cannot see a future use of it. It seems to be just copied from the catalog code which has wider use. I found it easier to fix by API (essentially, a grep, followed by looking at where they are, and further grep), although, by purpose, these are two areas, names vs attributes, and for back-port purposes one which divide these patches differently. Even by API, the division is not unambiguous, since one use of uni2asc involves incorrect use of HFSPLUS_ATTR_MAX_STRLEN, and one error propagation of hfsplus_asc2uni() is also fixed in correcting usage of HFSPLUS_ATTR_MAX_STRLEN. But that's how I like to divide them (i.e. "mostly" by API in a historical context). I think dividing by name vs attributes would not be clearer, even though for back-porting purpose it would be better that way, for older kernels without attribute support. Patches to follow, after doing some extended write-up of relevant commit messages etc. Hin-Tak ------------------------------ On Sun, Apr 13, 2014 1:52 AM BST Hin-Tak Leung wrote: >Hi Anton, > >Thanks again for a rather thoughtful review. > >I do completely agree, that the bound checks are almost completely useless before and after - before >the change they were simply wrong, after the change they are simply doing twice the work, just earlier. > >There is no need to do twice the work - if hfsplus_asc2uni() works correctly, and failures are caught properly, >there is no need to try() it first. So that's why I simply removed the one in hfsplus_attr_build_key(),since >the check happens immediately before the actual call. It is definitely stupid doing a try() then immediately >the real thing. > >But there is one advantage of doing it early - assuming try() is cheap (may or may not be the case): >the real hfsplus_asc2uni() is buried quite a lot further after setting up the atttribute B-tree for >searches, etc. So by the time the actual call happens, throwing away all that work B-tree >find_init, etc might be wasteful; besides that stuff also have mutex locks around it, etc, so >unwinding and catch errors there may not be as easy. > >In principle, I agree that if any error from hfsplus_asc2uni() is checked and caught, there >is no need to do a try() first; the practical dificulty is just that the real >hfsplus_asc2uni() is buried rather deep, and within at least one mutex lock. > >So I did think about the first approach you suggested, just thought it is much harder >- the current patch is just too boring and not doing anything risky, so it is safe to use. I think the >2nd approach (keeping the try() result) would get around my worry about that mutex lock, so >I may look into that. > >Now I have an "tedious, slow but working correctly how it should be" patch, for reference, and >it is a good thing to have. - it is quite educational to try to use all 127 unicode character >limit of the attribute - and not possible to do that in a UTF-8 locale. Both the userland tools (xttr) >and many of the other file systems have a 255 byte limit for EA's (in nls, and 127 >unicode char can be as bad as 127 x 3 = 381 in UTF-8). I had to use an explicit nls=<not-utf-8> >mount option and run my terminal/content at a different encoding to fully use >that 127 unit storage. > >When a Mac user had already written an unusually long attribute, and we try to >read it under linux - it currently (without this patch) gives a scary I/O error. That's >really not good. > >i have thought of a genuine (mis-) use of non-english attributes. All the english >alphabets have their "half/full-width" equivalent at a higher >code point. ( http://en.wikipedia.org/wiki/Halfwidth_and_fullwidth_forms ). >It is entirely possible for a japanese person to be typing what he thought >is english, and looks like english, and read like english - only stretched out >or shrunk - but not actually ascii. > >I''ll have a go at ripping all the useless bound checks out, and >see how painful it gets. > >Hin-Tak > >------------------------------ >On Sun, Apr 13, 2014 12:01 AM BST Anton Altaparmakov wrote: > >>Hi Hin-Tak, >> >>I think you are taking a sub-optimal approach! Think about it: you are now performing the hfsplus_asc2uni() twice for each name being handled! Once to check if the length is ok and once to get the actual result. This is a waste of time. >> >>Instead, just do the conversion. If the name is longer than allowed, hfsplus_asc2uni() will return -ENAMETOOLONG and at that point you can bail out. There is no need to do a "test conversion" first the result of which you through away (and even worse you do a kmalloc() twice - once for the "test" and once for the real thing)... >> >>In fact you do it correctly in your proposed patch in hfsplus_attr_build_key(): You thrown out the length check and replace it with a call to hfsplus_asc2uni() that returns -ENAMETOOLONG if the name is too long and thus hfsplus_attr_build_key() returns -ENAMETOOLONG. All you need to do is change the code to do the Right Thing when it gets back -ENAMETOOLONG and you are done (and you throw away all the bogus length checks in the upper layers). >> >>If you definitely want to do checking in advance, then that is fine, too - but don't do it with a test conversion - instead, convert it, check the length of the converted string and then use the converted name from there onwards. >> >>Both my suggestions are perhaps more involved changes than what you are doing but I think they are much more worthwhile as they do not introduce double the allocations and conversions. >> >>Best regards, >> >> Anton >> >>On 12 Apr 2014, at 04:26, Hin-Tak Leung <hintak.leung@xxxxxxxxx> wrote: >> >> From: Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx> >> >> HFSPLUS_ATTR_MAX_STRLEN (=127) is a limit for the number of unicode character >> (UTF-16BE) storable in the HFS+ file system. Almost all the current >> usage of it is wrong in relation to NLS to on-disk conversion. >> >> Except for one use calling hfsplus_asc2uni (which should stay the same) and >> its uses in calling hfsplus_uni2asc (which was changed in an earlier patch), >> all the other uses are of the forms: >> >> - char buffer[size] >> >> - bound check: "if (input_length > size) return failure;" >> >> Conversion between on-disk and NLS char strings (in whichever direction) >> always needs to be in the worst, so all char buffers of that size >> need to have a NLS_MAX_CHARSET_SIZE x . >> >> The bound checks are all wrong, since they all compare >> nls_length to unicode_length_limit. >> >> A new function, hfsplus_try_asc2uni(), is introduced to >> try the conversion, to do the bound-check instead. (After review this >> will be splitted off as a separate earlier patch). >> >> Before this patch, trying to set a 55 CJK character (in a UTF-8 locale) >> + user prefix fails with: >> >> $ setfattr -n user.`cat testing-string` -v `cat testing-string` testing-string >> setfattr: testing-string: Operation not supported >> >> and getting long attributes is particular ugly(!): >> >> find /mnt/* -type f -exec getfattr -d {} \; >> getfattr: /mnt/testing-string: Input/output error >> >> with console log: >> [268008.389781] hfsplus: unicode conversion failed >> >> After the patch, both of the above works. >> >> However, I was not able to set double the size (110 + 5 is still under 127): >> >> $setfattr -n user.`cat testing-string testing-string` -v `cat testing-string testing-string` testing-string >> setfattr: testing-string: Numerical result out of range >> >> But this also fails on ext2, so this failure is not HFS+ specific. It seems >> that either the command line or the system call itself has a 256-byte limit. >> (110 CJK char in UTF-8 is 330 bytes). >> >> Signed-off-by: Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx> >> --- >> fs/hfsplus/attributes.c | 11 ++++----- >> fs/hfsplus/hfsplus_fs.h | 7 ++++++ >> fs/hfsplus/unicode.c | 18 +++++++++++++++ >> fs/hfsplus/xattr.c | 40 ++++++++++++++++++++++---------- >> fs/hfsplus/xattr_security.c | 56 ++++++++++++++++++++++++++++++--------------- >> fs/hfsplus/xattr_trusted.c | 36 +++++++++++++++++++++-------- >> fs/hfsplus/xattr_user.c | 36 +++++++++++++++++++++-------- >> 7 files changed, 147 insertions(+), 57 deletions(-) >> >> diff --git a/fs/hfsplus/attributes.c b/fs/hfsplus/attributes.c >> index caf89a7..f3345c0 100644 >> --- a/fs/hfsplus/attributes.c >> +++ b/fs/hfsplus/attributes.c >> @@ -54,14 +54,11 @@ int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key, >> memset(key, 0, sizeof(struct hfsplus_attr_key)); >> key->attr.cnid = cpu_to_be32(cnid); >> if (name) { >> - len = strlen(name); >> - if (len > HFSPLUS_ATTR_MAX_STRLEN) { >> - pr_err("invalid xattr name's length\n"); >> - return -EINVAL; >> - } >> - hfsplus_asc2uni(sb, >> + int res = hfsplus_asc2uni(sb, >> (struct hfsplus_unistr *)&key->attr.key_name, >> - HFSPLUS_ATTR_MAX_STRLEN, name, len); >> + HFSPLUS_ATTR_MAX_STRLEN, name, strlen(name)); >> + if (res) >> + return res; >> len = be16_to_cpu(key->attr.key_name.length); >> } else { >> key->attr.key_name.length = 0; >> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h >> index 83dc292..80257d0 100644 >> --- a/fs/hfsplus/hfsplus_fs.h >> +++ b/fs/hfsplus/hfsplus_fs.h >> @@ -507,6 +507,13 @@ int hfsplus_uni2asc(struct super_block *, >> const struct hfsplus_unistr *, char *, int *); >> int hfsplus_asc2uni(struct super_block *, >> struct hfsplus_unistr *, int, const char *, int); >> +int hfsplus_try_asc2uni_sb(struct super_block *sb, >> + const char *nls_name, int unilimit); >> +static inline int hfsplus_try_asc2uni(const struct dentry *dentry, >> + const char *nls_name, int unilimit) >> +{ >> + return hfsplus_try_asc2uni_sb(dentry->d_sb, nls_name, unilimit); >> +} >> int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str); >> int hfsplus_compare_dentry(const struct dentry *parent, const struct dentry *dentry, >> unsigned int len, const char *str, const struct qstr *name); >> diff --git a/fs/hfsplus/unicode.c b/fs/hfsplus/unicode.c >> index e8ef121..cd00bd2 100644 >> --- a/fs/hfsplus/unicode.c >> +++ b/fs/hfsplus/unicode.c >> @@ -330,6 +330,24 @@ int hfsplus_asc2uni(struct super_block *sb, >> } >> >> /* >> + * Try encoding an nls_name within a unicode size limit, >> + * and see whether it will fit. >> + */ >> +int hfsplus_try_asc2uni_sb(struct super_block *sb, >> + const char *nls_name, int unilimit) >> +{ >> + int res = 0; >> + struct hfsplus_unistr *ustr = kmalloc(sizeof(*ustr), GFP_KERNEL); >> + if (!ustr) >> + return -ENOMEM; >> + >> + res = hfsplus_asc2uni(sb, >> + ustr, unilimit, nls_name, strlen(nls_name)); >> + kfree(ustr); >> + return res; >> +} >> + >> +/* >> * Hash a string to an integer as appropriate for the HFS+ filesystem. >> * Composed unicode characters are decomposed and case-folding is performed >> * if the appropriate bits are (un)set on the superblock. >> diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c >> index 40c0a63..a05cab9 100644 >> --- a/fs/hfsplus/xattr.c >> +++ b/fs/hfsplus/xattr.c >> @@ -805,15 +805,15 @@ 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 + >> - XATTR_MAC_OSX_PREFIX_LEN + 1] = {0}; >> - size_t len = strlen(name); >> + char *xattr_name; >> + int res; >> >> if (!strcmp(name, ")) >> return -EINVAL; >> >> - if (len > HFSPLUS_ATTR_MAX_STRLEN) >> - return -EOPNOTSUPP; >> + res = hfsplus_try_asc2uni(dentry, name, HFSPLUS_ATTR_MAX_STRLEN); >> + if (res) >> + return res; >> >> /* >> * Don't allow retrieving properly prefixed attributes >> @@ -821,22 +821,30 @@ static int hfsplus_osx_getxattr(struct dentry *dentry, const char *name, >> */ >> if (is_known_namespace(name)) >> return -EOPNOTSUPP; >> + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN >> + + XATTR_MAC_OSX_PREFIX_LEN + 1, GFP_KERNEL); >> + if (!xattr_name) >> + return -ENOMEM; >> + strcpy(xattr_name, XATTR_MAC_OSX_PREFIX); >> + strcpy(xattr_name + XATTR_MAC_OSX_PREFIX_LEN, name); >> >> - return hfsplus_getxattr(dentry, xattr_name, buffer, size); >> + res = hfsplus_getxattr(dentry, xattr_name, buffer, size); >> + kfree(xattr_name); >> + return res; >> } >> >> 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 + >> - XATTR_MAC_OSX_PREFIX_LEN + 1] = {0}; >> - size_t len = strlen(name); >> + char *xattr_name; >> + int res; >> >> if (!strcmp(name, ")) >> return -EINVAL; >> >> - if (len > HFSPLUS_ATTR_MAX_STRLEN) >> - return -EOPNOTSUPP; >> + res = hfsplus_try_asc2uni(dentry, name, HFSPLUS_ATTR_MAX_STRLEN); >> + if (res) >> + return res; >> >> /* >> * Don't allow setting properly prefixed attributes >> @@ -844,8 +852,16 @@ static int hfsplus_osx_setxattr(struct dentry *dentry, const char *name, >> */ >> if (is_known_namespace(name)) >> return -EOPNOTSUPP; >> + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN >> + + XATTR_MAC_OSX_PREFIX_LEN + 1, GFP_KERNEL); >> + if (!xattr_name) >> + return -ENOMEM; >> + strcpy(xattr_name, XATTR_MAC_OSX_PREFIX); >> + strcpy(xattr_name + XATTR_MAC_OSX_PREFIX_LEN, name); >> >> - return hfsplus_setxattr(dentry, xattr_name, buffer, size, flags); >> + res = hfsplus_setxattr(dentry, xattr_name, buffer, size, flags); >> + kfree(xattr_name); >> + return res; >> } >> >> static size_t hfsplus_osx_listxattr(struct dentry *dentry, char *list, >> diff --git a/fs/hfsplus/xattr_security.c b/fs/hfsplus/xattr_security.c >> index 0072276..f4993e0 100644 >> --- a/fs/hfsplus/xattr_security.c >> +++ b/fs/hfsplus/xattr_security.c >> @@ -14,37 +14,53 @@ >> 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}; >> - size_t len = strlen(name); >> + char *xattr_name; >> + int res; >> >> if (!strcmp(name, ")) >> return -EINVAL; >> >> - if (len + XATTR_SECURITY_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN) >> - return -EOPNOTSUPP; >> + res = hfsplus_try_asc2uni(dentry, name, >> + HFSPLUS_ATTR_MAX_STRLEN - XATTR_SECURITY_PREFIX_LEN); >> + if (res) >> + return res; >> >> + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, >> + GFP_KERNEL); >> + if (!xattr_name) >> + return -ENOMEM; >> strcpy(xattr_name, XATTR_SECURITY_PREFIX); >> strcpy(xattr_name + XATTR_SECURITY_PREFIX_LEN, name); >> >> - return hfsplus_getxattr(dentry, xattr_name, buffer, size); >> + res = hfsplus_getxattr(dentry, xattr_name, buffer, size); >> + kfree(xattr_name); >> + return res; >> } >> >> 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}; >> - size_t len = strlen(name); >> + char *xattr_name; >> + int res; >> >> if (!strcmp(name, ")) >> return -EINVAL; >> >> - if (len + XATTR_SECURITY_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN) >> - return -EOPNOTSUPP; >> + res = hfsplus_try_asc2uni(dentry, name, >> + HFSPLUS_ATTR_MAX_STRLEN - XATTR_SECURITY_PREFIX_LEN); >> + if (res) >> + return res; >> >> + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, >> + GFP_KERNEL); >> + if (!xattr_name) >> + return -ENOMEM; >> strcpy(xattr_name, XATTR_SECURITY_PREFIX); >> strcpy(xattr_name + XATTR_SECURITY_PREFIX_LEN, name); >> >> - return hfsplus_setxattr(dentry, xattr_name, buffer, size, flags); >> + res = hfsplus_setxattr(dentry, xattr_name, buffer, size, flags); >> + kfree(xattr_name); >> + return res; >> } >> >> static size_t hfsplus_security_listxattr(struct dentry *dentry, char *list, >> @@ -62,31 +78,35 @@ static int hfsplus_initxattrs(struct inode *inode, >> void *fs_info) >> { >> const struct xattr *xattr; >> - char xattr_name[HFSPLUS_ATTR_MAX_STRLEN + 1] = {0}; >> - size_t xattr_name_len; >> + char *xattr_name; >> int err = 0; >> >> + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, >> + GFP_KERNEL); >> + if (!xattr_name) >> + return -ENOMEM; >> for (xattr = xattr_array; xattr->name != NULL; xattr++) { >> - xattr_name_len = strlen(xattr->name); >> >> - if (xattr_name_len == 0) >> + if (!strcmp(xattr->name, ")) >> continue; >> >> - if (xattr_name_len + XATTR_SECURITY_PREFIX_LEN > >> - HFSPLUS_ATTR_MAX_STRLEN) >> - return -EOPNOTSUPP; >> + err = hfsplus_try_asc2uni_sb(inode->i_sb, xattr->name, >> + HFSPLUS_ATTR_MAX_STRLEN - XATTR_SECURITY_PREFIX_LEN); >> + if (err) >> + break; >> >> strcpy(xattr_name, XATTR_SECURITY_PREFIX); >> strcpy(xattr_name + >> XATTR_SECURITY_PREFIX_LEN, xattr->name); >> memset(xattr_name + >> - XATTR_SECURITY_PREFIX_LEN + xattr_name_len, 0, 1); >> + XATTR_SECURITY_PREFIX_LEN + strlen(xattr->name), 0, 1); >> >> err = __hfsplus_setxattr(inode, xattr_name, >> xattr->value, xattr->value_len, 0); >> if (err) >> break; >> } >> + kfree(xattr_name); >> return err; >> } >> >> diff --git a/fs/hfsplus/xattr_trusted.c b/fs/hfsplus/xattr_trusted.c >> index 426cee2..36716af 100644 >> --- a/fs/hfsplus/xattr_trusted.c >> +++ b/fs/hfsplus/xattr_trusted.c >> @@ -12,37 +12,53 @@ >> 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}; >> - size_t len = strlen(name); >> + char *xattr_name; >> + int res; >> >> if (!strcmp(name, ")) >> return -EINVAL; >> >> - if (len + XATTR_TRUSTED_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN) >> - return -EOPNOTSUPP; >> + res = hfsplus_try_asc2uni(dentry, name, >> + HFSPLUS_ATTR_MAX_STRLEN - XATTR_TRUSTED_PREFIX_LEN); >> + if (res) >> + return res; >> >> + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, >> + GFP_KERNEL); >> + if (!xattr_name) >> + return -ENOMEM; >> strcpy(xattr_name, XATTR_TRUSTED_PREFIX); >> strcpy(xattr_name + XATTR_TRUSTED_PREFIX_LEN, name); >> >> - return hfsplus_getxattr(dentry, xattr_name, buffer, size); >> + res = hfsplus_getxattr(dentry, xattr_name, buffer, size); >> + kfree(xattr_name); >> + return res; >> } >> >> 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}; >> - size_t len = strlen(name); >> + char *xattr_name; >> + int res; >> >> if (!strcmp(name, ")) >> return -EINVAL; >> >> - if (len + XATTR_TRUSTED_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN) >> - return -EOPNOTSUPP; >> + res = hfsplus_try_asc2uni(dentry, name, >> + HFSPLUS_ATTR_MAX_STRLEN - XATTR_TRUSTED_PREFIX_LEN); >> + if (res) >> + return res; >> >> + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, >> + GFP_KERNEL); >> + if (!xattr_name) >> + return -ENOMEM; >> strcpy(xattr_name, XATTR_TRUSTED_PREFIX); >> strcpy(xattr_name + XATTR_TRUSTED_PREFIX_LEN, name); >> >> - return hfsplus_setxattr(dentry, xattr_name, buffer, size, flags); >> + res = hfsplus_setxattr(dentry, xattr_name, buffer, size, flags); >> + kfree(xattr_name); >> + return res; >> } >> >> static size_t hfsplus_trusted_listxattr(struct dentry *dentry, char *list, >> diff --git a/fs/hfsplus/xattr_user.c b/fs/hfsplus/xattr_user.c >> index e340165..26b832c 100644 >> --- a/fs/hfsplus/xattr_user.c >> +++ b/fs/hfsplus/xattr_user.c >> @@ -12,37 +12,53 @@ >> 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}; >> - size_t len = strlen(name); >> + char *xattr_name; >> + int res; >> >> if (!strcmp(name, ")) >> return -EINVAL; >> >> - if (len + XATTR_USER_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN) >> - return -EOPNOTSUPP; >> + res = hfsplus_try_asc2uni(dentry, name, >> + HFSPLUS_ATTR_MAX_STRLEN - XATTR_USER_PREFIX_LEN); >> + if (res) >> + return res; >> >> + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, >> + GFP_KERNEL); >> + if (!xattr_name) >> + return -ENOMEM; >> strcpy(xattr_name, XATTR_USER_PREFIX); >> strcpy(xattr_name + XATTR_USER_PREFIX_LEN, name); >> >> - return hfsplus_getxattr(dentry, xattr_name, buffer, size); >> + res = hfsplus_getxattr(dentry, xattr_name, buffer, size); >> + kfree(xattr_name); >> + return res; >> } >> >> 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}; >> - size_t len = strlen(name); >> + char *xattr_name; >> + int res; >> >> if (!strcmp(name, ")) >> return -EINVAL; >> >> - if (len + XATTR_USER_PREFIX_LEN > HFSPLUS_ATTR_MAX_STRLEN) >> - return -EOPNOTSUPP; >> + res = hfsplus_try_asc2uni(dentry, name, >> + HFSPLUS_ATTR_MAX_STRLEN - XATTR_USER_PREFIX_LEN); >> + if (res) >> + return res; >> >> + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, >> + GFP_KERNEL); >> + if (!xattr_name) >> + return -ENOMEM; >> strcpy(xattr_name, XATTR_USER_PREFIX); >> strcpy(xattr_name + XATTR_USER_PREFIX_LEN, name); >> >> - return hfsplus_setxattr(dentry, xattr_name, buffer, size, flags); >> + res = hfsplus_setxattr(dentry, xattr_name, buffer, size, flags); >> + kfree(xattr_name); >> + return res; >> } >> >> static size_t hfsplus_user_listxattr(struct dentry *dentry, char *list, >> -- >> 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 >> >>Best regards, >> >> Anton >>-- >>Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @) >>Unix Support, Computing Service, University of Cambridge >>J.J. Thomson Avenue, Cambridge, CB3 0RB, UK >> > -- 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
