[PATCH] hfsplus: use xattr handlers for removexattr

[PATCH] hfsplus: use xattr handlers for removexattr

[Christoph Hellwig]

hfsplus was already using the handlers for get and set operations,
and with the removal of can_set_xattr we've now allow operations that
wouldn't otherwise be allowed.

With this we can also centralize the special-casing of the osx.
attrs that don't have prefixes on disk in the osx xattr handlers.

Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 9ee6298..bdec665 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -529,7 +529,7 @@ const struct inode_operations hfsplus_dir_inode_operations = {
 	.setxattr		= generic_setxattr,
 	.getxattr		= generic_getxattr,
 	.listxattr		= hfsplus_listxattr,
-	.removexattr		= hfsplus_removexattr,
+	.removexattr		= generic_removexattr,
 #ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
 	.get_acl		= hfsplus_get_posix_acl,
 	.set_acl		= hfsplus_set_posix_acl,
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 2e10993..83c9166 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -390,7 +390,7 @@ static const struct inode_operations hfsplus_file_inode_operations = {
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= hfsplus_listxattr,
-	.removexattr	= hfsplus_removexattr,
+	.removexattr	= generic_removexattr,
 #ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
 	.get_acl	= hfsplus_get_posix_acl,
 	.set_acl	= hfsplus_set_posix_acl,
diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index 0b4a5c9..4e27edc 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -11,6 +11,8 @@
 #include "xattr.h"
 #include "acl.h"
 
+static int hfsplus_removexattr(struct inode *inode, const char *name);
+
 const struct xattr_handler *hfsplus_xattr_handlers[] = {
 	&hfsplus_xattr_osx_handler,
 	&hfsplus_xattr_user_handler,
@@ -274,14 +276,8 @@ int __hfsplus_setxattr(struct inode *inode, const char *name,
 				HFSPLUS_IS_RSRC(inode))
 		return -EOPNOTSUPP;
 
-	if (strncmp(name, XATTR_MAC_OSX_PREFIX,
-				XATTR_MAC_OSX_PREFIX_LEN) == 0)
-		name += XATTR_MAC_OSX_PREFIX_LEN;
-
-	if (value == NULL) {
-		value = "";
-		size = 0;
-	}
+	if (value == NULL)
+		return hfsplus_removexattr(inode, name);
 
 	err = hfs_find_init(HFSPLUS_SB(inode->i_sb)->cat_tree, &cat_fd);
 	if (err) {
@@ -399,16 +395,11 @@ end_setxattr:
 	return err;
 }
 
-static inline int is_osx_xattr(const char *xattr_name)
-{
-	return !is_known_namespace(xattr_name);
-}
-
 static int name_len(const char *xattr_name, int xattr_name_len)
 {
 	int len = xattr_name_len + 1;
 
-	if (is_osx_xattr(xattr_name))
+	if (!is_known_namespace(xattr_name))
 		len += XATTR_MAC_OSX_PREFIX_LEN;
 
 	return len;
@@ -419,7 +410,7 @@ static int copy_name(char *buffer, const char *xattr_name, int name_len)
 	int len = name_len;
 	int offset = 0;
 
-	if (is_osx_xattr(xattr_name)) {
+	if (!is_known_namespace(xattr_name)) {
 		strncpy(buffer, XATTR_MAC_OSX_PREFIX, XATTR_MAC_OSX_PREFIX_LEN);
 		offset += XATTR_MAC_OSX_PREFIX_LEN;
 		len += XATTR_MAC_OSX_PREFIX_LEN;
@@ -497,18 +488,6 @@ ssize_t __hfsplus_getxattr(struct inode *inode, const char *name,
 				HFSPLUS_IS_RSRC(inode))
 		return -EOPNOTSUPP;
 
-	if (strncmp(name, XATTR_MAC_OSX_PREFIX,
-				XATTR_MAC_OSX_PREFIX_LEN) == 0) {
-		/* skip "osx." prefix */
-		name += XATTR_MAC_OSX_PREFIX_LEN;
-		/*
-		 * Don't allow retrieving properly prefixed attributes
-		 * by prepending them with "osx."
-		 */
-		if (is_known_namespace(name))
-			return -EOPNOTSUPP;
-	}
-
 	if (!strcmp_xattr_finder_info(name))
 		return hfsplus_getxattr_finder_info(inode, value, size);
 
@@ -743,28 +722,18 @@ end_listxattr:
 	return res;
 }
 
-int hfsplus_removexattr(struct dentry *dentry, const char *name)
+static int hfsplus_removexattr(struct inode *inode, const char *name)
 {
 	int err = 0;
-	struct inode *inode = dentry->d_inode;
 	struct hfs_find_data cat_fd;
 	u16 flags;
 	u16 cat_entry_type;
 	int is_xattr_acl_deleted = 0;
 	int is_all_xattrs_deleted = 0;
 
-	if ((!S_ISREG(inode->i_mode) &&
-			!S_ISDIR(inode->i_mode)) ||
-				HFSPLUS_IS_RSRC(inode))
-		return -EOPNOTSUPP;
-
 	if (!HFSPLUS_SB(inode->i_sb)->attr_tree)
 		return -EOPNOTSUPP;
 
-	if (strncmp(name, XATTR_MAC_OSX_PREFIX,
-				XATTR_MAC_OSX_PREFIX_LEN) == 0)
-		name += XATTR_MAC_OSX_PREFIX_LEN;
-
 	if (!strcmp_xattr_finder_info(name))
 		return -EOPNOTSUPP;
 
@@ -838,8 +807,12 @@ static int hfsplus_osx_getxattr(struct dentry *dentry, const char *name,
 	if (len > HFSPLUS_ATTR_MAX_STRLEN)
 		return -EOPNOTSUPP;
 
-	strcpy(xattr_name, XATTR_MAC_OSX_PREFIX);
-	strcpy(xattr_name + XATTR_MAC_OSX_PREFIX_LEN, name);
+	/*
+	 * Don't allow retrieving properly prefixed attributes
+	 * by prepending them with "osx."
+	 */
+	if (is_known_namespace(name))
+		return -EOPNOTSUPP;
 
 	return hfsplus_getxattr(dentry, xattr_name, buffer, size);
 }
@@ -857,12 +830,13 @@ static int hfsplus_osx_setxattr(struct dentry *dentry, const char *name,
 	if (len > HFSPLUS_ATTR_MAX_STRLEN)
 		return -EOPNOTSUPP;
 
+	/*
+	 * Don't allow setting properly prefixed attributes
+	 * by prepending them with "osx."
+	 */
 	if (is_known_namespace(name))
 		return -EOPNOTSUPP;
 
-	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);
 }
 
diff --git a/fs/hfsplus/xattr.h b/fs/hfsplus/xattr.h
index 9e21449..288530c 100644
--- a/fs/hfsplus/xattr.h
+++ b/fs/hfsplus/xattr.h
@@ -40,8 +40,6 @@ static inline ssize_t hfsplus_getxattr(struct dentry *dentry,
 
 ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size);
 
-int hfsplus_removexattr(struct dentry *dentry, const char *name);
-
 int hfsplus_init_security(struct inode *inode, struct inode *dir,
 				const struct qstr *qstr);
 

Re: [PATCH] hfsplus: use xattr handlers for removexattr

[Christoph Hellwig]

On Thu, Jan 30, 2014 at 12:19:48PM +0300, Vyacheslav Dubeyko wrote:
> As I remember, we discussed this changes in the scope of big
> patchset. So, as far as I can judge, you simply decided to extract it
> in one patch. Am I correct?

Well, Al pulled in the earlier version of the patch, so we'll need this
one as a fixup.

> But how is this change safe without generic part of code?
> Especially, for the case of "osx." prefix. Or, maybe, I missed
> some commits in the kernel?

Yeah, the older version got merged via Al's tree.

Re: [PATCH] hfsplus: use xattr handlers for removexattr

[Vyacheslav Dubeyko]

Hi Christoph,

On Jan 30, 2014, at 10:59 AM, Christoph Hellwig wrote:

> hfsplus was already using the handlers for get and set operations,
> and with the removal of can_set_xattr we've now allow operations that
> wouldn't otherwise be allowed.
> 
> With this we can also centralize the special-casing of the osx.
> attrs that don't have prefixes on disk in the osx xattr handlers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 

As I remember, we discussed this changes in the scope of big
patchset. So, as far as I can judge, you simply decided to extract it
in one patch. Am I correct?

But how is this change safe without generic part of code?
Especially, for the case of "osx." prefix. Or, maybe, I missed
some commits in the kernel?

Thanks,
Vyacheslav Dubeyko.

Re: [PATCH] hfsplus: use xattr handlers for removexattr

[Vyacheslav Dubeyko]

On Jan 30, 2014, at 11:17 AM, Christoph Hellwig wrote:

> On Thu, Jan 30, 2014 at 12:19:48PM +0300, Vyacheslav Dubeyko wrote:
>> As I remember, we discussed this changes in the scope of big
>> patchset. So, as far as I can judge, you simply decided to extract it
>> in one patch. Am I correct?
> 
> Well, Al pulled in the earlier version of the patch, so we'll need this
> one as a fixup.
> 

Ok. I see.

This patch looks good for me.

Reviewed-by: Vyacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Vyacheslav Dubeyko.

>> But how is this change safe without generic part of code?
>> Especially, for the case of "osx." prefix. Or, maybe, I missed
>> some commits in the kernel?
> 
> Yeah, the older version got merged via Al's tree.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html