On Sun, 2015-04-05 at 00:36 -0400, Thomas Hebb wrote: > This version of the commit is rebased on linux-next, as Andrew requested. > The conflicting changes were just refactoring, so this version ought to > be functionally identical to the first. > > It does change the osx-namespace functions to call __hfsplus_?etxattr() > directly instead of going through the refactored hfsplus_?etxattr() > functions, which allocate space for a copy of the name with a prepended > namespace. Since the osx attributes aren't stored with a namespace, we > save ourselves a copy by bypassing the functions, but we also introduce > the potential for future bugs if the hfsplus_?etattr() functions are > later changed to do something that all namespaces need. > > If the extra copy is inconsequential (or if there's a reason it's needed > that I'm unaware of), I'm happy to revise the patch again to call > hfsplus_?etattr() with an empty namespace parameter, instead of bypassing > them altogether. Looks good for me. Thanks, Vyacheslav Dubeyko. > > Thanks, > -Thomas > > On Sat, 2015-04-04 at 19:08 -0400, Thomas Hebb wrote: > > On Mac OS X, HFS+ extended attributes are not namespaced. Since we want > > to be compatible with OS X filesystems and yet still support the Linux > > namespacing system, the hfsplus driver implements a special "osx" > > namespace that is reported for any attribute that is not namespaced > > on-disk. However, the current code for getting and setting these > > unprefixed attributes is broken. > > > > hfsplus_osx_setattr() and hfsplus_osx_getattr() are passed names that > > have already had their "osx." prefixes stripped by the generic functions. > > The functions first, quite correctly, check those names to make sure > > that they aren't prefixed with a known namespace, which would allow > > namespace access restrictions to be bypassed. However, the functions > > then prepend "osx." to the name they're given before passing it on to > > hfsplus_getattr() and hfsplus_setattr(). Not only does this cause the > > "osx." prefix to be stored on-disk, defeating its purpose, it also breaks > > the check for the special "com.apple.FinderInfo" attribute, which is > > reported for all files, and as a consequence makes some userspace > > applications (e.g. GNU patch) fail even when extended attributes are not > > otherwise in use. > > > > There are five commits which have touched this particular code: > > > > 127e5f5ae51e ("hfsplus: rework functionality of getting, setting and deleting of extended attributes") > > b168fff72109 ("hfsplus: use xattr handlers for removexattr") > > bf29e886b242 ("hfsplus: correct usage of HFSPLUS_ATTR_MAX_STRLEN for non-English attributes") > > fcacbd95e121 ("fs/hfsplus: move xattr_name allocation in hfsplus_getxattr()") > > ec1bbd346f18 ("fs/hfsplus: move xattr_name allocation in hfsplus_setxattr()") > > > > The first commit creates the functions to begin with. The namespace is > > prepended by the original code, which I believe was correct at the time, > > since hfsplus_?etattr() stripped the prefix if found. The second commit > > removes this behavior from hfsplus_?etattr() and appears to have been > > intended to also remove the prefixing from hfsplus_osx_?etattr(). However, > > what it actually does is remove a necessary strncpy() call completely, > > breaking the osx namespace entirely. The third commit re-adds the > > strncpy() call as it was originally, but doesn't mention it in its > > commit message. The final two commits refactor the code and don't > > affect its functionality. > > > > This commit does what b168fff attempted to do (prevent the prefix from > > being added), but does it properly, instead of passing in an empty buffer > > (which is what b168fff actually did). > > > > Fixes: b168fff72109 ("hfsplus: use xattr handlers for removexattr") > > Cc: stable@xxxxxxxxxxxxxxx > > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > > Cc: Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx> > > Cc: Sergei Antonov <saproj@xxxxxxxxx> > > Cc: Anton Altaparmakov <anton@xxxxxxxxxx> > > Cc: Fabian Frederick <fabf@xxxxxxxxx> > > Cc: Christian Kujau <lists@xxxxxxxxxxxxxxx> > > Signed-off-by: Thomas Hebb <tommyhebb@xxxxxxxxx> > > --- > > fs/hfsplus/xattr.c | 19 ++++++++++++++----- > > 1 file changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c > > index 8d62de0..89f262d 100644 > > --- a/fs/hfsplus/xattr.c > > +++ b/fs/hfsplus/xattr.c > > @@ -862,8 +862,13 @@ static int hfsplus_osx_getxattr(struct dentry *dentry, const char *name, > > if (is_known_namespace(name)) > > return -EOPNOTSUPP; > > > > - return hfsplus_getxattr(dentry, name, buffer, size, > > - XATTR_MAC_OSX_PREFIX, XATTR_MAC_OSX_PREFIX_LEN); > > + /* > > + * osx is the namespace we use to indicate an unprefixed > > + * attribute on the filesystem (like the ones that OS X > > + * creates), so we pass the name through unmodified (after > > + * ensuring it doesn't conflict with another namespace). > > + */ > > + return __hfsplus_getxattr(dentry->d_inode, name, buffer, size); > > } > > > > static int hfsplus_osx_setxattr(struct dentry *dentry, const char *name, > > @@ -879,9 +884,13 @@ static int hfsplus_osx_setxattr(struct dentry *dentry, const char *name, > > if (is_known_namespace(name)) > > return -EOPNOTSUPP; > > > > - return hfsplus_setxattr(dentry, name, buffer, size, flags, > > - XATTR_MAC_OSX_PREFIX, > > - XATTR_MAC_OSX_PREFIX_LEN); > > + /* > > + * osx is the namespace we use to indicate an unprefixed > > + * attribute on the filesystem (like the ones that OS X > > + * creates), so we pass the name through unmodified (after > > + * ensuring it doesn't conflict with another namespace). > > + */ > > + return __hfsplus_setxattr(dentry->d_inode, name, buffer, size, flags); > > } > > > > static size_t hfsplus_osx_listxattr(struct dentry *dentry, char *list, -- 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