Skip to contentSkip to conversation

ocfs2: add security EA and ACL support v1

PATCH New Tiger Yang 3 participants 14 messages Sep 19, 2008 – Oct 2, 2008 OCFS2

lore
Share & tools
lore

Tiger Yang

author
#0 lore raw

cover ocfs2: add security EA and ACL support v1

Hi,
The previous patches about EA didn't implement POSIX ACL and security EA. I add those support here.

The first patch fix some problem in xattr code.
The second patch add security EA support.
The third patch add ACL support basically.

Best regards,
tiger

author
#1 lore raw

PATCH 1/3 ocfs2: small fix in xattr

This patch fix some define problems.

Signed-off-by Tiger Yang <[email protected]>
---
 fs/ocfs2/xattr.h         |   12 ++++++------
 fs/ocfs2/xattr_trusted.c |    4 +---
 fs/ocfs2/xattr_user.c    |    4 +---
 3 files changed, 8 insertions(+), 12 deletions(-)
fs/ocfs2/xattr.h +6 -6
index d33dbe5..af2ba32 100644
@@ -47,14 +47,14 @@ extern struct xattr_handler ocfs2_xattr_acl_default_handler;
 #ifdef CONFIG_OCFS2_FS_SECURITY
 extern struct xattr_handler ocfs2_xattr_security_handler;
 #endif
-
-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[];
 
+ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);
+int ocfs2_xattr_get(struct inode *, int, const char *, void *, size_t);
+int ocfs2_xattr_set(struct inode *, int, const char *, const void *,
+		    size_t, int);
+int ocfs2_xattr_remove(struct inode *inode, struct buffer_head *di_bh);
+
 static inline u16 ocfs2_xattr_buckets_per_cluster(struct ocfs2_super *osb)
 {
 	return (1 << osb->s_clustersize_bits) / OCFS2_XATTR_BUCKET_SIZE;
fs/ocfs2/xattr_trusted.c +1 -3
index 4c589c4..cc15d40 100644
@@ -38,13 +38,11 @@
 #include "ocfs2_fs.h"
 #include "xattr.h"
 
-#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) {
fs/ocfs2/xattr_user.c +1 -3
index 93ba716..cb0bc41 100644
@@ -38,13 +38,11 @@
 #include "ocfs2_fs.h"
 #include "xattr.h"
 
-#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
author
#2 lore raw

PATCH 2/3 ocfs2: Add security xattr support in ocfs2

In reply to Tiger Yang

This patch add security extended attribute support in ocfs2.

Signed-off-by Tiger Yang <[email protected]>
---
 fs/ocfs2/Makefile         |    3 +-
 fs/ocfs2/xattr.c          |    4 --
 fs/ocfs2/xattr.h          |    2 -
 fs/ocfs2/xattr_security.c |   81 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 7 deletions(-)
 create mode 100644 fs/ocfs2/xattr_security.c
fs/ocfs2/Makefile +2 -1
index 21323da..73c1c82 100644
@@ -37,7 +37,8 @@ ocfs2-objs := \
 	ver.o			\
 	xattr.o			\
 	xattr_user.o		\
-	xattr_trusted.o
+	xattr_trusted.o		\
+	xattr_security.o
 
 ocfs2_stackglue-objs := stackglue.o
 ocfs2_stack_o2cb-objs := stack_o2cb.o
fs/ocfs2/xattr.c +0 -4
index 505fb40..d57cfae 100644
@@ -81,9 +81,7 @@ struct xattr_handler *ocfs2_xattr_handlers[] = {
 	&ocfs2_xattr_acl_default_handler,
 #endif
 	&ocfs2_xattr_trusted_handler,
-#ifdef CONFIG_OCFS2_FS_SECURITY
 	&ocfs2_xattr_security_handler,
-#endif
 	NULL
 };
 

@@ -96,9 +94,7 @@ static struct xattr_handler *ocfs2_xattr_handler_map[] = {
 					= &ocfs2_xattr_acl_default_handler,
 #endif
 	[OCFS2_XATTR_INDEX_TRUSTED]	= &ocfs2_xattr_trusted_handler,
-#ifdef CONFIG_OCFS2_FS_SECURITY
 	[OCFS2_XATTR_INDEX_SECURITY]	= &ocfs2_xattr_security_handler,
-#endif
 };
 
 struct ocfs2_xattr_info {
fs/ocfs2/xattr.h +0 -2
index af2ba32..f3ec79a 100644
@@ -44,9 +44,7 @@ extern struct xattr_handler ocfs2_xattr_trusted_handler;
 extern struct xattr_handler ocfs2_xattr_acl_access_handler;
 extern struct xattr_handler ocfs2_xattr_acl_default_handler;
 #endif
-#ifdef CONFIG_OCFS2_FS_SECURITY
 extern struct xattr_handler ocfs2_xattr_security_handler;
-#endif
 extern struct xattr_handler *ocfs2_xattr_handlers[];
 
 ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);
fs/ocfs2/xattr_security.c new file +81 -0

81 lines not on this page · raw message

Signature · 1 line
1.5.4.1
#3 lore raw

PATCH 2/3 ocfs2: Add security xattr support in ocfs2

Quoted 1 line · Tiger Yang

This patch add security extended attribute support in ocfs2.

Ok, thanks for this.

fs/ocfs2/xattr_security.c line 81

Quoted 6 lines
fs/ocfs2/xattr_security.c new file +6 -0
+struct xattr_handler ocfs2_xattr_security_handler = {
+	.prefix	= XATTR_SECURITY_PREFIX,
+	.list	= ocfs2_xattr_security_list,
+	.get	= ocfs2_xattr_security_get,
+	.set	= ocfs2_xattr_security_set,
+};

Don't we need to add a call to 'security_inode_init_security()' when creating a new inode?

The patch otherwise looks good. How was it tested?

	--Mark

--
Mark Fasheh

author
#4 lore raw

PATCH 2/3 ocfs2: Add security xattr support in ocfs2

Hi,Mark,

Quoted 1 line · Mark Fasheh

Don't we need to add a call to 'security_inode_init_security()' when

creating a new

Quoted 1 line

inode?

In security_inode_init_security(), it will check IS_PRIVATE(inode), if it is not a private inode, it will return.
It seems we don't use S_PRIVATE in ocfs2, do we?

anyway, I can add this call in ocfs2 quickly.

Best regards,
tiger

#5 lore raw

PATCH 2/3 ocfs2: Add security xattr support in ocfs2

Quoted 9 lines · Tiger Yang

Hi,Mark,

Quoted 1 line · Mark Fasheh

Don't we need to add a call to 'security_inode_init_security()' when

creating a new

Quoted 1 line

inode?

In security_inode_init_security(), it will check IS_PRIVATE(inode), if it is not a private inode, it will return.
It seems we don't use S_PRIVATE in ocfs2, do we?

Are you sure? This is the code I have in my git tree:

int security_inode_init_security(struct inode *inode, struct inode *dir,
char **name, void **value, size_t *len)

{
	if (unlikely(IS_PRIVATE(inode)))
		return -EOPNOTSUPP;
	return security_ops->inode_init_security(inode, dir, name, value,

len);
}
EXPORT_SYMBOL(security_inode_init_security);

So the check is looking for S_PRIVATE and skipping only if it's set, which would mean that this gets run for most inodes...

Quoted 1 line

anyway, I can add this call in ocfs2 quickly.

Yeah, I think we need it.

Thanks,

	--Mark

--
Mark Fasheh

#6 lore raw

PATCH 2/3 ocfs2: Add security xattr support in ocfs2

In reply to Tiger Yang
Quoted 15 lines · Tiger Yang
fs/ocfs2/xattr.c +0 -2
index 505fb40..d57cfae 100644
@@ -81,9 +81,7 @@ struct xattr_handler *ocfs2_xattr_handlers[] = {
 	&ocfs2_xattr_acl_default_handler,
 #endif
 	&ocfs2_xattr_trusted_handler,
-#ifdef CONFIG_OCFS2_FS_SECURITY
 	&ocfs2_xattr_security_handler,
-#endif
 	NULL
 };

fs/ocfs2/xattr.c static struct xattr_handler *ocfs2_xattr_handler_map[] = { · line 98

Quoted 7 lines
fs/ocfs2/xattr.c +0 -2
@@ -96,9 +94,7 @@ static struct xattr_handler *ocfs2_xattr_handler_map[] = {
 					= &ocfs2_xattr_acl_default_handler,
 #endif
 	[OCFS2_XATTR_INDEX_TRUSTED]	= &ocfs2_xattr_trusted_handler,
-#ifdef CONFIG_OCFS2_FS_SECURITY
 	[OCFS2_XATTR_INDEX_SECURITY]	= &ocfs2_xattr_security_handler,
-#endif
 };

Oh, almost missed this actually. Isn't it customary to allow the user to select whether to compile in support for security attributes, or acls? You can pretty much copy the code from any other file system really.

Also, would you mind rebasing these against the latest xattrs branch in ocfs2.git? The only change was that I removed the code in '#ifdef CONFIG_OCFS2_FS_SECURITY' and '#ifdef CONFIG_OCFS2_FS_ACL' because we weren't ever defining those yet. Obviously, you get to add them back now :)

	--Mark

--
Mark Fasheh

author
#7 lore raw

PATCH 2/3 ocfs2: Add security xattr support in ocfs2

Quoted 3 lines · Mark Fasheh

Oh, almost missed this actually. Isn't it customary to allow the user to select whether to compile in support for security attributes, or acls? You can pretty much copy the code from any other file system really.

OK. I will refer to others.

One more question, They also allow user to select whether support EA, in ocfs2, do we need this option in Kconfig? or we support EA by default?

Quoted 4 lines

Also, would you mind rebasing these against the latest xattrs branch in ocfs2.git? The only change was that I removed the code in '#ifdef CONFIG_OCFS2_FS_SECURITY' and '#ifdef CONFIG_OCFS2_FS_ACL' because we weren't ever defining those yet. Obviously, you get to add them back now :)

no problem. I will rebase the branch and generate all the patches.

best regards,
tiger

#8 lore raw

PATCH 2/3 ocfs2: Add security xattr support in ocfs2

Quoted 8 lines · Tiger Yang
Quoted 3 lines · Mark Fasheh

Oh, almost missed this actually. Isn't it customary to allow the user to select whether to compile in support for security attributes, or acls? You can pretty much copy the code from any other file system really.

OK. I will refer to others.

One more question, They also allow user to select whether support EA, in ocfs2, do we need this option in Kconfig? or we support EA by default?

Hmm... Do you know why they allow EA's to be compiled optionally? Maybe we should do the same. I don't have any strong opinions one way or the other though.

Quoted 6 lines
Quoted 5 lines

Also, would you mind rebasing these against the latest xattrs branch in ocfs2.git? The only change was that I removed the code in '#ifdef CONFIG_OCFS2_FS_SECURITY' and '#ifdef CONFIG_OCFS2_FS_ACL' because we weren't ever defining those yet. Obviously, you get to add them back now :)

no problem. I will rebase the branch and generate all the patches.

Thanks, I appreciate this.

	--Mark

--
Mark Fasheh

#9 lore raw

PATCH 2/3 ocfs2: Add security xattr support in ocfs2

In reply to Mark Fasheh
Quoted 4 lines · Mark Fasheh

Also, would you mind rebasing these against the latest xattrs branch in ocfs2.git? The only change was that I removed the code in '#ifdef CONFIG_OCFS2_FS_SECURITY' and '#ifdef CONFIG_OCFS2_FS_ACL' because we weren't ever defining those yet. Obviously, you get to add them back now :)

An option for ACLs makes some sense, but the security attrs are so little code that's it's pointless.

author
#10 lore raw

PATCH 3/3 ocfs2: Add posix ACL support in ocfs2 v1

In reply to Tiger Yang

This patch add posix ACL support in ocfs2.

Signed-off-by Tiger Yang <[email protected]>
---
 fs/ocfs2/Makefile |    1 +
 fs/ocfs2/acl.c    |  383 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/acl.h    |   40 ++++++
 fs/ocfs2/file.c   |    3 +-
 fs/ocfs2/namei.c  |    3 +
 fs/ocfs2/xattr.c  |    4 -
 fs/ocfs2/xattr.h  |    2 -
 7 files changed, 429 insertions(+), 7 deletions(-)
 create mode 100644 fs/ocfs2/acl.c
 create mode 100644 fs/ocfs2/acl.h
fs/ocfs2/Makefile +1 -0
index 73c1c82..27f0954 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_OCFS2_FS_O2CB) += ocfs2_stack_o2cb.o
 obj-$(CONFIG_OCFS2_FS_USERSPACE_CLUSTER) += ocfs2_stack_user.o
 
 ocfs2-objs := \
+	acl.o			\
 	alloc.o 		\
 	aops.o 			\
 	buffer_head_io.o	\
fs/ocfs2/acl.c new file +383 -0

383 lines not on this page · raw message

fs/ocfs2/acl.h new file +40 -0
new file mode 100644
index 0000000..ba1cb92
@@ -0,0 +1,40 @@
+/* -*- mode: c; c-basic-offset: 8; -*-
+ * vim: noexpandtab sw=8 ts=8 sts=0:
+ *
+ * acl.h
+ *
+ * Function prototypes
+ *
+ * Copyright (C) 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.
+ *
+ * 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_ACL_H
+#define OCFS2_ACL_H
+
+#include <linux/posix_acl_xattr.h>
+
+struct ocfs2_acl_entry {
+	__le16 e_tag;
+	__le16 e_perm;
+	__le32 e_id;
+};
+
+int ocfs2_check_acl(struct inode *inode, int mask);
+int ocfs2_init_acl(struct inode *inode, struct inode *dir);
+
+#endif /* OCFS2_ACL_H */
fs/ocfs2/file.c +2 -1
index edcdd39..96a53eb 100644
@@ -56,6 +56,7 @@
 #include "suballoc.h"
 #include "super.h"
 #include "xattr.h"
+#include "acl.h"
 
 #include "buffer_head_io.h"
 

@@ -1031,7 +1032,7 @@ int ocfs2_permission(struct inode *inode, int mask, struct nameidata *nd)
 		goto out;
 	}
 
