hfsplus: add missing osx prefix to FinderInfo attribute check

By the time we reach strcmp_xattr_finder_info(), hfsplus_osx_getxattr() has already prefixed the user-provided xattr name with "osx.", so the existing check for the special FinderInfo attribute will always fail, and ENODATA will be returned to userspace.

Since hfsplus_listxattr() reports the FinderInfo attribute, this behavior causes certain userspace utilities (e.g. patch) to fail, since the kernel lists an attribute that doesn't exist.

---
 fs/hfsplus/xattr.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

index d98094a..da0664e 100644
@@ -28,8 +28,16 @@ const struct xattr_handler *hfsplus_xattr_handlers[] = {
  static int strcmp_xattr_finder_info(const char *name)
 {
+	int err = 0;
+
 	if (name) {
-		return strncmp(name, HFSPLUS_XATTR_FINDER_INFO_NAME,
+		err = strncmp(name, XATTR_MAC_OSX_PREFIX,
+				XATTR_MAC_OSX_PREFIX_LEN);
+		if (err)
+			return err;
+
+		return strncmp(name + XATTR_MAC_OSX_PREFIX_LEN,
+				HFSPLUS_XATTR_FINDER_INFO_NAME,
 				sizeof(HFSPLUS_XATTR_FINDER_INFO_NAME));
 	}
 	return -1;

2.3.4

Could you describe in more details how do you use userspace utilities for extracting xattr? I suspect that you simply use it in incorrect way.

Thanks,
Vyacheslav Dubeyko.

Of course. I first create a new file on an HFS+ filesystem (case- sensitive, journaling disabled). I then call listxattr() on that file and receive a list of one attribute:
osx.com.apple.FinderInfo
I call getxattr() with this name as the second argument.

Without the patch, getxattr() returns -1 and sets errno to either ENOATTR (ENODATA) if the volume has an attribute tree or EOPNOTSUPP if it doesn't. This behavior is clearly wrong, since listxattr() told me that the attribute existed.

In the case where ENOATTR is returned (when an attribute b-tree exists,
as it does when any file on the filesystem has a real attribute), the
error causes GNU patch 2.7.5 to fail with the error message
getting attribute osx.com.apple.FinderInfo of osx.com.apple.FinderInfo: No data available
(As shown in the attached typescript.) That's how I originally discovered the problem.

I have written a test program to demonstrate the issue. It builds on both Linux and OS X, so that the behaviors can be compared. You can find it at https://github.com/tchebb/xattr-test.

After more investigation, however, I must withdraw the initial patch I sent. It is not a correct solution to the entire problem; it's simply a band-aid that fixes this one particular symptom. As you can see from the included typescript, *any* osx-namespaced attribute is given an additional copy of the prefix. I believe I've found a commit from 2014 that introduced this incorrect behavior, and I'll send another patch soon that should resolve the issue entirely.

Thanks,
-Thomas Hebb

typescript
---

$ cat >a				# Make some test files
line 1
line 2
$ cat >b
line 1
line 2
line 3
$ diff -u a b | tee patch		# Create a diff between them
--- a	2015-04-02 17:03:18.000000000 -0400
+++ b	2015-04-02 17:03:30.000000000 -0400
@@ -1,2 +1,3 @@
 line 1
 line 2
+line 3
$ patch -i patch a			# Patch the file. Works fine, for now...
patching file a
$ ./xattr-test a			# But getxattr() still returns EOPNOTSUPP
Listing attributes of "a"
listxattr() returned 25
  List contents:
    osx.com.apple.FinderInfo

Getting attribute "osx.com.apple.FinderInfo"
getxattr() returned -1
getxattr() failed: Operation not supported

$ cat >a				# Let's try again
line 1
line 2
$ setfattr -n 'user.test' -v 'abc' a	# This time, give the volume an attr tree
$ patch -i patch a			# And patch fails!
patching file a
patch: getting attribute osx.com.apple.FinderInfo of osx.com.apple.FinderInfo: No data available
$ ./xattr-test a			# Because getxattr() returns ENODATA
Listing attributes of "a"
listxattr() returned 35
  List contents:
    osx.com.apple.FinderInfo
    user.test

Getting attribute "osx.com.apple.FinderInfo"
getxattr() returned -1
getxattr() failed: No data available

Getting attribute "user.test"
getxattr() returned 3
Attribute value:

    0x61 0x62 0x63

$ setfattr -n 'osx.test' -v 'def' a	# Now, what if we create an attr in the osx namespace?
$ ./xattr-test a			# Aha, here's the real problem: "osx." is prefixed an extra time!
Listing attributes of "a"
listxattr() returned 48
  List contents:
    osx.com.apple.FinderInfo
    osx.osx.test
    user.test

Getting attribute "osx.com.apple.FinderInfo"
getxattr() returned -1
getxattr() failed: No data available

Getting attribute "osx.osx.test"
getxattr() returned -1
getxattr() failed: No data available

Getting attribute "user.test"
getxattr() returned 3
Attribute value:

    0x61 0x62 0x63

$ exit

OK. Thank you. I need to check that xattr support doesn't work for HFS+ now. It worked pretty well earlier.

Thanks,
Vyacheslav Dubeyko.

Your patch does fix a bug (reproduced through listxattr/getxattr/setxattr apis). But I'd recommend a more elegant code:

@@ -29,7 +29,9 @@ const struct xattr_handler *hfsplus_xattr_handlers[] = {
 static int strcmp_xattr_finder_info(const char *name)
 {
     if (name) {
-        return strncmp(name, HFSPLUS_XATTR_FINDER_INFO_NAME,
+        return strncmp(name, XATTR_MAC_OSX_PREFIX
+                HFSPLUS_XATTR_FINDER_INFO_NAME,
+                XATTR_MAC_OSX_PREFIX_LEN +
                 sizeof(HFSPLUS_XATTR_FINDER_INFO_NAME));
     }
     return -1;

If you resubmit it, make it Reviewed-by: Sergei Antonov <[email protected]>

The issue is located in hfsplus_osx_getxattr() method:

{
        char *xattr_name;
        int res;

        if (!strcmp(name, ""))
                return -EINVAL;

        /*
         * Don't allow retrieving properly prefixed attributes
         * by prepending them with "osx."
         */
        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);

*************************************************************

This part adds "osx." prefix second time. So, it needs to fix the issue here.

Thanks,
Vyacheslav Dubeyko.