Skip to contentSkip to conversation

ocfs2: some xattr fixes

Ocfs2-devel New Tiger Yang 2 participants 11 messages Oct 23, 2008 OCFS2

lore
Share & tools
lore

Tiger Yang

author
#0 lore raw

Makr, Joel,

There are 4 fixes for xattr. The top 3 patches in the first patch of acl v3 patches before. The fourth patch is a new fix due to I met a bug about this.

1. fix licence in xattr
2. fix function declaration in xattr
3. remove duplicate definition in xattr
4. add array bounds checking

Best regards,
tiger

author
#1 lore raw

PATCH 1/4 ocfs2: fix license in xattr

This patch fix license in xattr.c and xattr.h.

---
 fs/ocfs2/xattr.c |   13 ++++---------
 fs/ocfs2/xattr.h |   12 ++----------
 2 files changed, 6 insertions(+), 19 deletions(-)
fs/ocfs2/xattr.c +4 -9
index 0239078..baa4825 100644
@@ -3,25 +3,20 @@
  *
  * xattr.c
  *
- * Copyright (C) 2008 Oracle.  All rights reserved.
+ * Copyright (C) 2004, 2008 Oracle.  All rights reserved.
  *
  * CREDITS:
- * Lots of code in this file is taken from ext3.
+ * Lots of code in this file is copy from linux/fs/ext3/xattr.c.
+ * Copyright (C) 2001-2003 Andreas Gruenbacher, <[email protected]>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
+ * License version 2 as published by the Free Software Foundation.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public
- * License along with this program; if not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 021110-1307, USA.
  */
 
 #include <linux/capability.h>
fs/ocfs2/xattr.h +2 -10
index c25c7c6..e4e45c8 100644
@@ -3,24 +3,16 @@
  *
  * xattr.h
  *
- * Function prototypes
- *
- * Copyright (C) 2008 Oracle.  All rights reserved.
+ * Copyright (C) 2004, 2008 Oracle.  All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
+ * License version 2 as published by the Free Software Foundation.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public
- * License along with this program; if not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 021110-1307, USA.
  */
 
 #ifndef OCFS2_XATTR_H
Signature · 1 line
1.5.4.1
#2 lore raw

PATCH 1/4 ocfs2: fix license in xattr

Quoted 3 lines · Tiger Yang

This patch fix license in xattr.c and xattr.h.

Quoted 73 lines
---
 fs/ocfs2/xattr.c |   13 ++++---------
 fs/ocfs2/xattr.h |   12 ++----------
 2 files changed, 6 insertions(+), 19 deletions(-)
fs/ocfs2/xattr.c +4 -9
index 0239078..baa4825 100644
@@ -3,25 +3,20 @@
  *
  * xattr.c
  *
- * Copyright (C) 2008 Oracle.  All rights reserved.
+ * Copyright (C) 2004, 2008 Oracle.  All rights reserved.
  *
  * CREDITS:
- * Lots of code in this file is taken from ext3.
+ * Lots of code in this file is copy from linux/fs/ext3/xattr.c.
+ * Copyright (C) 2001-2003 Andreas Gruenbacher, <[email protected]>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
+ * License version 2 as published by the Free Software Foundation.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public
- * License along with this program; if not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 021110-1307, USA.
  */
 
 #include <linux/capability.h>
fs/ocfs2/xattr.h +2 -10
index c25c7c6..e4e45c8 100644
@@ -3,24 +3,16 @@
  *
  * xattr.h
  *
- * Function prototypes
- *
- * Copyright (C) 2008 Oracle.  All rights reserved.
+ * Copyright (C) 2004, 2008 Oracle.  All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
+ * License version 2 as published by the Free Software Foundation.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public
- * License along with this program; if not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 021110-1307, USA.
  */
 
 #ifndef OCFS2_XATTR_H
Signature · 1 line
1.5.4.1
Signature · 9 lines
"Depend on the rabbit's foot if you will, but remember, it didn't
 help the rabbit."
	- R. E. Shay

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
author
#3 lore raw

PATCH 2/4 ocfs2: fix function declaration and definition in xattr