-	ret = generic_permission(inode, mask, NULL);
+	ret = generic_permission(inode, mask, ocfs2_check_acl);
 
 	ocfs2_inode_unlock(inode, 0);
 out:
fs/ocfs2/namei.c +3 -0
index 76d1d13..9f8f313 100644
@@ -61,6 +61,7 @@
 #include "sysfile.h"
 #include "uptodate.h"
 #include "xattr.h"
+#include "acl.h"
 
 #include "buffer_head_io.h"
 

@@ -328,6 +329,8 @@ leave:
 	if (status == -ENOSPC)
 		mlog(0, "Disk is full\n");
 
+	status = ocfs2_init_acl(inode, dir);
+
 	if (new_fe_bh)
 		brelse(new_fe_bh);
fs/ocfs2/xattr.c +0 -4
index d57cfae..1daff4f 100644
@@ -76,10 +76,8 @@ static struct ocfs2_xattr_def_value_root def_xv = {
 
 struct xattr_handler *ocfs2_xattr_handlers[] = {
 	&ocfs2_xattr_user_handler,
-#ifdef CONFIG_OCFS2_FS_POSIX_ACL
 	&ocfs2_xattr_acl_access_handler,
 	&ocfs2_xattr_acl_default_handler,
-#endif
 	&ocfs2_xattr_trusted_handler,
 	&ocfs2_xattr_security_handler,
 	NULL

@@ -87,12 +85,10 @@ struct xattr_handler *ocfs2_xattr_handlers[] = {
 
 static struct xattr_handler *ocfs2_xattr_handler_map[] = {
 	[OCFS2_XATTR_INDEX_USER]	= &ocfs2_xattr_user_handler,
-#ifdef CONFIG_OCFS2_FS_POSIX_ACL
 	[OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS]
 					= &ocfs2_xattr_acl_access_handler,
 	[OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT]
 					= &ocfs2_xattr_acl_default_handler,
-#endif
 	[OCFS2_XATTR_INDEX_TRUSTED]	= &ocfs2_xattr_trusted_handler,
 	[OCFS2_XATTR_INDEX_SECURITY]	= &ocfs2_xattr_security_handler,
 };
fs/ocfs2/xattr.h +0 -2
index f3ec79a..b8d6522 100644
@@ -40,10 +40,8 @@ enum ocfs2_xattr_type {
 
 extern struct xattr_handler ocfs2_xattr_user_handler;
 extern struct xattr_handler ocfs2_xattr_trusted_handler;
-#ifdef CONFIG_OCFS2_FS_POSIX_ACL
 extern struct xattr_handler ocfs2_xattr_acl_access_handler;
 extern struct xattr_handler ocfs2_xattr_acl_default_handler;
-#endif
 extern struct xattr_handler ocfs2_xattr_security_handler;
 extern struct xattr_handler *ocfs2_xattr_handlers[];
Signature · 1 line
1.5.4.1
#11 lore raw

PATCH 3/3 ocfs2: Add posix ACL support in ocfs2 v1

Quoted 13 lines · Tiger Yang
fs/ocfs2/file.c +1 -0
index edcdd39..96a53eb 100644
@@ -56,6 +56,7 @@
 #include "suballoc.h"
 #include "super.h"
 #include "xattr.h"
+#include "acl.h"
 
 #include "buffer_head_io.h"

fs/ocfs2/file.c int ocfs2_permission(struct inode *inode, int mask, struct nameidata *nd) · line 1038

Quoted 8 lines
fs/ocfs2/file.c +1 -1
@@ -1031,7 +1032,7 @@ int ocfs2_permission(struct inode *inode, int mask, struct nameidata *nd)
 		goto out;
 	}
 
-	ret = generic_permission(inode, mask, NULL);
+	ret = generic_permission(inode, mask, ocfs2_check_acl);
 
 	ocfs2_inode_unlock(inode, 0);
 out:

I think we also need some acl-specific handling of chmod in ocfs2_setattr. See ext3_acl_chmod, where it's called and what it does.

Quoted 13 lines
fs/ocfs2/namei.c +1 -0
index 76d1d13..9f8f313 100644
@@ -61,6 +61,7 @@
 #include "sysfile.h"
 #include "uptodate.h"
 #include "xattr.h"
+#include "acl.h"
 
 #include "buffer_head_io.h"

fs/ocfs2/namei.c leave: · line 332

Quoted 4 lines
fs/ocfs2/namei.c +1 -0
@@ -328,6 +329,8 @@ leave:
 	if (status == -ENOSPC)
 		mlog(0, "Disk is full\n");
 
+	status = ocfs2_init_acl(inode, dir);

Err, this is a pretty bad place for a call which must do work only if we're succesfull in creating the inode. A better place would be a few lines up, maybe even just before the call to 'ocfs2_add_entry()', so that an acl failure won't result in a non-acl-copied inode which is accessible from a directory.

Quoted 22 lines
fs/ocfs2/namei.c +1 -0
+
 	if (new_fe_bh)
 		brelse(new_fe_bh);
fs/ocfs2/xattr.c +0 -2
index d57cfae..1daff4f 100644
@@ -76,10 +76,8 @@ static struct ocfs2_xattr_def_value_root def_xv = {
 
 struct xattr_handler *ocfs2_xattr_handlers[] = {
 	&ocfs2_xattr_user_handler,
-#ifdef CONFIG_OCFS2_FS_POSIX_ACL
 	&ocfs2_xattr_acl_access_handler,
 	&ocfs2_xattr_acl_default_handler,
-#endif
 	&ocfs2_xattr_trusted_handler,
 	&ocfs2_xattr_security_handler,
 	NULL

@@ -87,12 +85,10 @@ struct xattr_handler *ocfs2_xattr_handlers[] = {
 
 static struct xattr_handler *ocfs2_xattr_handler_map[] = {

fs/ocfs2/xattr.c struct xattr_handler *ocfs2_xattr_handlers[] = { · line 94

Quoted 10 lines
fs/ocfs2/xattr.c +0 -2
 	[OCFS2_XATTR_INDEX_USER]	= &ocfs2_xattr_user_handler,
-#ifdef CONFIG_OCFS2_FS_POSIX_ACL
 	[OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS]
 					= &ocfs2_xattr_acl_access_handler,
 	[OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT]
 					= &ocfs2_xattr_acl_default_handler,
-#endif
 	[OCFS2_XATTR_INDEX_TRUSTED]	= &ocfs2_xattr_trusted_handler,
 	[OCFS2_XATTR_INDEX_SECURITY]	= &ocfs2_xattr_security_handler,
 };

Same as before about the #ifdef's, and adding a proper Kconfig item for this.

	--Mark

--
Mark Fasheh

author
#12 lore raw

PATCH 3/3 ocfs2: Add posix ACL support in ocfs2 v1

Hi, Mark,

Quoted 2 lines · Mark Fasheh

I think we also need some acl-specific handling of chmod in ocfs2_setattr. See ext3_acl_chmod, where it's called and what it does.

OK. I will add it later after learning ext3.

Quoted 5 lines

Err, this is a pretty bad place for a call which must do work only if we're succesfull in creating the inode. A better place would be a few lines up, maybe even just before the call to 'ocfs2_add_entry()', so that an acl failure won't result in a non-acl-copied inode which is accessible from a directory.

I put this function here because ocfs2_xattr_set need start transaction. So I put it after commit transaction.

To avoid the issue you mentioned, I modify it to the following
- status = ocfs2_init_acl(inode, dir);
+ if (!status) {
+ if (ocfs2_init_acl(inode, dir))
+ mlog(0, "Init acl error\n");
+ }
is this acceptable?

Quoted 2 lines

Same as before about the #ifdef's, and adding a proper Kconfig item for this.

No problem.

Best regards,
tiger

#13 lore raw

PATCH 3/3 ocfs2: Add posix ACL support in ocfs2 v1

Quoted 4 lines · Tiger Yang
Quoted 2 lines · Mark Fasheh

I think we also need some acl-specific handling of chmod in ocfs2_setattr. See ext3_acl_chmod, where it's called and what it does.

OK. I will add it later after learning ext3.

Great, thanks.

Quoted 14 lines
Quoted 5 lines

Err, this is a pretty bad place for a call which must do work only if we're succesfull in creating the inode. A better place would be a few lines up, maybe even just before the call to 'ocfs2_add_entry()', so that an acl failure won't result in a non-acl-copied inode which is accessible from a directory.

I put this function here because ocfs2_xattr_set need start transaction. So I put it after commit transaction.

To avoid the issue you mentioned, I modify it to the following

(quoted diff) +4 -1
-       status = ocfs2_init_acl(inode, dir);
+       if (!status) {
+               if (ocfs2_init_acl(inode, dir))
+                       mlog(0, "Init acl error\n");
+       }
Quoted 1 line

is this acceptable?

It might be better if we can add the acls before the directory entry. This means though that you'll have to add them as part of the current transaction. I think this requires a few changes:
- Add code to reserve the meta data alloc file
- Symlinks will need this if the link name is so large it won't allow inline EA's.
- 512 byte blocks will need to always reserve a block for the external EA block
- If the acl value won't fit inline, reserve more clusters from the cluster allocator.
- Add the proper number of credits to those already passed to ocfs2_start_trans(). I think the common case will be zero, but when we attach an external ea block or have a value in an extent, we'll need more.
- Pass the handle, and allocators through to ocfs2_init_acl() so that it can all be done in the current transaction.

Thanks,

	--Mark

--
Mark Fasheh