In reply to Tiger Yang

Because we merge split xattr files into one file, some functions no need declare or define in the head file.

---
 fs/ocfs2/xattr.c |   28 +++++++++++++++++++++++-----
 fs/ocfs2/xattr.h |   26 ++++----------------------
 2 files changed, 27 insertions(+), 27 deletions(-)
fs/ocfs2/xattr.c +23 -5
index baa4825..667525b 100644
@@ -142,6 +142,24 @@ static int ocfs2_xattr_set_entry_index_block(struct inode *inode,
 static int ocfs2_delete_xattr_index_block(struct inode *inode,
 					  struct buffer_head *xb_bh);
 
+static inline u16 ocfs2_xattr_buckets_per_cluster(struct ocfs2_super *osb)
+{
+	return (1 << osb->s_clustersize_bits) / OCFS2_XATTR_BUCKET_SIZE;
+}
+
+static inline u16 ocfs2_blocks_per_xattr_bucket(struct super_block *sb)
+{
+	return OCFS2_XATTR_BUCKET_SIZE / (1 << sb->s_blocksize_bits);
+}
+
+static inline u16 ocfs2_xattr_max_xe_in_bucket(struct super_block *sb)
+{
+	u16 len = sb->s_blocksize -
+		 offsetof(struct ocfs2_xattr_header, xh_entries);
+
+	return len / sizeof(struct ocfs2_xattr_entry);
+}
+
 static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
@@ -793,11 +811,11 @@ cleanup:
  * Copy an extended attribute into the buffer provided.
  * Buffer is NULL to compute the size of buffer required.
  */
-int ocfs2_xattr_get(struct inode *inode,
-		    int name_index,
-		    const char *name,
-		    void *buffer,
-		    size_t buffer_size)
+static int ocfs2_xattr_get(struct inode *inode,
+			   int name_index,
+			   const char *name,
+			   void *buffer,
+			   size_t buffer_size)
 {
 	int ret;
 	struct ocfs2_dinode *di = NULL;
fs/ocfs2/xattr.h +4 -22
index e4e45c8..1d8314c 100644
@@ -32,29 +32,11 @@ enum ocfs2_xattr_type {
 
 extern struct xattr_handler ocfs2_xattr_user_handler;
 extern struct xattr_handler ocfs2_xattr_trusted_handler;
-
-extern ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);
-extern int ocfs2_xattr_get(struct inode *, int, const char *, void *, size_t);
-extern int ocfs2_xattr_set(struct inode *, int, const char *, const void *,
-			   size_t, int);
-extern int ocfs2_xattr_remove(struct inode *inode, struct buffer_head *di_bh);
 extern struct xattr_handler *ocfs2_xattr_handlers[];
 
-static inline u16 ocfs2_xattr_buckets_per_cluster(struct ocfs2_super *osb)
-{
-	return (1 << osb->s_clustersize_bits) / OCFS2_XATTR_BUCKET_SIZE;
-}
-
-static inline u16 ocfs2_blocks_per_xattr_bucket(struct super_block *sb)
-{
-	return OCFS2_XATTR_BUCKET_SIZE / (1 << sb->s_blocksize_bits);
-}
-
-static inline u16 ocfs2_xattr_max_xe_in_bucket(struct super_block *sb)
-{
-	u16 len = sb->s_blocksize -
-		 offsetof(struct ocfs2_xattr_header, xh_entries);
+ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);
+int ocfs2_xattr_set(struct inode *, int, const char *, const void *,
+		    size_t, int);
+int ocfs2_xattr_remove(struct inode *, struct buffer_head *);
 
-	return len / sizeof(struct ocfs2_xattr_entry);
-}
 #endif /* OCFS2_XATTR_H */
Signature · 1 line
1.5.4.1
#4 lore raw

PATCH 2/4 ocfs2: fix function declaration and definition in xattr

Quoted 4 lines · Tiger Yang

Because we merge split xattr files into one file, some functions no need declare or define in the head file.

Quoted 92 lines
---
 fs/ocfs2/xattr.c |   28 +++++++++++++++++++++++-----
 fs/ocfs2/xattr.h |   26 ++++----------------------
 2 files changed, 27 insertions(+), 27 deletions(-)
fs/ocfs2/xattr.c +23 -5
index baa4825..667525b 100644
@@ -142,6 +142,24 @@ static int ocfs2_xattr_set_entry_index_block(struct inode *inode,
 static int ocfs2_delete_xattr_index_block(struct inode *inode,
 					  struct buffer_head *xb_bh);
 
+static inline u16 ocfs2_xattr_buckets_per_cluster(struct ocfs2_super *osb)
+{
+	return (1 << osb->s_clustersize_bits) / OCFS2_XATTR_BUCKET_SIZE;
+}
+
+static inline u16 ocfs2_blocks_per_xattr_bucket(struct super_block *sb)
+{
+	return OCFS2_XATTR_BUCKET_SIZE / (1 << sb->s_blocksize_bits);
+}
+
+static inline u16 ocfs2_xattr_max_xe_in_bucket(struct super_block *sb)
+{
+	u16 len = sb->s_blocksize -
+		 offsetof(struct ocfs2_xattr_header, xh_entries);
+
+	return len / sizeof(struct ocfs2_xattr_entry);
+}
+
 static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
@@ -793,11 +811,11 @@ cleanup:
  * Copy an extended attribute into the buffer provided.
  * Buffer is NULL to compute the size of buffer required.
  */
-int ocfs2_xattr_get(struct inode *inode,
-		    int name_index,
-		    const char *name,
-		    void *buffer,
-		    size_t buffer_size)
+static int ocfs2_xattr_get(struct inode *inode,
+			   int name_index,
+			   const char *name,
+			   void *buffer,
+			   size_t buffer_size)
 {
 	int ret;
 	struct ocfs2_dinode *di = NULL;
fs/ocfs2/xattr.h +4 -22
index e4e45c8..1d8314c 100644
@@ -32,29 +32,11 @@ enum ocfs2_xattr_type {
 
 extern struct xattr_handler ocfs2_xattr_user_handler;
 extern struct xattr_handler ocfs2_xattr_trusted_handler;
-
-extern ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);
-extern int ocfs2_xattr_get(struct inode *, int, const char *, void *, size_t);
-extern int ocfs2_xattr_set(struct inode *, int, const char *, const void *,
-			   size_t, int);
-extern int ocfs2_xattr_remove(struct inode *inode, struct buffer_head *di_bh);
 extern struct xattr_handler *ocfs2_xattr_handlers[];
 
-static inline u16 ocfs2_xattr_buckets_per_cluster(struct ocfs2_super *osb)
-{
-	return (1 << osb->s_clustersize_bits) / OCFS2_XATTR_BUCKET_SIZE;
-}
-
-static inline u16 ocfs2_blocks_per_xattr_bucket(struct super_block *sb)
-{
-	return OCFS2_XATTR_BUCKET_SIZE / (1 << sb->s_blocksize_bits);
-}
-
-static inline u16 ocfs2_xattr_max_xe_in_bucket(struct super_block *sb)
-{
-	u16 len = sb->s_blocksize -
-		 offsetof(struct ocfs2_xattr_header, xh_entries);
+ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);
+int ocfs2_xattr_set(struct inode *, int, const char *, const void *,
+		    size_t, int);
+int ocfs2_xattr_remove(struct inode *, struct buffer_head *);
 
-	return len / sizeof(struct ocfs2_xattr_entry);
-}
 #endif /* OCFS2_XATTR_H */
Signature · 1 line
1.5.4.1
Signature · 9 lines
"The real reason GNU ls is 8-bit-clean is so that they can
 start using ISO-8859-1 option characters."
	- Christopher Davis (ckd at loiosh.kei.com)

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
author
#5 lore raw

PATCH 3/4 ocfs2: remove duplicate definition in xattr

In reply to Tiger Yang

Include/linux/xattr.h already has the definition about xattr prefix, so remove the duplicate definitions in xattr.c.

---
 fs/ocfs2/xattr.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)
fs/ocfs2/xattr.c +2 -9
index 667525b..754e5fa 100644
@@ -4830,14 +4830,11 @@ out:
 /*
  * 'trusted' attributes support
  */
-
-#define XATTR_TRUSTED_PREFIX "trusted."
-
 static size_t ocfs2_xattr_trusted_list(struct inode *inode, char *list,
 				       size_t list_size, const char *name,
 				       size_t name_len)
 {
-	const size_t prefix_len = sizeof(XATTR_TRUSTED_PREFIX) - 1;
+	const size_t prefix_len = XATTR_TRUSTED_PREFIX_LEN;
 	const size_t total_len = prefix_len + name_len + 1;
 
 	if (list && total_len <= list_size) {
@@ -4874,18 +4871,14 @@ struct xattr_handler ocfs2_xattr_trusted_handler = {
 	.set	= ocfs2_xattr_trusted_set,
 };
 
-
 /*
  * 'user' attributes support
  */
-
-#define XATTR_USER_PREFIX "user."
-
 static size_t ocfs2_xattr_user_list(struct inode *inode, char *list,
 				    size_t list_size, const char *name,
 				    size_t name_len)
 {
-	const size_t prefix_len = sizeof(XATTR_USER_PREFIX) - 1;
+	const size_t prefix_len = XATTR_USER_PREFIX_LEN;
 	const size_t total_len = prefix_len + name_len + 1;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 
Signature · 1 line
1.5.4.1
#6 lore raw

PATCH 3/4 ocfs2: remove duplicate definition in xattr

Quoted 4 lines · Tiger Yang

Include/linux/xattr.h already has the definition about xattr prefix, so remove the duplicate definitions in xattr.c.

Quoted 47 lines
---
 fs/ocfs2/xattr.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)
fs/ocfs2/xattr.c +2 -9
index 667525b..754e5fa 100644
@@ -4830,14 +4830,11 @@ out:
 /*
  * 'trusted' attributes support
  */
-
-#define XATTR_TRUSTED_PREFIX "trusted."
-
 static size_t ocfs2_xattr_trusted_list(struct inode *inode, char *list,
 				       size_t list_size, const char *name,
 				       size_t name_len)
 {
-	const size_t prefix_len = sizeof(XATTR_TRUSTED_PREFIX) - 1;
+	const size_t prefix_len = XATTR_TRUSTED_PREFIX_LEN;
 	const size_t total_len = prefix_len + name_len + 1;
 
 	if (list && total_len <= list_size) {
@@ -4874,18 +4871,14 @@ struct xattr_handler ocfs2_xattr_trusted_handler = {
 	.set	= ocfs2_xattr_trusted_set,
 };
 
-
 /*
  * 'user' attributes support
  */
-
-#define XATTR_USER_PREFIX "user."
-
 static size_t ocfs2_xattr_user_list(struct inode *inode, char *list,
 				    size_t list_size, const char *name,
 				    size_t name_len)
 {
-	const size_t prefix_len = sizeof(XATTR_USER_PREFIX) - 1;
+	const size_t prefix_len = XATTR_USER_PREFIX_LEN;
 	const size_t total_len = prefix_len + name_len + 1;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 
Signature · 1 line
1.5.4.1
Signature · 8 lines
"If at first you don't succeed, cover all traces that you tried."
                                                        -Unknown

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
author
#7 lore raw

PATCH 4/4 ocfs2: add array bounds checking

In reply to Tiger Yang

This could avoid arry bounds exceeded issue.

---
 fs/ocfs2/xattr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/ocfs2/xattr.c +1 -1
index 754e5fa..888641d 100644
@@ -164,7 +164,7 @@ static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
 
-	if (name_index > 0 && name_index < OCFS2_XATTR_MAX)
+	if (name_index > 0 && name_index < ARRAY_SIZE(ocfs2_xattr_handler_map))
 		handler = ocfs2_xattr_handler_map[name_index];
 
 	return handler ? handler->prefix : NULL;
Signature · 1 line
1.5.4.1
#8 lore raw

PATCH 4/4 ocfs2: add array bounds checking

Quoted 12 lines · Tiger Yang

This could avoid arry bounds exceeded issue.

---
 fs/ocfs2/xattr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/ocfs2/xattr.c +0 -0
index 754e5fa..888641d 100644

fs/ocfs2/xattr.c static inline const char *ocfs2_xattr_prefix(int name_index) · line 168

Quoted 6 lines
fs/ocfs2/xattr.c +1 -1
@@ -164,7 +164,7 @@ static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
 
-	if (name_index > 0 && name_index < OCFS2_XATTR_MAX)
+	if (name_index > 0 && name_index < ARRAY_SIZE(ocfs2_xattr_handler_map))
 		handler = ocfs2_xattr_handler_map[name_index];

Another option is to define the map as:

static struct xattr_handler *ocfs2_xattr_handler_map[OCFS2_ATTR_MAX] = {

This reuses our nice OCFS2_XATTR_MAX value. But both methods work.

Joel

Signature · 15 lines
"You cannot bring about prosperity by discouraging thrift. You cannot
 strengthen the weak by weakening the strong. You cannot help the wage
 earner by pulling down the wage payer. You cannot further the
 brotherhood of man by encouraging class hatred. You cannot help the
 poor by destroying the rich. You cannot build character and courage by
 taking away a man's initiative and independence. You cannot help men
 permanently by doing for them what they could and should do for
 themselves."
	- Abraham Lincoln 

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
author
#9 lore raw

PATCH 4/4 ocfs2: add array bounds checking

Thanks for your quick review and suggestion.
I think your solutions is better than mine. It's more simple.

thanks,
tiger

Quoted 28 lines · Joel Becker
Quoted 12 lines · Tiger Yang

This could avoid arry bounds exceeded issue.

---
 fs/ocfs2/xattr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/ocfs2/xattr.c +0 -0
index 754e5fa..888641d 100644

fs/ocfs2/xattr.c static inline const char *ocfs2_xattr_prefix(int name_index) · line 168

Quoted 6 lines
fs/ocfs2/xattr.c +1 -1
@@ -164,7 +164,7 @@ static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
 
-	if (name_index > 0 && name_index < OCFS2_XATTR_MAX)
+	if (name_index > 0 && name_index < ARRAY_SIZE(ocfs2_xattr_handler_map))
 		handler = ocfs2_xattr_handler_map[name_index];

Another option is to define the map as:

static struct xattr_handler *ocfs2_xattr_handler_map[OCFS2_ATTR_MAX] = {

This reuses our nice OCFS2_XATTR_MAX value. But both methods work.

Joel

#10 lore raw

PATCH 4/4 ocfs2: add array bounds checking

Quoted 2 lines · Tiger Yang

Thanks for your quick review and suggestion.
I think your solutions is better than mine. It's more simple.

I pulled your change but fixed it up this way.

Joel

Quoted 39 lines

thanks,
tiger

Quoted 22 lines · Joel Becker
Quoted 12 lines · Tiger Yang

This could avoid arry bounds exceeded issue.

---
 fs/ocfs2/xattr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/ocfs2/xattr.c +0 -0
index 754e5fa..888641d 100644

fs/ocfs2/xattr.c static inline const char *ocfs2_xattr_prefix(int name_index) · line 168

Quoted 6 lines
fs/ocfs2/xattr.c +1 -1
@@ -164,7 +164,7 @@ static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
 
-	if (name_index > 0 && name_index < OCFS2_XATTR_MAX)
+	if (name_index > 0 && name_index < ARRAY_SIZE(ocfs2_xattr_handler_map))
 		handler = ocfs2_xattr_handler_map[name_index];

Another option is to define the map as:

Quoted 6 lines

static struct xattr_handler *ocfs2_xattr_handler_map[OCFS2_ATTR_MAX] = {

This reuses our nice OCFS2_XATTR_MAX value. But both methods work.

Joel

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel

Signature · 9 lines
"What do you take me for, an idiot?"  
        - General Charles de Gaulle, when a journalist asked him
          if he was happy.

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127